Last modified: 2013-02-26 02:26:13 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 T40798, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 38798 - Garbage in URL after saving an edit on the English Wikipedia
Garbage in URL after saving an edit on the English Wikipedia
Status: RESOLVED FIXED
Product: Wikimedia
Classification: Unclassified
General/Unknown (Other open bugs)
unspecified
All All
: Unprioritized normal (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-29 18:40 UTC by MZMcBride
Modified: 2013-02-26 02:26 UTC (History)
10 users (show)

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


Attachments

Description MZMcBride 2012-07-29 18:40:37 UTC
When I save a page on the English Wikipedia, I'm currently redirected to a URL that contains incomprehensible garbage. Example after editing a section on a user talk page: <https://en.wikipedia.org/w/index.php?title=User_talk:Kirill_Lokshin&pe=1&#You.27re_invited_to_Masterpiece_Museum_Edit-a-Thon.21>.

The URL has been rewritten from example.com/wiki/ syntax to the far worse example.com/w/index.php syntax. It now also includes "&pe=1", which makes sense to no one.

URLs are very important. They're heavily passed around on the Internet. Unless you have a really good reason to be messing around with them and inserting garbage into them, PLEASE DON'T DO IT.

I believe this particular garbage is coming from the E3 team. I see absolutely no reason it should be doing this for my user account.
Comment 1 Bawolff (Brian Wolff) 2012-07-29 18:51:25 UTC
wow, people get angsty over short urls ;)


yeah is the e3 people (As I'm sure you've seen from the VP posts...). I think the pe stands for post-edit feedback or something.
Comment 2 Ori Livneh 2012-07-29 19:35:47 UTC
You're welcome to submit a patch, but I won't fix this unless I have the time to spare. This is an annoyance, not a bug, and it's temporary.

Our experiments are live for a short period of time, and they are optimized for insight and speed rather than polish. They are not permanent features. They will be taken down regardless of how well-liked they are, because they are there to answer questions, not to be liked.

In April of 2011, the board unanimously approved Resolution:Openness[1], which designates editor engagement and retention as the top priority for the foundation. We won't fix the editor decline problem unless we know what is causing it, and we won't know what is causing it unless we run a set of experiments that test a range of hypotheses.

Multivariate testing *always* takes a toll. Think, for example, how disorienting it would be to ask a question on IRC on the presumption that the interface you just interacted with is universal, only to find that no one knows what you're talking about, because they haven't been bucketed into the same experimental group as you. We are aware of these effects and strive to minimize them, but we will allow ourselves to behave like elephants from time to time and we expect to be suffered.

  [1]: http://wikimediafoundation.org/wiki/Resolution:Openness
Comment 3 Bawolff (Brian Wolff) 2012-07-29 20:22:00 UTC
>"but we will allow ourselves to behave like elephants from time to time
>and we expect to be suffered."

*get's the popcorn*

-----

On a serious note, the linker supports an option to use short urls even with parameters ("forcearticlepath") [OTOH i don't even know if this link comes from the linker]. If the long url is the primary annoyance (unclear if it is, or if the fact that the url is non-canonical whatsoever) one could use that.
Comment 4 Steven Walling 2012-07-29 21:08:57 UTC
Per Ori's comment and our statements on the Village Pump, I'm recommending we close this as WONTFIX, with the caveat that patches are welcome.  

This experiment will end in two weeks, at which point URLs will return to normal. In the meantime, manually entering or clicking on URLs without the &pe=1 string will work as expected, so no breakage should occur, and thus I'm going to second the request that you bear with our temporary imperfections in implementation.  More about our testing of this feature at: https://www.mediawiki.org/wiki/Talk:Post-edit_feedback and https://www.mediawiki.org/wiki/Extension:E3_Experiments/Testing -- if you see any major cases that we have missed, please leave a comment.
Comment 5 Jarry1250 2012-07-29 21:47:55 UTC
> On a serious note, the linker supports an option to use short urls even with
> parameters ("forcearticlepath") [OTOH i don't even know if this link comes from
> the linker]. If the long url is the primary annoyance (unclear if it is, or if
> the fact that the url is non-canonical whatsoever) one could use that.

It's not the new extension that's choosing to breakout into long URLs, it's some logic in Title::getFullURL(). A bug could be raised against core for that, perhaps.
Comment 6 MZMcBride 2012-07-29 23:48:08 UTC
(In reply to comment #4)
> Per Ori's comment and our statements on the Village Pump, I'm recommending we
> close this as WONTFIX, with the caveat that patches are welcome.

The experiment is testing established community members' patience?

I'd _strongly_ urge you to not take brain-dead actions such as closing this bug as wontfix and ignoring the community (I wasn't even aware of the village pump discussions, but they're absolutely no surprise given the stupidity of this code's present behavior).

You're playing with very hot fire here and if you continue experimenting on established community members, I think you'll quickly find the E3 team marked as historical.
Comment 7 Mark Holmquist 2012-07-30 00:43:51 UTC
Guys, guys. 4 lines. Chill out.

https://gerrit.wikimedia.org/r/16937

If there's additional stuff that needs to happen for this to be closed, feel free to elaborate.
Comment 8 Mark Holmquist 2012-07-30 02:01:49 UTC
After discussions on IRC, we have hashed out the problem a little further:

There are several places where the experiment is outlined as only applying to users who register new accounts within some short period before the deployment date. Basically, there is no check for that condition, because the constraints are magic-number'd into the client-side code, and cannot be used on the backend right now without some more changes.

This seems like it's a rather integral part of the experiment's parameters, so it would be awesome to have it working, especially considering the (apparently) public assurances that it wouldn't affect established users.

I'm not currently sure about the best way to go about this, it probably involves using config variables to pass data between client and server, and making them more configurable. More on that tomorrow, perhaps.
Comment 9 MZMcBride 2012-07-30 02:02:15 UTC
(In reply to comment #7)
> Guys, guys. 4 lines. Chill out.
> 
> https://gerrit.wikimedia.org/r/16937
> 
> If there's additional stuff that needs to happen for this to be closed, feel
> free to elaborate.

Thank you for attempting to address the problem, however this bug is not about making the checks consistent between the PHP side and the JavaScript side.

This bug is about not hitting every editor with code that's only designed to be used with brand new editors. Doing a simple check of $wgUser->isNewbie() or something should suffice. Alternately, you can check registration time, edit count, or a number of other metrics that are available on the PHP and JavaScript sides.

But by any metric, my account (and the accounts of nearly every current user) should not be hit by this code.
Comment 10 Bawolff (Brian Wolff) 2012-07-30 12:13:02 UTC
(In reply to comment #5)
> > On a serious note, the linker supports an option to use short urls even with
> > parameters ("forcearticlepath") [OTOH i don't even know if this link comes from
> > the linker]. If the long url is the primary annoyance (unclear if it is, or if
> > the fact that the url is non-canonical whatsoever) one could use that.
> 
> It's not the new extension that's choosing to breakout into long URLs, it's
> some logic in Title::getFullURL(). A bug could be raised against core for that,
> perhaps.

Well in general, long urls are what we want in most cases. Query parameters on short urls are not garunteed to be supported on all configurations.

>Per Ori's comment and our statements on the Village Pump, I'm recommending we
>close this as WONTFIX...

Interesting definition of wontfix. My definition of WONTFIX means something is a bad idea and should never be fixed (and what you're suggesting would be more "priority lowest". However given this will be gone in two weeks, I could see why one would want to wontfix this bug).
Comment 11 Steven Walling 2012-08-16 23:39:13 UTC
Just an update: the first iteration of this experiment is over, so Ori has removed the &pe=1 from the query string for now. I am not closing the bug as RESOLVED because it may appear again as part of a second (and at this point in the plan, final) two week experiment.
Comment 12 Krinkle 2012-08-24 17:41:10 UTC
Why is it needed at all? What is it used for? Surely one can come up with a less visible / annoying solution to this problem?

Users shouldn't have to see this as visible like that. Especially since (as correctly pointed out in comment 0) urls should be canonical whenever possible.

This could probably be solved with a cookie, user session variable, url hash value. Or a state in the database, or calculating it dynamically (user.editcount > 0?) whatever the case. A url variable seems rather hacky for anything other than simulation (e.g. like CentralNotice's "template" query variable).
Comment 13 Jarry1250 2012-08-24 18:06:31 UTC
(In reply to comment #12)
> Or a state in the database, or calculating it dynamically
> (user.editcount > 0?) whatever the case. A url variable seems rather hacky for
> anything other than simulation (e.g. like CentralNotice's "template" query
> variable).

In fairness, the use of the URL clutter was to indicate that the page was the page displayed after an edit submit. It's not obvious how to reliably recreate that.
Comment 14 Steven Walling 2012-09-19 21:18:06 UTC
The previous experiment which added query strings to the URL is over, and the second iteration currently deployed does not change URLs.

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


Navigation
Links