Regression: Some MobileFrontend styles are not applied on mobile view of GlobalUserPages
Closed, ResolvedPublic3 Estimated Story Points
Actions

Assigned To

Authored By

	Jdlrobson
	Feb 13 2018, 5:24 PM

Description

Visit https://m.mediawiki.org/wiki/User:CKoerner_(WMF)

Screen Shot 2018-02-13 at 9.23.22 AM.png (566×1 px, 206 KB)

Cannot replicate this on Jon's: Visit https://m.mediawiki.org/wiki/User:Jdlrobson

How it should look:

Screen Shot 2018-02-13 at 9.40.30 AM.png (363×986 px, 48 KB)

The problem seems to be that the following styles are not loaded:

..mw-ui-icon-mf-talk:before, mw-ui-icon-talk:before {}

.user-links li {
    display: inline-block;
}

mobile.userpage.icons and mobile.userpage.icons.styles appear to not be loaded on Chris's page.

Developer notes

The issue effects Global user pages only.
The problem is that ExtMobileFrontend ::domParse is called inside OutputPageBeforeHTML - a hook which is not invoked for global user pages.

There seems to be confusion about responsibility between Minerva and MobileFrontend here.
While, a user page that does not exist works is transformed for Mobile Minerva and[[ https://m.mediawiki.org/w/index.php?title=User:Rickathesperian&useskin=vector | Mobile Vector ]]

The header with the talk page icon and menu bar belong to Minerva:
(note Mobile Vector does not show this header)

Thus, the talk page icon and the rules relating to .user-links that currently live in the module mobile.userpage.styles should in fact belong to Minerva and the talk icon belonging to mobile.userpage.icons should actually live in a module that lives in Minerva.

This was broken in T182162

Fix

Restore modules skins.minerva.userpage.styles and skins.minerva.userpage.icons with the styles that should be shipped. This is done in https://gerrit.wikimedia.org/r/410364
Remove the talk page icon from the definition inside mobile.userpage.icons
Remove styles relating to user-links from the mobile.userpage.styles module.

QA steps

I've setup GlobalUserPages on reading web staging.

Visit http://reading-web-staging.wmflabs.org/w/index.php?title=User:CKoerner_(WMF)&mobileaction=toggle_view_mobile

The navigation bar at the top of the page should appear horizontally

Check empty message shows on http://reading-web-staging.wmflabs.org/w/index.php?title=User:Jdlrobson

Check empty message + icon shows on http://reading-web-staging.wmflabs.org/w/index.php?title=User:Jdlrobson&useskin=vector

Details

Subject	Repo	Branch	Lines +/-
Remove unused userpage icon from repo	mediawiki/skins/MinervaNeue	master	+0 -1
Don't ship unnecessary styles to mobile userpages that are not Minerva	mediawiki/extensions/MobileFrontend	master	+0 -25
Restore Minerva specific userpage styles for all user pages	mediawiki/skins/MinervaNeue	master	+4 -42

Customize query in gerrit

Related Objects

Mentioned Here: T182162: Prefix all icons with `mf-` and fix the download icon glyph change glitch

Event Timeline

Jdlrobson created this task.Feb 13 2018, 5:24 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 13 2018, 5:24 PM

ovasileva triaged this task as High priority.Feb 13 2018, 5:29 PM

Jdlrobson updated the task description. (Show Details)Feb 13 2018, 5:40 PM

Jdlrobson updated the task description. (Show Details)Feb 13 2018, 5:51 PM

As the estimates for this are currently quite high and we expect the issues to disappear when the cache is purged, we're dropping the priority on this for the moment.

Looks like this might be specific to GlobalUserPage's. I'm not sure how the cache strategy works there.

Jdlrobson moved this task from Incoming to 2017-18 Q3 on the Web-Team-Backlog board.Feb 13 2018, 5:54 PM

ovasileva removed a project: Readers-Web-Kanbanana-Board-Old.Feb 13 2018, 6:15 PM

ovasileva moved this task from 2017-18 Q3 to Needs Prioritization on the Web-Team-Backlog board.

/me slowly backs away from Phabricator... 👀

What caching do you think is the problem here?

Legoktm renamed this task from Regression: Caching issue on Chris's user page (and probably others) to Regression: Some MobileFrontend styles are not applied on mobile view of GlobalUserPages.Feb 13 2018, 8:52 PM

Legoktm added a project: MobileFrontend.

This is the normal cached HTML/new CSS problem that we're hit with all the time as skin developers.

Looks like https://m.mediawiki.org/wiki/User:CKoerner_(WMF) is requesting the skins.minerva.icons.images module via the style tag in the HTML (https://m.mediawiki.org/w/load.php?debug=false&lang=en&modules=mediawiki.hlist%7Cmediawiki.ui.button%2Cicon%7Cskins.minerva.base.reset%2Cstyles%7Cskins.minerva.content.styles%7Cskins.minerva.icons.images%7Cskins.minerva.tablet.styles&only=styles&skin=minerva)
This module was replaced another module.

Problem is fixed trivially by the following JS:

mw.loader.using(['mobile.userpage.icons','mobile.userpage.styles'])

or flushing the Varnish cache for that page. It seems applying ?action=purge on this page or logging in does not serve the most recent HTML. I thought that should fix it, but it doesn't, which is strange. Given we've only seen this on a GlobalUserPage it would be worth understanding how the cache gets invalidated there as this appears to work differently from other user pages.

It's not a varnish cache issue, because https://m.mediawiki.org/wiki/User:CKoerner_(WMF)?1 exhibits the same behavior.

I purged the meta userpage that is the source of the globaluserpage, but that didn't change anything either...

According to https://phabricator.wikimedia.org/diffusion/SMIN/browse/master/includes/skins/SkinMinerva.php$1451 that module is still supposed to be added on all pages?

The only code path I see that loads mobile.userpage.icons is https://phabricator.wikimedia.org/diffusion/EMFR/browse/master/includes/MobileFrontend.body.php$75 - is that supposed to be the only place?

My bad skins.minerva.icons.images is something else.
So the problem here is mobile.userpage.icons is not being added to the global user page . Sounds like we can rule out caching being a problem.

The domParse function is invoked by hook onOutputPageBeforeHTML (https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/blob/304c0b0923a263386c54f2a484b8a10a26cb094c/includes/MobileFrontend.hooks.php#L230) which modifies the html passed in by reference.

Is it possible the global user page somehow bypasses that?

(Side note and off topic the side effects of that domParse are extremely yucky)

In T187222#3969314, @Jdlrobson wrote:

My bad skins.minerva.icons.images is something else.
So the problem here is mobile.userpage.icons is not being added to the global user page . Sounds like we can rule out caching being a problem.

The domParse function is invoked by hook onOutputPageBeforeHTML (https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/blob/304c0b0923a263386c54f2a484b8a10a26cb094c/includes/MobileFrontend.hooks.php#L230) which modifies the html passed in by reference.

Is it possible the global user page somehow bypasses that?

Yep, that hook is only called in OutputPage::addParserOutputText(), which GlobalUserPage skips.

Change 410364 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Restore Minerva specific userpage styles for all user pages

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

gerritbot added a project: Patch-For-Review.Feb 14 2018, 12:15 AM

Ahh. I'm removing the estimation as given this new information, the scope of the bug has changed and it doesn't look like it will magically fix itself. Have updated instructions about how to fix this. Thanks @Legoktm for helping me debug this.

Jdlrobson awarded a token.Feb 14 2018, 12:16 AM

Jdlrobson added a project: Readers-Web-Kanbanana-Board-Old.

Jdlrobson moved this task from Needs Prioritization to Upcoming on the Web-Team-Backlog board.

Jdlrobson moved this task from Upcoming to 2017-18 Q3 on the Web-Team-Backlog board.Feb 14 2018, 5:09 PM

ovasileva set the point value for this task to 3.Feb 14 2018, 5:09 PM

Change 410364 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Restore Minerva specific userpage styles for all user pages

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

Change 410564 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Don't ship unnecessary styles to mobile userpages that are not Minerva

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

Jdlrobson moved this task from To Do to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.Feb 14 2018, 9:13 PM

Jdlrobson updated the task description. (Show Details)

ReleaseTaggerBot added a project: MW-1.31-release-notes (WMF-deploy-2018-02-20 (1.31.0-wmf.22)).Feb 14 2018, 10:01 PM

Change 411014 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Remove unused userpage icon from repo

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

Change 411014 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Remove unused userpage icon from repo

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

Change 410564 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Don't ship unnecessary styles to mobile userpages that are not Minerva

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

This is going to be difficult to test but let me see if I can work something out. ..

Over you Anthony. This can be tested now!

Looks ok to me. Here are some screenshots

phuedx reassigned this task from ABorbaWMF to ovasileva.Feb 16 2018, 10:10 AM

phuedx added a subscriber: ABorbaWMF.

looks like we're all done here!

	F13783245: image.png
	Feb 16 2018, 1:11 AM

	F13783276: image.png
	Feb 16 2018, 1:11 AM

	F13783236: image.png
	Feb 16 2018, 1:11 AM

	F13783256: image.png
	Feb 16 2018, 1:11 AM

Regression: Some MobileFrontend styles are not applied on mobile view of GlobalUserPagesClosed, ResolvedPublic3 Estimated Story PointsActions