Page MenuHomePhabricator

Make MediaWiki:Mobile.css render-blocking
Open, HighPublic

Description

MediaWiki:Common.css is already render-blocking, so this would make things more consistent too. This requires splitting the CSS off into its own module (mobile.site.styles), because only pure-CSS modules can be made render-blocking.

While MediaWiki:Mobile.css is loaded via JS it will also fail to load when JS is disabled.
(see trade offs and long term plan in comments)

Event Timeline

Catrope triaged this task as High priority.Mar 19 2018, 7:18 PM
Catrope created this task.
Restricted Application added a project: Performance-Team. · View Herald TranscriptMar 19 2018, 7:18 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change 420405 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/MobileFrontend@master] Load MediaWiki:Mobile.css as a render-blocking style

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

This exists somewhere and was declined. I'll try and pull out the task and reasoning...

Here's the relevant quote https://phabricator.wikimedia.org/T126137#2186236

When we last experimented with top loading this stylesheet first paint was delayed by 2s (I think on 2g) but I remember there was also a delay for 3g too due to the additional stylesheet url. This may be less now but I still think this is unacceptable.

The strategy so far has been to put layout styles in a hacks module. I'm happy to shift more rules over there or explore ways to group the stylesheet in the existing skin styles url if that's an option.

To build on that, the plan with TemplateStyles on mobile was to move these styles into skins/templates where they made sense and remove the need for it altogether.

Provided we find that the first paint change with this change is insignificant I'm happy to go ahead with it, but I worry about the long term impacts - I'm constantly finding myself removing unnecessary css rules which either have no impact on mobile or are unnecessary and that have seemingly been copy/pasted from elsewhere e.g. 1 2 and 3.

Krinkle moved this task from Inbox to Radar on the Performance-Team board.Mar 19 2018, 8:02 PM
Krinkle edited projects, added Performance-Team (Radar); removed Performance-Team.

I agree the extra request is something to be aware of, but should be minimal in terms of overhead. If a previous attempt caused a 2s delay on 3G, Performance Team can help evaluate alternative ways, because that should not be the case. From a developer perspective, the extra request may stand out visually when looking at the HTML, but I would estimate it as low-impact for the critical path.

But even with overhead... I don't like isolation/duplication as a way to avoid solving technical debt. It worked well for the MVP, but I do not believe this would realistically scale in the long-term to all of Wikimedia.

Collaboration is key, and it'll enable a long-term result that is a maintainable, user-friendly, and (eventually) performant. The current situation is broken, so current performance isn't really what we should compare. I'm sure we can find a way to make the extra request work and not regress 2s. It may take some effort.

On the other hand, I do not oppose continuing the direction of maintaining hacks.less if you prefer that, but then we should be consistent and remove Mobile.css support from MobileFrontend. Leaving it as-is was a waste of effort for the communities whom took the time to migrate to Mobile.css from Common.css, which we also disabled.

Through Tech-News we can announce its removal, and inform users they should request CSS changes for mobile via Gerrit/Phabricator in MobileFrontend/hacks.less or, where possible, through TemplateStyles.

To capture conversation with Roan

  • it seems the additional HTTP request might not be happening any more so that's good.
  • Current mobile CSS is 8.86kb gzipped for the Barack Obama article. Mobile.css is small right now so on short term we wouldn't expect a big impact, but it would be prudent to run a webpagetest job on a set of wikis to evaluate the short term impact of rolling this out rather than be surprised.
  • Long term plan is that TemplateStyles will make this wikipage redundant.
  • Generally the MediaWiki:Mobile.css is small but historically it can be large. Most wikis appear to be less than 1000 word length (Roan checked the db). Some of the MediaWiki:Mobile.css files on certain wikis however are quite large (e.g. https://de.m.wikivoyage.org/wiki/MediaWiki:Mobile.css) which seem to be a result of cargo-cult programming. It would be useful to be able to have some systems in place that alert us to large site stylesheets on mobile and possibly restrict/warn editors when they go over a certain threshold. Historically I've spent a lot of time grokking Enwiki's MediaWiki:Mobile.css and keeping it sane.

On the other hand, I do not oppose continuing the direction of maintaining hacks.less if you prefer that, but then we should be consistent and remove Mobile.css support from MobileFrontend

This is fair feedback. I'm okay with this, or enabling it providing we can limit its size+verify it doesn't introduce any performance regressions.

Change 420405 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Load MediaWiki:Mobile.css as a render-blocking style

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

Jdlrobson added a subscriber: Peter.

This is now merged with a feature flag. We can enable the feature flag to make them render blocking at will.

Seems a few things to decide before that

  1. @Peter with a feature flag is it possible to run a few webpage jobs against production pages with the flag enabled to see how it impacts performance
  2. Should we remove Mobile.css support in favor of hacks approach (and wait for TemplateStyles)
  3. Should we add a hard limit to how much CSS can go into this wiki page (inside MobileFrontend?)
Nirmos added a subscriber: Nirmos.May 30 2018, 9:46 PM