Page MenuHomePhabricator

Skin stylesheet no longer unaffected by broken Common.css as of 1.28.0-wmf.7
Closed, ResolvedPublic

Description

We had several reports of CSS being broken. Apparently happened over european night. Examples:

T138578: [Regression] en.wikipedia.org Mobile main page no longer being transformed
T138537: checkuserwiki doesn't have any skin
T138579: nl.wikisource CSS loading is broken

For the three first examples, appending ?debug=true to by pass the CSS/JS cache fix the rendering.

Best candidate is the RL site.styles which, when included, breaks the whole layout/stylesheet.

Details

Related Gerrit Patches:

Event Timeline

Mentioned in SAL [2016-06-24T12:34:50Z] <hashar> Random resource loader entries are apparently faulty causing issues with css and/or javascript T138586

matmarex updated the task description. (Show Details)Jun 24 2016, 12:37 PM
matmarex added a subscriber: matmarex.

The UploadWizard thing is not the same issue, it's about bad/weird responses being returned by the API.

An history of events for resource loader in our logs aggregator ( https://logstash.wikimedia.org/#/dashboard/elasticsearch/resourceloader , private )

Times are UTC. Notice the spike roughly after 3am which would match the following events:

02:38	<mwdeploy@tin>	scap sync-l10n completed (1.28.0-wmf.6) (duration: 17m 08s)
03:12	<mwdeploy@tin>	scap sync-l10n completed (1.28.0-wmf.7) (duration: 17m 24s)
03:19	<l10nupdate@tin>	ResourceLoader cache refresh completed at Fri Jun 24 03:19:55 UTC 2016 (duration 7m 4s)

Zooming on it shows the first few events started after the sync-l10n for 1.28.0-wmf.7 .

The logstash events are all Failed to find {messageKey} ({lang}) apparently referring to /w/load.php?debug=false&lang=ps&modules=startup&only=scripts&skin=vector

With a few having target=mobile and skin=minerva: /w/load.php?debug=false&lang=en&modules=startup&only=scripts&skin=minerva&target=mobile (76)

hashar added a comment.EditedJun 24 2016, 12:54 PM
This comment has been deleted.

Looked at checkuser.wikimedia.org. There is a stylesheet pointing to:

/w/load.php?debug=false&lang=en&modules=ext.tmh.thumbnail.styles%7Cext.uls.nojs%7Cext.visualEditor.desktopArticleTarget.noscript%7Cmediawiki.legacy.commonPrint%2Cshared%7Cmediawiki.raggett%2CsectionAnchor%7Cmediawiki.skinning.interface%7Csite.styles%7Cskins.vector.styles&only=styles&skin=vector

I have changed the URL to solely load the skins.vector.styles RL module and that fix the page layout:

href="/w/load.php?debug=false&lang=en&modules=skins.vector.styles&only=styles&skin=vector"
hashar added a subscriber: Anomie.Jun 24 2016, 1:47 PM

So for checkuser.wikimedia.org the stylesheet url has:

/w/load.php?debug=false&lang=en
&modules=
 ext.tmh.thumbnail.styles| 
 ext.uls.nojs| 
 ext.visualEditor.desktopArticleTarget.noscript| 
 mediawiki.legacy.commonPrint,shared| 
 mediawiki.raggett,sectionAnchor| 
 mediawiki.skinning.interface| 
 site.styles| 
 skins.vector.styles
&only=styles&skin=vector

If I drop the site.styles module it works just fine.

Quoting @Anomie :

It's looking to me like either (a) RL somehow used to fix CSS errors in stuff like Common.css, but isn't doing that now, or (b) RL started doing something different that's making browsers recover differently from syntax errors in code included from Common.css.
For example, https://nl.wikisource.org/wiki/MediaWiki:Common.css has what's supposed to be "/* </pre>\nblah blah wikitext\n<pre> */", except some of them seem to be missing the "/*" and that's making the browser lose some of the styles in the vicinity..
When I look at checkuserwiki's MediaWiki:Common.css using eval.php, there's a missing "}" in one place.

hashar renamed this task from Resource loader stall cache / breakage to Resource loader as of 1.28.0-wmf.7 no more auto-correct common.css errors breaking several sites layouts.Jun 24 2016, 1:49 PM
hashar raised the priority of this task from Medium to High.
hashar updated the task description. (Show Details)

On the enwiki the site module is loaded independently:

<link rel="stylesheet" href="/w/load.php?debug=false&amp;lang=en&amp;modules=site&amp;only=styles&amp;skin=vector"/>

Whereas on checkuser wiki site.styles is multiplexed with the rest.

Krinkle renamed this task from Resource loader as of 1.28.0-wmf.7 no more auto-correct common.css errors breaking several sites layouts to Skin stylesheet no longer unaffected by broken Common.css as of 1.28.0-wmf.7.Jun 24 2016, 1:54 PM
Krinkle updated the task description. (Show Details)

Change 295914 had a related patch set uploaded (by Krinkle):
Restore load position override for 'site.styles' module

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

Change 295915 had a related patch set uploaded (by Krinkle):
Restore load position override for 'site.styles' module

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

Krinkle claimed this task.Jun 24 2016, 1:54 PM
Krinkle raised the priority of this task from High to Unbreak Now!.
Restricted Application added subscribers: Luke081515, TerraCodes, Urbanecm. · View Herald TranscriptJun 24 2016, 1:54 PM

Potential candidate is rMW93ed259cf3e5: resourceloader: Create 'site.styles' module for T92459 which introduces the sites.styles module which ends up multiplexed with the rest.

Mentioned in SAL [2016-06-24T14:05:42Z] <krinkle@tin> Synchronized php-1.28.0-wmf.7/includes/OutputPage.php: T138586 hotfix (duration: 00m 47s)

Around 14:00 UTC
<hashar> * 93ed259 - resourceloader: Create 'site.styles' module (Wed Jun 15 23:06:50 2016 -0700) <Timo Tijhof>
<anomie> Yeah. Tested on mw1017 by reverting just https://gerrit.wikimedia.org/r/#/c/292972/5/includes/OutputPage.php and suddenly styles work again.

<dr0ptp4kt>	Krinkle: any adverse impacts if touching https://github.com/wikimedia/mediawiki/commit/93ed259cf3e52a4f5e192c233b2df56d88a0e14c ?
<Krinkle>	dr0ptp4kt: hashar: It's being fixed now.

Change 295915 merged by jenkins-bot:
Restore load position override for 'site.styles' module

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

Change 295914 merged by jenkins-bot:
Restore load position override for 'site.styles' module

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