Page MenuHomePhabricator

Echo should generate canonical diff links
Closed, ResolvedPublic

Description

Right now Echo generates non-canonical diff links for various notifications. For example, when you receive an email about your talk page being editing, the link is in the form:
https://en.wikipedia.org/w/index.php?title=User_talk:Kaldari&oldid=prev&diff=623938341

Ideally, it should be in the form:
https://en.wikipedia.org/wiki/User_talk:Kaldari?oldid=prev&diff=623938341

Otherwise, things like mobile redirects don't work properly.

See also

Details

Reference
bz70318

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 3:43 AM
bzimport added a project: Notifications.
bzimport set Reference to bz70318.
bzimport added a subscriber: Unknown Object (MLST).

This sounds like a bug in mobile, not in Echo. If you add a query string to a url, MediaWiki core converts it to use the script path directly instead of short url.

echo Title::newMainPage()->getFullUrl();

http://localhost/wiki/Main_Page

echo Title::newMainPage()->getFullUrl("action=edit");

http://localhost/w/index.php?title=Main_Page&action=edit

Legoktm: The mobile bug (Bug 70312) was closed as WONTFIX per MaxSem. I'll discuss the issue with him further on that bug.

Per @Legoktm, the normal (canonical) diff link does use index.php. Just go to https://en.wikipedia.org/w/index.php?title=Earth&action=history (on desktop) and click any prev link. An example is https://en.wikipedia.org/w/index.php?title=Earth&diff=637363072&oldid=637362826 .

If there's some interface we can use that generates the desired form on both desktop and mobile, that works. Otherwise, we'll probably decline this.

So far, the solution seems to be to create a Special:Diff in core and send pretty links to it, which will also give everyone the benefit of having the ability to use wikilinks to diffs.

Yeah, Special:Diff exists, though you can't visit it directly without parameters.

If the canonical diff link were to change, the new format should include the title as the current one does. Although it's not actually significant (the oldid and diff take precedence) it makes the URL far more readable.

@MaxSem you said you would reply with some steps forward :)

@MaxSem Jon says he's waiting for you to answer the question. ;-)

@Mattflaschen, do you think it would be a sane thing to do to send diff links à-la https://en.wikipedia.org/wiki/Article?diff=123 ?

Yeah, I think that's fine for a single-edit diff. For diffs that span multiple revisions (e.g. diff between "Article a month ago" and "Article today"), oldid is required.

One question is whether to include oldid in the URL for a single-edit diff. @Quiddity, do you recall any reason this is advisable?

Also, we should make sure diff link-generation doesn't break for wikis using other URL formats (e.g. I think the default does not actually use /wiki).

See https://www.mediawiki.org/wiki/Special:MyLanguage/Manual:$wgActionPaths , https://www.mediawiki.org/wiki/Special:MyLanguage/Manual:$wgUsePathInfo , https://www.mediawiki.org/wiki/Manual:Short_URL .

@Mattflaschen, do you think it would be a sane thing to do to send diff links à-la https://en.wikipedia.org/wiki/Article?diff=123 ?

Is there a reason why MediaWiki will switch to index.php when a parameter is added instead of just appending it to the url?

There was some creepy stuff about IE content detection...

How long is a plan off? When I click Flow notifications on mobile I get taken to the desktop experience which is very jarring :(

How long is a plan off? When I click Flow notifications on mobile I get taken to the desktop experience which is very jarring :(

It sounds like we need to partly fix T72312: mobile redirect only works on canonical URLs, at least for diff URLs and Flow URLs. Any pointers on how to do that? It's presumably the redirect code in MobileFrontend, so we don't have a lot of experience/expertise there.

I'm a bit hazy on all this but if I recall correctly the logic is actually a varnish config:
http://git.wikimedia.org/blob/operations%2Fpuppet.git/3e6275024bcd42d38ab9aa22a369760a56e50332/templates%2Fvarnish%2Fmobile-frontend.inc.vcl.erb#L37 is probably a good starting point.

I suspect this doesn't catch this url for whatever reason.
MobileFrontend to my knowledge doesn't do any domain mangling to the .m domain except with respect to a user switching between versions... but I may be wrong.
@MaxSem hopefully can confirm where the code can be found.

SBisson claimed this task.
SBisson subscribed.

It looks like redirects are working now and diff links from Echo are landing on Special:MobileDiff.

Please reopen if I'm missing something.