Last modified: 2009-09-08 14:29:36 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T20827, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 18827 - PageHistory.php deleterevision <form> breaks on ugly URL wikis
PageHistory.php deleterevision <form> breaks on ugly URL wikis
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
Revision deletion (Other open bugs)
1.16.x
All All
: Normal normal (vote)
: ---
Assigned To: Aaron Schulz
:
Depends on:
Blocks: 18674 revdel
  Show dependency treegraph
 
Reported: 2009-05-18 01:27 UTC by Dan Jacobson
Modified: 2009-09-08 14:29 UTC (History)
4 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Dan Jacobson 2009-05-18 01:27:56 UTC
Big problems at PageHistory.php's
  if( $this->linesonpage > 1 && $wgUser->isAllowed('deleterevision') ) {...

which makes
<form action="http://example.org/index.php?title=Special:Revisiondelete"
            method="get" id="mw-history-revdeleteform"
            style="visibility:hidden;float:right;">
              <input name="target" type="hidden" value="A" />
              <input name="oldid" type="hidden" value=""
              id="revdel-oldid" />
              <input type="submit"
              value="Show/hide selected revisions" />
            </form>

Let's examine this one by one:
1. action="http://example.org/index.php?title=Special:Revisiondelete"
This should just be action="/index.php". What is a "?" doing in
action? Maybe you fellows were just testing with "pretty URLs" wikis.

2. You depend on Javascript to put the values into the form, as
apparently this is the only way you can deal with "dueling forms" here.

Why not combine the two forms into one? Just have a different <input
type="submit" ...> for the second.

I would be willing to write a patch, if you were willing to not insist
on Javascript.

P.S., I don't know what that style="visibility:hidden" stuff is hiding.

By the way,
the above test should be reversed, to
  if( $wgUser->isAllowed('deleterevision') && $this->linesonpage > 1  ) {...
considering most views are from normal users, so quit early.

And here's the background of how I found the bug:

Noting SpecialRevisiondelete.php is the second largest special page,
$ ls -S specials|nl|sed 3q
     1	SpecialUpload.php
     2	SpecialRevisiondelete.php
     3	SpecialSearch.php
I decided to give it a try.
We see in the HISTORY file
* Further work on rev_deleted; changed to a bitfield with several data-hiding
  options. Not yet ready for production use; Special:Revisiondelete is
  incomplete, and the flags are not preserved across page deletion/undeletion.
  To try it; add the 'deleterevision' permission to a privileged group.
OK, we do
$wgGroupPermissions['sysop']['deleterevision']=true;
and proceed to browse the history of some page (with more than 1 revision),
   http://example.org/index.php?title=A&action=history ... and the
rest is, well, history.
Comment 1 Mike.lifeguard 2009-05-18 01:53:11 UTC
Could you please provide an accurate bug summary? Clearly the form /does/ work as written, since we're using it.
Comment 2 Roan Kattouw 2009-05-18 07:05:57 UTC
(In reply to comment #0)
> Big problems at PageHistory.php's
>   if( $this->linesonpage > 1 && $wgUser->isAllowed('deleterevision') ) {...
> 
> which makes
> <form action="http://example.org/index.php?title=Special:Revisiondelete"
>             method="get" id="mw-history-revdeleteform"
>             style="visibility:hidden;float:right;">
>               <input name="target" type="hidden" value="A" />
>               <input name="oldid" type="hidden" value=""
>               id="revdel-oldid" />
>               <input type="submit"
>               value="Show/hide selected revisions" />
>             </form>
> 
> Let's examine this one by one:
> 1. action="http://example.org/index.php?title=Special:Revisiondelete"
> This should just be action="/index.php". What is a "?" doing in
> action? Maybe you fellows were just testing with "pretty URLs" wikis.
> 
POSTing to URLs with query strings works for me (haven't tested this particular form, but I've used it in other web apps).

> 2. You depend on Javascript to put the values into the form, as
> apparently this is the only way you can deal with "dueling forms" here.
> 
> Why not combine the two forms into one? Just have a different <input
> type="submit" ...> for the second.
> 
Yeah, this looks evil to me.

> I would be willing to write a patch, if you were willing to not insist
> on Javascript.
> 
Nobody's insisting on anything. The fact that there's Javascript in that form doesn't automatically mean that everyone agrees with it and will defend its existence, as you seem to be suggesting. Go right ahead and write a patch, nobody's discouraging you.


> P.S., I don't know what that style="visibility:hidden" stuff is hiding.
> 
It seems to be hiding the form; a piece of Javascript is probably unhiding it though.

> By the way,
> the above test should be reversed, to
>   if( $wgUser->isAllowed('deleterevision') && $this->linesonpage > 1  ) {...
> considering most views are from normal users, so quit early.
> 
There's no real gain in that, since the $this->linesonpage > 1 check isn't slow. If anything, isAllowed() is probably slower.

The summary still doesn't look right to me: I'm pretty sure the form will work on ugly URL wikis (will test when I have a chance), but it seems the form is relying on Javascript while that's probably not necessary. Assigning to Aaron as RevisionDelete is his baby IIRC.
Comment 3 Dan Jacobson 2009-05-18 23:53:52 UTC
> POSTing to URLs with query strings works for me
I'll believe it when I see it.
Re: visibility:hidden, and also bug 18674: just be sure to not
violate (the spirit too of)
http://en.wikipedia.org/wiki/Wikipedia:Accessibility
http://en.wikipedia.org/wiki/Wikipedia:HiddenStructure
even if only for administrators. OK, leaving this in
Aaron's hands.
Comment 4 Niklas Laxström 2009-06-20 08:01:18 UTC
1) Works for me. Can anybody reproduce?
2) One thing in one bug please!
Comment 5 Alexandre Emsenhuber [IAlex] 2009-07-23 20:39:04 UTC
The form works with ugly URLs since r51279, but other issues are still not fixed.
Comment 6 Aaron Schulz 2009-09-02 21:43:21 UTC
Tim removed the JS dependencies.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links