Page MenuHomePhabricator

[Technical debt pay off] Remove MFMobileMainPageCss from MobileFrontend
Closed, ResolvedPublic

Description

We are supporting a ResourceLoader module and associated PHP code for 2 pages on 2 wikis that we added temporarily for a Hindi Wikipedia main page video campaign back in 2018 when TemplateStyles wasn't deployed (T193292). Now TemplateStyles is deployed this code is no longer needed. We will drop the associated code prior to the next MediaWiki release.

Developer notes

https://gerrit.wikimedia.org/r/c/569175 is ready to go when we are.

Background (if interested)

wgMFMobileMainPageCss is being used by Hindi and Russian Wikipedia. It was originally added for a specific campaign in Hindu Wikipedia and was never meant to be permanent. It was later adopted in T195905.

This config flag adds an unnecessary ResourceLoader module that applies styles in MobileMainPage.css to the main page.

This expensive config variable seems to serve one purpose on Hindi Wikipedia to change the background color - however after checking in with @alexhollender the design works fine without it and we want to discourage changing the background color.

https://hi.m.wikipedia.org/wiki/%E0%A4%AE%E0%A5%80%E0%A4%A1%E0%A4%BF%E0%A4%AF%E0%A4%BE%E0%A4%B5%E0%A4%BF%E0%A4%95%E0%A4%BF:MobileMainPage.css

body.skin-minerva #content {
  background-color: #fff;
  background-image: linear-gradient(to top, #ffffff, #eaecf0);
}

However Russian Wikipedia also makes use of this config
and on Russian Wikipedia this CSS rule seems to be disabled at desktop resolutions and accompanied by a rule to disable box-shadow on the header:

/*
 * Важные стили для заглавной страницы в мобильной версии
 * См. [[phab:T195905]]
*/
@media (max-width: 719px) {
	.page-Main_Page #content {
		background-color: #eaecf0;
	}
	
	.page-Main_Page .header-container.header-chrome {
		box-shadow: none; 
	}
}

After talking to @alexhollender we will drop this module. 1 less ResourceLoader (1 small step for performance) and 1 less PHP class. Russian Wikipedia will need to make adjustments to their main page to account for the removal of this page.

Existing pages:

Acceptance criteria

  • Check in with @alexhollender about the appropriate behaviour here - box shadow or no box shadow
  • Move content specific Hindi styles to TemplateStyle applied to MainPage scoped to body.skin-minerva - I can do this using staff account if given permission.
  • Ensure Russian Wikipedia are aware of this change so they can make adjustments if they want to.
  • Drop code from MobileFrontend
  • Drop the config flag.

Event Timeline

Restricted Application added subscribers: Masumrezarock100, Aklapper. · View Herald TranscriptJan 31 2020, 8:53 AM

Change 569175 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Drop support for MediaWiki:MobileMainPage

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

Jdlrobson triaged this task as High priority.
Jdlrobson moved this task from Incoming to Needs Prioritization on the Readers-Web-Backlog board.
ovasileva lowered the priority of this task from High to Medium.Feb 17 2020, 2:01 PM

Check in with @alexhollender about the appropriate behaviour here - box shadow or no box shadow

^ needed from Alex. Apart from this purely technical

Jdlrobson updated the task description. (Show Details)Mar 6 2020, 10:42 PM
Jdlrobson moved this task from Needs analysis to Next up on the Readers-Web-Backlog (Design) board.

I've coordinated the major changes with Hindi Wikipedia. @alexhollender I'd love to chat with you next week at a convenient time to remove this before the April release.

Jdlrobson updated the task description. (Show Details)Mar 11 2020, 6:30 PM
Jdlrobson added a subscriber: stjn.Mar 11 2020, 6:32 PM

@stjn letting you know about this change per our conversation in T195905. This config variable was only ever meant to be temporary.

The Russian Wikipedia main page without the stylesheet looks mostly fine to me, although I suspect you'll need to add a more clearer box shadow around the search input. Let me know if I can help in any way.

stjn added a comment.Mar 12 2020, 10:45 AM

Can you consider making Mobile.css loaded normally, and not via JS, at least on main pages, where there’s not that much of a content? That’s the only reason MobileMainPage.css was used by us in the first place. I guess people will have to live with a repaint after this variable is removed, but I find the decision to make mobile CSS non-blocking really rather limiting to communities’ ability to use CSS.

@stjn that capability already exists (wgMFSiteStylesRenderBlocking) however I don't recommend it given the current state of MediaWiki:Mobile.css on Russian Wikipedia. The current mobile site loads 9kb of render blocking CSS on Russian Wikipedia and if that flag was enabled you'd be adding 2.52kb of render blocking CSS to all page views just to change the background image on the main page. Pages would take longer to render, which is particularly a problem on slower connections.

I think this option becomes more viable if the contents of MediaWiki:Mobile.less are reduced in size and put into appropriate TemplateStyles. For instance I see a lot of HList styles which would be better inside the corresponding template. I see lots of infobox specific styles.

Moving these styles out of Mobile.css and reserving styles here for UI interface elements would also be better for the project as a whole as it would avoid repaints/reflows when these styles are needed in the initial render and avoid loading them on pages where they are unused increasing the efficiency of pages. Russian Wikipedia would also be trailblazers if they were to do that.. I've been trying to encourage other projects to do that for some time :)

Jdlrobson renamed this task from Tech debt: Remove MFMobileMainPageCss from MobileFrontend to [Technical debt pay off] Remove MFMobileMainPageCss from MobileFrontend.EditedMar 12 2020, 8:12 PM

Olga similar to T240622 I'd like us to get this in for the next MediaWiki release by 7 April 2020. Note that's a hard deadline so we'll likely want to get this and that task estimated by 24th March at the absolute latest.

stjn added a comment.Mar 13 2020, 9:17 AM

I think this option becomes more viable if the contents of MediaWiki:Mobile.less are reduced in size and put into appropriate TemplateStyles. For instance I see a lot of HList styles which would be better inside the corresponding template. I see lots of infobox specific styles.

If we were open to do it, how does it fare with the real-life usage of hlist, for example. You can’t really know when it is used or not in infoboxes, for example, so in practicality you would have to add it to all 1.1 million articles using infoboxes, out of 1.6 million total. It is also used exclusively in navboxes, which doesn’t affect mobile, but it is still would be more than 700 hundred articles out of 1.6 million with inline styles (some without infobox). Is that an OK trade-off in regards to performance, 2 Kb of render-blocking CSS added almost always via HTML vs CSS request?

In all honesty, nearly all CSS there is already fairly critical (infoboxes/hlist/plainlist/reflist stuff) or can’t be done any other way (TemplateStyles restrictions), it just gets loaded with a visible delay to users.

Jdlrobson updated the task description. (Show Details)Mar 13 2020, 5:03 PM
Jdlrobson updated the task description. (Show Details)

I think this option becomes more viable if the contents of MediaWiki:Mobile.less are reduced in size and put into appropriate TemplateStyles. For instance I see a lot of HList styles which would be better inside the corresponding template. I see lots of infobox specific styles.

If we were open to do it, how does it fare with the real-life usage of hlist, for example. You can’t really know when it is used or not in infoboxes, for example, so in practicality you would have to add it to all 1.1 million articles using infoboxes, out of 1.6 million total. It is also used exclusively in navboxes, which doesn’t affect mobile, but it is still would be more than 700 hundred articles out of 1.6 million with inline styles (some without infobox). Is that an OK trade-off in regards to performance, 2 Kb of render-blocking CSS added almost always via HTML vs CSS request?

In all honesty, nearly all CSS there is already fairly critical (infoboxes/hlist/plainlist/reflist stuff) or can’t be done any other way (TemplateStyles restrictions), it just gets loaded with a visible delay to users.

Yeh maybe Hlist wasn't the best choice here. I guess the hlist class is used minus the template in a lot of places. I realise this curtails a lot of work. It would be helpful to me to know which CSS is trying to overcome restricts of TemplateStyles as I can help champion fixing those issues. Either way, that option remains in future if you need it.

As for the main page one way you could get the same existing effect is to use negative margins and put the background on main-top with negative margins:

.main-top {
    background-color: #eaecf0;
    margin-left: -16px;
    margin-right: -16px;
    overflow: hidden;
    padding: 0 8px;
        padding-bottom: 0px;
    margin-top: 0 !important;
}
stjn added a comment.Mar 13 2020, 7:40 PM

Yeh maybe Hlist wasn't the best choice here. I guess the hlist class is used minus the template in a lot of places. I realise this curtails a lot of work. It would be helpful to me to know which CSS is trying to overcome restricts of TemplateStyles as I can help champion fixing those issues. Either way, that option remains in future if you need it.

There can be, theoretically, a solution where you could put hlist/plainlist styling everywhere where it can be potentially used, but we in our community obviously can’t tell which one of the pathways is the more beneficial one, one where you increase CSS load by 2 Kb for those styles or where you increase the size of every HTML document by the same amount. I wonder whether you have some insight on this.

Sorry for not writing that out, I was talking about T162379: Decide which non-standard CSS properties to support in TemplateStyles in regards to TS, which is quite a big issue to overcome, tbh :-) But it would cut quite a lot of code there, both from Mobile.css and Common.css.

The option to have a wrapper block around the main page code that would add that background in Minerva is a possible workaround, but there would still be a noticeable white gap in the main page, so I guess a repaint is good enough.

pathways is the more beneficial one, one where you increase CSS load by 2 Kb for those styles or where you increase the size of every HTML document by the same amount. I wonder whether you have some insight on this.

My guess based on similar accidental spikes in CSS is that such an amount of kb would add up to 0.5s to first paint on a 3G/2G connection at most. Our Performance-Team could add metrics for us to monitor it and I think that would be a good idea to do prior to the change.

There's definitely some benefits of enabling it, but the danger however is if it's allowed to grow.

MediaWiki:Common.css currently clocks in at 6.52kb (23.31kb without gzip) - which would almost double the payload for mobile and most of mobile's rules are copied and pasted directly from Common.css already. Right now the fact Mobile.css is not render blocking may be helping add an incentive to use other alternatives where available rather than let it be a dumping ground. Now doubling CSS is likely to lead to bigger delays (think full seconds) in a user reading than 0.5s. However this is purely hypothetical at this point.

In summary I think it's okay to make this switch if you think the main page needs it but would recommend you only do it if you are willing to monitor performance along with changes and setup some guidelines for editors around responsibly using template styles where ever possible.

Let me know what makes sense for you. I am happy to help with performance metrics being setup if you decide to test out this path.

Change 569175 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Drop support for MediaWiki:MobileMainPage

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

Jdlrobson updated the task description. (Show Details)
Jdlrobson added a subscriber: polishdeveloper.

I'll take care of the config flag on Monday.
No need to track this any more. Thanks for the review @polishdeveloper

Change 587800 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[operations/mediawiki-config@master] Drop unused config for main page css

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

Change 587800 merged by jenkins-bot:
[operations/mediawiki-config@master] Drop unused config for main page css

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

Mentioned in SAL (#wikimedia-operations) [2020-04-09T23:21:55Z] <catrope@deploy1001> Synchronized wmf-config/InitialiseSettings.php: Drop unused config for main page CSS (T243996) (duration: 00m 58s)

Jdlrobson closed this task as Resolved.Apr 9 2020, 11:26 PM
Jdlrobson updated the task description. (Show Details)