Garbage in URL after saving an edit on the English Wikipedia
Closed, ResolvedPublic

Description

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.


Version: unspecified
Severity: normal

bzimport added a subscriber: Unknown Object (MLST).
bzimport set Reference to bz38798.
MZMcBride created this task.Via LegacyJul 29 2012, 6:40 PM
Bawolff added a comment.Via ConduitJul 29 2012, 6:51 PM

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.

ori added a comment.Via ConduitJul 29 2012, 7:35 PM

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
Bawolff added a comment.Via ConduitJul 29 2012, 8:22 PM

"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.

bzimport added a comment.Via ConduitJul 29 2012, 9:08 PM

swalling wrote:

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.

Jarry1250 added a comment.Via ConduitJul 29 2012, 9:47 PM

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.

MZMcBride added a comment.Via ConduitJul 29 2012, 11:48 PM

(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.

MarkTraceur added a comment.Via ConduitJul 30 2012, 12:43 AM

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.

MarkTraceur added a comment.Via ConduitJul 30 2012, 2:01 AM

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.

MZMcBride added a comment.Via ConduitJul 30 2012, 2:02 AM

(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.

Bawolff added a comment.Via ConduitJul 30 2012, 12:13 PM

(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).

bzimport added a comment.Via ConduitAug 16 2012, 11:39 PM

swalling wrote:

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.

Krinkle added a comment.Via ConduitAug 24 2012, 5:41 PM

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).

Jarry1250 added a comment.Via ConduitAug 24 2012, 6:06 PM

(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.

bzimport added a comment.Via ConduitSep 19 2012, 9:18 PM

swalling wrote:

The previous experiment which added query strings to the URL is over, and the second iteration currently deployed does not change URLs.

Add Comment