r102192 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r102191‎ | r102192 | r102193 >
Date:20:30, 6 November 2011
Author:ialex
Status:ok
Tags:
Comment:
* Only show "view" tabs when the user hasn't the permission to read the page and put the whole stuff in a $userCanRead check instead of doing that for each check
* Factorise duplicate code (for the "protect" tab generation)
* Use Title::quickUserCan() check for delete and protect actions instead of User::isAllowed()
* Pass the User object to Title::quickUserCan()
Modified paths:
  • /trunk/phase3/includes/SkinTemplate.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/SkinTemplate.php
@@ -806,8 +806,7 @@
807807 // parameters
808808 $action = $request->getVal( 'action', 'view' );
809809
810 - $userCanRead = $title->userCanRead();
811 - $skname = $this->skinname;
 810+ $userCanRead = $title->quickUserCan( 'read', $user );
812811
813812 $preventActiveTabs = false;
814813 wfRunHooks( 'SkinTemplatePreventOtherActiveTabs', array( &$this, &$preventActiveTabs ) );
@@ -830,6 +829,8 @@
831830 $talkId = "{$subjectId}_talk";
832831 }
833832
 833+ $skname = $this->skinname;
 834+
834835 // Adds namespace links
835836 $subjectMsg = array( "nstab-$subjectId" );
836837 if ( $subjectPage->isMainPage() ) {
@@ -844,160 +845,148 @@
845846 );
846847 $content_navigation['namespaces'][$talkId]['context'] = 'talk';
847848
848 - // Adds view view link
849 - if ( $title->exists() && $userCanRead ) {
850 - $content_navigation['views']['view'] = $this->tabAction(
851 - $isTalk ? $talkPage : $subjectPage,
852 - array( "$skname-view-view", 'view' ),
853 - ( $onPage && ($action == 'view' || $action == 'purge' ) ), '', true
854 - );
855 - $content_navigation['views']['view']['redundant'] = true; // signal to hide this from simple content_actions
856 - }
 849+ if ( $userCanRead ) {
 850+ // Adds view view link
 851+ if ( $title->exists() ) {
 852+ $content_navigation['views']['view'] = $this->tabAction(
 853+ $isTalk ? $talkPage : $subjectPage,
 854+ array( "$skname-view-view", 'view' ),
 855+ ( $onPage && ($action == 'view' || $action == 'purge' ) ), '', true
 856+ );
 857+ $content_navigation['views']['view']['redundant'] = true; // signal to hide this from simple content_actions
 858+ }
857859
858 - wfProfileIn( __METHOD__ . '-edit' );
 860+ wfProfileIn( __METHOD__ . '-edit' );
859861
860 - // Checks if user can...
861 - if (
862 - // read and edit the current page
863 - $userCanRead && $title->quickUserCan( 'edit' ) &&
864 - (
865 - // if it exists
866 - $title->exists() ||
867 - // or they can create one here
868 - $title->quickUserCan( 'create' )
869 - )
870 - ) {
871 - // Builds CSS class for talk page links
872 - $isTalkClass = $isTalk ? ' istalk' : '';
873 - $section = $request->getVal( 'section' );
 862+ // Checks if user can edit the current page if it exists or create it otherwise
 863+ if ( $title->quickUserCan( 'edit', $user ) && ( $title->exists() || $title->quickUserCan( 'create', $user ) ) ) {
 864+ // Builds CSS class for talk page links
 865+ $isTalkClass = $isTalk ? ' istalk' : '';
 866+ $section = $request->getVal( 'section' );
874867
875 - // Determines if we're in edit mode
876 - $selected = (
877 - $onPage &&
878 - ( $action == 'edit' || $action == 'submit' ) &&
879 - ( $section != 'new' )
880 - );
881 - $msgKey = $title->exists() || ( $title->getNamespace() == NS_MEDIAWIKI && $title->getDefaultMessageText() !== false ) ?
882 - "edit" : "create";
883 - $content_navigation['views']['edit'] = array(
884 - 'class' => ( $selected ? 'selected' : '' ) . $isTalkClass,
885 - 'text' => wfMessageFallback( "$skname-view-$msgKey", $msgKey )->setContext( $this->getContext() )->text(),
886 - 'href' => $title->getLocalURL( $this->editUrlOptions() ),
887 - 'primary' => true, // don't collapse this in vector
888 - );
889 - // Checks if this is a current rev of talk page and we should show a new
890 - // section link
891 - if ( ( $isTalk && $this->isRevisionCurrent() ) || ( $out->showNewSectionLink() ) ) {
892 - // Checks if we should ever show a new section link
893 - if ( !$out->forceHideNewSectionLink() ) {
894 - // Adds new section link
895 - //$content_navigation['actions']['addsection']
896 - $content_navigation['views']['addsection'] = array(
897 - 'class' => $section == 'new' ? 'selected' : false,
898 - 'text' => wfMessageFallback( "$skname-action-addsection", 'addsection' )->setContext( $this->getContext() )->text(),
899 - 'href' => $title->getLocalURL( 'action=edit&section=new' )
900 - );
 868+ // Determines if we're in edit mode
 869+ $selected = (
 870+ $onPage &&
 871+ ( $action == 'edit' || $action == 'submit' ) &&
 872+ ( $section != 'new' )
 873+ );
 874+ $msgKey = $title->exists() || ( $title->getNamespace() == NS_MEDIAWIKI && $title->getDefaultMessageText() !== false ) ?
 875+ "edit" : "create";
 876+ $content_navigation['views']['edit'] = array(
 877+ 'class' => ( $selected ? 'selected' : '' ) . $isTalkClass,
 878+ 'text' => wfMessageFallback( "$skname-view-$msgKey", $msgKey )->setContext( $this->getContext() )->text(),
 879+ 'href' => $title->getLocalURL( $this->editUrlOptions() ),
 880+ 'primary' => true, // don't collapse this in vector
 881+ );
 882+ // Checks if this is a current rev of talk page and we should show a new
 883+ // section link
 884+ if ( ( $isTalk && $this->isRevisionCurrent() ) || ( $out->showNewSectionLink() ) ) {
 885+ // Checks if we should ever show a new section link
 886+ if ( !$out->forceHideNewSectionLink() ) {
 887+ // Adds new section link
 888+ //$content_navigation['actions']['addsection']
 889+ $content_navigation['views']['addsection'] = array(
 890+ 'class' => $section == 'new' ? 'selected' : false,
 891+ 'text' => wfMessageFallback( "$skname-action-addsection", 'addsection' )->setContext( $this->getContext() )->text(),
 892+ 'href' => $title->getLocalURL( 'action=edit&section=new' )
 893+ );
 894+ }
901895 }
 896+ // Checks if the page has some kind of viewable content
 897+ } elseif ( $title->hasSourceText() ) {
 898+ // Adds view source view link
 899+ $content_navigation['views']['viewsource'] = array(
 900+ 'class' => ( $onPage && $action == 'edit' ) ? 'selected' : false,
 901+ 'text' => wfMessageFallback( "$skname-action-viewsource", 'viewsource' )->setContext( $this->getContext() )->text(),
 902+ 'href' => $title->getLocalURL( $this->editUrlOptions() ),
 903+ 'primary' => true, // don't collapse this in vector
 904+ );
902905 }
903 - // Checks if the page has some kind of viewable content
904 - } elseif ( $title->hasSourceText() && $userCanRead ) {
905 - // Adds view source view link
906 - $content_navigation['views']['viewsource'] = array(
907 - 'class' => ( $onPage && $action == 'edit' ) ? 'selected' : false,
908 - 'text' => wfMessageFallback( "$skname-action-viewsource", 'viewsource' )->setContext( $this->getContext() )->text(),
909 - 'href' => $title->getLocalURL( $this->editUrlOptions() ),
910 - 'primary' => true, // don't collapse this in vector
911 - );
912 - }
913 - wfProfileOut( __METHOD__ . '-edit' );
 906+ wfProfileOut( __METHOD__ . '-edit' );
914907
915 - wfProfileIn( __METHOD__ . '-live' );
 908+ wfProfileIn( __METHOD__ . '-live' );
 909+ // Checks if the page exists
 910+ if ( $title->exists() ) {
 911+ // Adds history view link
 912+ $content_navigation['views']['history'] = array(
 913+ 'class' => ( $onPage && $action == 'history' ) ? 'selected' : false,
 914+ 'text' => wfMessageFallback( "$skname-view-history", 'history_short' )->setContext( $this->getContext() )->text(),
 915+ 'href' => $title->getLocalURL( 'action=history' ),
 916+ 'rel' => 'archives',
 917+ );
916918
917 - // Checks if the page exists
918 - if ( $title->exists() && $userCanRead ) {
919 - // Adds history view link
920 - $content_navigation['views']['history'] = array(
921 - 'class' => ( $onPage && $action == 'history' ) ? 'selected' : false,
922 - 'text' => wfMessageFallback( "$skname-view-history", 'history_short' )->setContext( $this->getContext() )->text(),
923 - 'href' => $title->getLocalURL( 'action=history' ),
924 - 'rel' => 'archives',
925 - );
 919+ if ( $title->quickUserCan( 'delete', $user ) ) {
 920+ $content_navigation['actions']['delete'] = array(
 921+ 'class' => ( $onPage && $action == 'delete' ) ? 'selected' : false,
 922+ 'text' => wfMessageFallback( "$skname-action-delete", 'delete' )->setContext( $this->getContext() )->text(),
 923+ 'href' => $title->getLocalURL( 'action=delete' )
 924+ );
 925+ }
 926+ if ( $title->quickUserCan( 'move', $user ) ) {
 927+ $moveTitle = SpecialPage::getTitleFor( 'Movepage', $title->getPrefixedDBkey() );
 928+ $content_navigation['actions']['move'] = array(
 929+ 'class' => $this->getTitle()->isSpecial( 'Movepage' ) ? 'selected' : false,
 930+ 'text' => wfMessageFallback( "$skname-action-move", 'move' )->setContext( $this->getContext() )->text(),
 931+ 'href' => $moveTitle->getLocalURL()
 932+ );
 933+ }
926934
927 - if( $user->isAllowed( 'delete' ) ) {
928 - $content_navigation['actions']['delete'] = array(
929 - 'class' => ( $onPage && $action == 'delete' ) ? 'selected' : false,
930 - 'text' => wfMessageFallback( "$skname-action-delete", 'delete' )->setContext( $this->getContext() )->text(),
931 - 'href' => $title->getLocalURL( 'action=delete' )
932 - );
 935+ $isProtected = $title->isProtected();
 936+ } else {
 937+ // article doesn't exist or is deleted
 938+ if ( $user->isAllowed( 'deletedhistory' ) ) {
 939+ $n = $title->isDeleted();
 940+ if( $n ) {
 941+ $undelTitle = SpecialPage::getTitleFor( 'Undelete' );
 942+ // If the user can't undelete but can view deleted history show them a "View .. deleted" tab instead
 943+ $msgKey = $user->isAllowed( 'undelete' ) ? 'undelete' : 'viewdeleted';
 944+ $content_navigation['actions']['undelete'] = array(
 945+ 'class' => $this->getTitle()->isSpecial( 'Undelete' ) ? 'selected' : false,
 946+ 'text' => wfMessageFallback( "$skname-action-$msgKey", "{$msgKey}_short" )
 947+ ->setContext( $this->getContext() )->numParams( $n )->text(),
 948+ 'href' => $undelTitle->getLocalURL( array( 'target' => $title->getPrefixedDBkey() ) )
 949+ );
 950+ }
 951+ }
 952+
 953+ $isProtected = $title->getRestrictions( 'create' );
933954 }
934 - if ( $title->quickUserCan( 'move' ) ) {
935 - $moveTitle = SpecialPage::getTitleFor( 'Movepage', $title->getPrefixedDBkey() );
936 - $content_navigation['actions']['move'] = array(
937 - 'class' => $this->getTitle()->isSpecial( 'Movepage' ) ? 'selected' : false,
938 - 'text' => wfMessageFallback( "$skname-action-move", 'move' )->setContext( $this->getContext() )->text(),
939 - 'href' => $moveTitle->getLocalURL()
940 - );
941 - }
942955
943 - if ( $title->getNamespace() !== NS_MEDIAWIKI && $user->isAllowed( 'protect' ) ) {
944 - $mode = !$title->isProtected() ? 'protect' : 'unprotect';
 956+ if ( $title->getNamespace() !== NS_MEDIAWIKI && $title->quickUserCan( 'protect', $user ) ) {
 957+ $mode = $isProtected ? 'unprotect' : 'protect';
945958 $content_navigation['actions'][$mode] = array(
946959 'class' => ( $onPage && $action == $mode ) ? 'selected' : false,
947960 'text' => wfMessageFallback( "$skname-action-$mode", $mode )->setContext( $this->getContext() )->text(),
948961 'href' => $title->getLocalURL( "action=$mode" )
949962 );
950963 }
951 - } else {
952 - // article doesn't exist or is deleted
953 - if ( $user->isAllowed( 'deletedhistory' ) ) {
954 - $n = $title->isDeleted();
955 - if( $n ) {
956 - $undelTitle = SpecialPage::getTitleFor( 'Undelete' );
957 - // If the user can't undelete but can view deleted history show them a "View .. deleted" tab instead
958 - $msgKey = $user->isAllowed( 'undelete' ) ? 'undelete' : 'viewdeleted';
959 - $content_navigation['actions']['undelete'] = array(
960 - 'class' => $this->getTitle()->isSpecial( 'Undelete' ) ? 'selected' : false,
961 - 'text' => wfMessageFallback( "$skname-action-$msgKey", "{$msgKey}_short" )
962 - ->setContext( $this->getContext() )->numParams( $n )->text(),
963 - 'href' => $undelTitle->getLocalURL( array( 'target' => $title->getPrefixedDBkey() ) )
964 - );
965 - }
966 - }
967964
968 - if ( $title->getNamespace() !== NS_MEDIAWIKI && $user->isAllowed( 'protect' ) ) {
969 - $mode = !$title->getRestrictions( 'create' ) ? 'protect' : 'unprotect';
 965+ wfProfileOut( __METHOD__ . '-live' );
 966+
 967+ // Checks if the user is logged in
 968+ if ( $this->loggedin ) {
 969+ /**
 970+ * The following actions use messages which, if made particular to
 971+ * the any specific skins, would break the Ajax code which makes this
 972+ * action happen entirely inline. Skin::makeGlobalVariablesScript
 973+ * defines a set of messages in a javascript object - and these
 974+ * messages are assumed to be global for all skins. Without making
 975+ * a change to that procedure these messages will have to remain as
 976+ * the global versions.
 977+ */
 978+ $mode = $title->userIsWatching() ? 'unwatch' : 'watch';
 979+ $token = WatchAction::getWatchToken( $title, $user, $mode );
970980 $content_navigation['actions'][$mode] = array(
971 - 'class' => ( $onPage && $action == $mode ) ? 'selected' : false,
972 - 'text' => wfMessageFallback( "$skname-action-$mode", $mode )->setContext( $this->getContext() )->text(),
973 - 'href' => $title->getLocalURL( "action=$mode" )
 981+ 'class' => $onPage && ( $action == 'watch' || $action == 'unwatch' ) ? 'selected' : false,
 982+ 'text' => $this->msg( $mode )->text(), // uses 'watch' or 'unwatch' message
 983+ 'href' => $title->getLocalURL( array( 'action' => $mode, 'token' => $token ) )
974984 );
975985 }
976986 }
977 - wfProfileOut( __METHOD__ . '-live' );
978987
979 - // Checks if the user is logged in
980 - if ( $this->loggedin ) {
981 - /**
982 - * The following actions use messages which, if made particular to
983 - * the any specific skins, would break the Ajax code which makes this
984 - * action happen entirely inline. Skin::makeGlobalVariablesScript
985 - * defines a set of messages in a javascript object - and these
986 - * messages are assumed to be global for all skins. Without making
987 - * a change to that procedure these messages will have to remain as
988 - * the global versions.
989 - */
990 - $mode = $title->userIsWatching() ? 'unwatch' : 'watch';
991 - $token = WatchAction::getWatchToken( $title, $user, $mode );
992 - $content_navigation['actions'][$mode] = array(
993 - 'class' => $onPage && ( $action == 'watch' || $action == 'unwatch' ) ? 'selected' : false,
994 - 'text' => $this->msg( $mode )->text(), // uses 'watch' or 'unwatch' message
995 - 'href' => $title->getLocalURL( array( 'action' => $mode, 'token' => $token ) )
996 - );
997 - }
998 -
999988 wfRunHooks( 'SkinTemplateNavigation', array( &$this, &$content_navigation ) );
1000989
1001 - if ( !$wgDisableLangConversion ) {
 990+ if ( $userCanRead && !$wgDisableLangConversion ) {
1002991 $pageLang = $title->getPageLanguage();
1003992 // Gets list of language variants
1004993 $variants = $pageLang->getVariants();

Follow-up revisions

RevisionCommit summaryAuthorDate
r102281* Fix checks to show whether "edit" and "addsection" tabs should be marked as...ialex13:42, 7 November 2011

Status & tagging log