Page MenuHomePhabricator

Edit link is broken on file pages where magic commons is used
Closed, ResolvedPublic

Description

Looking at https://en.m.wikipedia.org/wiki/File:En-us-Barack-Hussein-Obama.ogg the edit link isn't provided enough horizontal space.

Developer notes

Can be easily replicated locally by visiting a file page in the mobile skin when $wgUseInstantCommons is triggered.

Works fine when the file is local e.g. https://en.m.wikipedia.beta.wmflabs.org/wiki/File:POC_phpinfo-metadata.gif
but not when magic e.g. https://en.m.wikipedia.beta.wmflabs.org/wiki/File:Banana.jpg

Per https://phabricator.wikimedia.org/T206546#4676306 the "magic" happens in FileRepo::getDescriptionRenderUrl()
action=render should not display section edit links.

Event Timeline

Volker_E created this task.Oct 9 2018, 4:12 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 9 2018, 4:12 PM
Jdlrobson added a subscriber: matmarex.

This looks like a regression. This used to be hidden on mobile. cc @matmarex as this might be related to your changes in Minerva.

File pages generate headings that don't go through Skin::doEditSectionLink

Jdlrobson renamed this task from Edit link misaligned on audio file to Regression: Edit link shown (and broken) on file pages.Oct 9 2018, 5:39 PM
Jdlrobson moved this task from Incoming to Needs Prioritization on the Readers-Web-Backlog board.

Related: T64071

My changes from https://gerrit.wikimedia.org/r/c/mediawiki/skins/MinervaNeue/+/461855/6/resources/skins.minerva.content.styles/hacks.less would have caused this, but they are not live in production yet, so it can't be caused by them. The .mw-editsection { display: none; } is being overridden by some very specific rule that sets display: table-cell;.

wowser. That's quite the CSS rule... Thanks for investigating!

It seems that the [edit] links shouldn't even be shown there. Following some links on T64071, I found T21415, which removed them from action=render pages. The code is still there, slightly changed (in Article::render()), but it does not work – on https://commons.wikimedia.org/wiki/File:En-us-Barack-Hussein-Obama.ogg?action=render the section edit links are plainly seen.

I suggest that you either investigate that, or just hide them with CSS again with a different selector. #shared-image-desc .mw-editsection { display: none; } would be more specific than the existing rule, and would not interfere with changed section edit links added in my patches.

matmarex removed a subscriber: matmarex.Oct 9 2018, 7:10 PM
Jdlrobson updated the task description. (Show Details)EditedOct 16 2018, 6:48 PM

This seems related to $wgUseInstantCommons.

https://commons.wikimedia.org/wiki/File:En-us-Barack-Hussein-Obama.ogg?action=render&useskin=minerva works fine, so I think this is just a case of wherever we're hitting commons passing useskin to the render function.

@matmarex any idea where that magic happens?

ovasileva triaged this task as Medium priority.Oct 16 2018, 9:17 PM

@Jdlrobson It happens in FileRepo::getDescriptionRenderUrl(), but I don't think it's feasible to add useskin= there. We'd have to pass it through a few levels of function calls, but more importantly, the foreign description is cached in File::getDescriptionText(), and that cache would have to be split by skin.

While playing with action=render, I just noticed that the section links are correctly not shown in https://en.wikipedia.org/wiki/The_Fighting_Temeraire?action=render. I thought it was Commons-specific, but then I noticed that they are incorrectly shown in https://en.wikipedia.org/wiki/File:Ustinov_is_Poirot.jpg?action=render. Looks like this issue is simpler than I though, it's just a bug with file pages…

matmarex claimed this task.Oct 18 2018, 3:03 AM

Change 468203 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] ImagePage: Inherit parent's handling for action=render

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

Change 468203 merged by jenkins-bot:
[mediawiki/core@master] ImagePage: Inherit parent's handling for action=render

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

Jdlrobson renamed this task from Regression: Edit link shown (and broken) on file pages to Edit link is broken on file pages where magic commons is used.Oct 18 2018, 5:38 PM
Jdlrobson updated the task description. (Show Details)

That is pulling the file and description from the production Commons (https://commons.wikimedia.org/wiki/File:Banana.jpg?action=render), which is not running the new code yet.

I tried to create a file that would be pulled from beta Commons: https://commons.wikimedia.beta.wmflabs.org/wiki/File:Test_2018-10-18.png. The action=render is correctly without section edit links: https://commons.wikimedia.beta.wmflabs.org/wiki/File:Test_2018-10-18.png?action=render and the image from beta Commons is shown correctly: https://en.wikipedia.beta.wmflabs.org/wiki/File:Test_2018-10-18.png but it has no description shown – apparently it is still trying to pull it from production Commons?

I think something is busted with the foreign file repo confing on beta (or possibly having multiple foreign file repos in general is busted). But I'm confident that it will work correctly in production, where we have a much simpler config (only one foreign repo).

Oohh thanks for that insight! Fingers crossed!
Why is beta cluster not pulling from commons beta cluster with magic commons?

I think I was wrong, it is pulling from the right repo, but failing to pull the description (I don't know why). I was confused because all of the on-wiki messages are customized to link to ommons.wikimedia.org even when the file is pulled from elsewhere :/ But with uselang=qqx, it is more clear:

https://en.wikipedia.beta.wmflabs.org/w/index.php?title=File:Banana.jpg&uselang=qqx(sharedupload-desc-here: (shared-repo-name-wikimediacommons), https://commons.wikimedia.org/wiki/File:Banana.jpg)
https://en.wikipedia.beta.wmflabs.org/wiki/File:Test_2018-10-18.png?uselang=qqx(sharedupload-desc-there: (shared-repo-name-shared), //commons.wikimedia.beta.wmflabs.org/wiki/File:Test_2018-10-18.png)

I think I see the issue: the beta Commons URL is protocol-relative. When asked to fetch it, MWHttpRequest expands it to a http:// URL, but we no longer serve the site over HTTP. There is a 'Location:' redirect, but MWHttpRequest doesn't follow redirects by default.

Change 468413 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[operations/mediawiki-config@master] Fix fetching file descriptions from beta Commons

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

Change 468413 merged by jenkins-bot:
[operations/mediawiki-config@master] Fix fetching file descriptions from beta Commons

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

I fixed the messages on beta English Wikipedia so that it's more obvious where it's fetching files from: https://en.wikipedia.beta.wmflabs.org/wiki/Special:Log?user=Yatu&wpdate=2018-10-18&limit=7

https://en.m.wikipedia.beta.wmflabs.org/wiki/File:Test_2018-10-18.png is now displaying the fetched file description, with no funky section edit links.

matmarex closed this task as Resolved.Oct 18 2018, 9:05 PM
matmarex updated the task description. (Show Details)