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

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?)
Krinkle reopened this task as Open.EditedJun 22 2020, 12:11 AM

Re-opening as it remains a performance issue and anti-pattern to load stylesheets that reflow the page asynchronously. Task T248416 does not yet appear to be near enough a resolution that I'm comfortable assuming this is as good as solved in the current quarter.

If we believe most pages are no longer affected and there's almost nothing left, then I suppose we can either help the community migrate the last bits, or just let it load alongside the main stylesheet request if there's not much left. It would cause fewer reflows, provide a sooner load event end, and even shave off a few bytes in the form of less wrapping boilerplate and more optimum compression.

Removing task assignee due to inactivity, as this open task has been assigned to the same person for more than two years (see the emails sent to the task assignee on Oct27 and Nov23). Please assign this task to yourself again if you still realistically [plan to] work on this task - it would be welcome.
(See https://www.mediawiki.org/wiki/Bug_management/Assignee_cleanup for tips how to best manage your individual work in Phabricator.)

I'm not entirely sure if I follow completely what the issue nor status-quo is for this issue, but from what I understand and am able to deduce, MediaWiki:Mobile.css is currently being loaded through JavaScript, instead of being loaded the sensible way through some system analogous to the main skin CSS:

<link rel="stylesheet" href="/w/load.php?debug=1&amp;lang=en&amp;modules=site.styles&amp;only=styles&amp;skin=vector"/>
<link rel="stylesheet" href="/w/load.php?debug=1&amp;lang=en&amp;modules=user.styles&amp;only=styles&amp;skin=vector&amp;user=Joeytje50&amp;version=6vpqq"/>

I have tried reading the comments made on this issue and I just don't really understand the issue with this. From what I gather, it improves load times, but that argument makes no sense to me. It sounds to me like sending an empty plate to your customers in the restaurant under the guise of "faster serve times", even though the actual food will only be dropped onto that plate moments later when it's actually finished. You're just serving a raw shell of what should soon become a readable page at that point.

What I especially don't understand is how nobody seems to have suggested to simply preload the mobile css at the top and then actually include it in the location it should actually be, based on the order of style precedence (which is right above the user.styles, but below everything else). This preload will ensure the download of these massive Mobile.css files will start right away, but the functionality of having near-top importance of the site styles over all of the default styles is not lost. And more importantly, loading the base styles is not delayed until potentially after the page has rendered.

I'm not sure if this issue is still being considered, being worked on, or anything of the sort, but I think it'd be by far the best solution to simply include the style in its normal location, and instead of tampering with where to include the style, simply add a preload at the top to ensure the browser starts downloading the file right away.

Example of how it would work:

<link rel="preload" href="Mobile.css" as="style" />
<!-- all other stylesheets that need to have less priority than Common.css -->
<link rel="stylesheet" href="Mobile.css" />

(or probably actually mobile.site or whatever module would need to be loaded, but this is just to give an example).