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).
kaldari created this task.Sep 3 2014, 1:05 AM

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.

matthiasmullie triaged this task as Normal priority.Dec 10 2014, 5:18 PM
matthiasmullie set Security to None.
kaldari claimed this task.Dec 10 2014, 9:18 PM

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.

MaxSem added a subscriber: MaxSem.Dec 15 2014, 6:22 PM

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.

We already have that.

Mattflaschen-WMF added a comment.EditedDec 15 2014, 7:43 PM

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 .

Deskana removed a subscriber: Deskana.Dec 19 2014, 10:00 PM

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

Legoktm moved this task from Backlog to Needs plan on the Notifications board.Jul 6 2015, 8:13 AM
He7d3r updated the task description. (Show Details)Jul 6 2015, 12:35 PM

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

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 27 2015, 11:29 PM

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.

DannyH removed a subscriber: DannyH.Oct 5 2015, 10:57 PM
kaldari removed kaldari as the assignee of this task.Feb 9 2016, 12:29 AM
Restricted Application added a project: Growth-Team. · View Herald TranscriptSep 4 2018, 2:45 PM
SBisson moved this task from Inbox to Triaged but Future on the Growth-Team board.Sep 7 2018, 3:03 PM
SBisson closed this task as Resolved.Apr 19 2019, 6:44 PM
SBisson claimed this task.
SBisson added a subscriber: SBisson.

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

Please reopen if I'm missing something.