r91949 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r91948‎ | r91949 | r91950 >
Date:11:53, 12 July 2011
Author:diebuche
Status:reverted (Comments)
Tags:
Comment:
Revert r91942, r91943 & reimplement. wgActions doesn't contain all possible actions!
Modified paths:
  • /trunk/phase3/includes/Wiki.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Wiki.php
——@@ -257,20 +257,20 @@
258258 }
259259
260260 /**
261 - * Returns the action that will be executed, not necesserly the one passed
 261+ * Returns the action that will be executed, not necessarily the one passed
262262 * passed through the "action" parameter. Actions disabled in
263263 * $wgDisabledActions will be replaced by "nosuchaction"
264264 *
265265 * @return String: action
266266 */
267267 public function getAction() {
268 - global $wgDisabledActions, $wgActions;
 268+ global $wgDisabledActions;
269269
270270 $request = $this->context->getRequest();
271271 $action = $request->getVal( 'action', 'view' );
272272
273273 // Check for disabled actions
274 - if ( in_array( $action, $wgDisabledActions ) || !in_array( $action, $wgActions ) ) {
 274+ if ( in_array( $action, $wgDisabledActions ) ) {
275275 $action = 'nosuchaction';
276276 } elseif ( $action === 'historysubmit' ) {
277277 // Workaround for bug #20966: inability of IE to provide an action dependent
——@@ -501,6 +501,7 @@
502502 break;
503503 default:
504504 if ( wfRunHooks( 'UnknownAction', array( $act, $article ) ) ) {
 505+ $request->setVal( 'action', 'nosuchaction' );
505506 $output->showErrorPage( 'nosuchaction', 'nosuchactiontext' );
506507 }
507508 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r92156r91949 : EditWatchlist assumed that action was always empty for viewdiebuche13:01, 14 July 2011
r93858Reverted r91942,r91943,r91949,r92156 per CRaaron22:08, 3 August 2011
r94446MFT to REL1_18:...hashar09:27, 14 August 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r91871Add action-* class to body. Bug 4438. Based on patch by Subfaderdiebuche10:57, 11 July 2011
r91942Followup r91871 per CR. Write back the actually executed action to . Ping r57415diebuche09:42, 12 July 2011
r91943r91942: Add nosuchaction also if action not indiebuche09:58, 12 July 2011

Comments

#Comment by Brion VIBBER (talk | contribs)   17:53, 12 July 2011

Ah this should work nicely. :)

Longer-term we should see about putting the canonicalized 'action' var into the Context state, but for now it's read back out of the request in many places, and this should usually clear it up.

One thing to watch out for might be extra 'action' parameters sneaking into URL generation when back-forming from existing links, but this shouldn't I think be a problem from what I'm seeing so far.

#Comment by SPQRobin (talk | contribs)   16:48, 13 July 2011

Stuff like $action = $wgRequest->getVal( 'action', $par ); in special pages like EditWatchlist/raw return the default action ('view' probably) instead of the $par, which is 'raw' in the case of EditWatchlist. Another example is the Interwiki extension: Special:Interwiki/add does not work, but Special:Interwiki?action=add does.

This is probably due to this commit, so marking as FIXME.

#Comment by SPQRobin (talk | contribs)   16:54, 13 July 2011

Indeed, it worked until r91941.

#Comment by Brion VIBBER (talk | contribs)   18:16, 13 July 2011

Looks like r34444 tried to fix this problem in EditWatchlist (for another case where running on a particular web server config the action was always set), but it was reverted in r34462, I'm guessing due to r34444 mistakenly doing an === compare between null and "").

#Comment by DieBuche (talk | contribs)   13:02, 14 July 2011

EditWatchlist is fixed in r92156

#Comment by SPQRobin (talk | contribs)   13:15, 14 July 2011

That is a hack only for EditWatchlist, while the syntax $request->getVal( 'action', $par ) is used on a lot of (special) pages, so this commit should be fixed so that it works again everywhere (and r92156 is not necessary).

#Comment by SPQRobin (talk | contribs)   13:21, 14 July 2011

In other words, it is/was expected that the action is empty when it is actually view.

Another observation due to this commit: Going to Special:Random redirects you to a page with the &action=view in the URL, which was not the case before this commit.

#Comment by SPQRobin (talk | contribs)   18:14, 22 July 2011

Note that I worked around this in the SpecialInterwiki extension, but it is still needed to fix this commit.

#Comment by 😂 (talk | contribs)   18:06, 26 July 2011

Suggest reversion of this and related revs until we have a sane canonical way of doing actions per Brion's comment above.

#Comment by Aaron Schulz (talk | contribs)   21:53, 3 August 2011

RevisionDelete broken for history buttons: 

if ( $request->getVal( 'action' ) == 'historysubmit' ) {

Status & tagging log