Page MenuHomePhabricator

Projects with FlaggedRevisions installed load the entire Codex library and OOUI and 10kb more CSS, over 500kb of JavaScript and likely have higher first paint
Open, Stalled, HighPublicBUG REPORT

Description

NOTE: This was flagged by new tests the web team is adding.

Steps to replicate the issue (include links if applicable):

What happens?:
The page requires an additional 10kb of render blocking CSS.
The entire Codex library appears to be loaded.
German Wikipedia in Vector 2022 loads 37.1kb compared with the 19.3kb of CSS loaded on English Wikipedia (note German Wikipedia also has significantly more site styles)

What should have happened instead?:

It appears FlaggedRevisions loads the codex-styles module. In Vector and Minerva we intentionally load the more limited codex-search-styles module so this means Codex's styles are being loaded twice on the same page and lots of unused CSS.

I suggest FlaggedRevisions loaded codex-search-styles for consistency with Minerva and Vector until the Design system team have implemented T335317.

Software version (skip for WMF-hosted wikis like Wikipedia):

Other information (browser name/version, screenshots, etc.):

Related Objects

Event Timeline

Restricted Application added subscribers: Masumrezarock100, Aklapper. · View Herald Transcript
Jdlrobson updated the task description. (Show Details)
Jdlrobson added a subscriber: ovasileva.

@ovasileva don't know too much about FlaggedRevisions but this might be a blocker for rolling out on German Wikipedia as I suspect the new skin will not perform as well as Vector legacy.

@ovasileva don't know too much about FlaggedRevisions but this might be a blocker for rolling out on German Wikipedia as I suspect the new skin will not perform as well as Vector legacy.

The diff and codex are loaded in all skins so it shouldn't matter.

It will be fixed in itself as codex tree shaking hopefully will go live soon.

Codex is out design system and will slowly become ubiquitous meaning it will be needed in more places and also it'll be cached more often in the client side so it shouldn't be a big issue.

The diff and codex are loaded in all skins so it shouldn't matter.
It will be fixed in itself as codex tree shaking hopefully will go live soon.
Codex is out design system and will slowly become ubiquitous meaning it will be needed in more places and also it'll be cached more often in the client side so it shouldn't be a big issue.

Can you elaborate on this? For article pages, a subset of Codex's styles are loaded on Minerva and Vector 2022. The entire Codex library is loaded when an interaction occurs e.g. search. This was a strong requirement from the old performance team and T248718 was requested a long time to provide clarity around the why (as efforts to adopt Codex in certain extensions were blocked).

I am too eagerly anticipating code splitting, but I am not expecting the result of that to be able to load all of Codex on the page.

The deployment to German Wikipedia is currently scheduled for January, at which point I am not sure Codex code splitting will be complete, and this would be considered a blocker. I can bring this up in the next design systems / web team sync and work out a proposal here.

Specifically what Codex components does FlaggedRevs use? This would be very helpful to know going into that conversation.

The diff and codex are loaded in all skins so it shouldn't matter.
It will be fixed in itself as codex tree shaking hopefully will go live soon.
Codex is out design system and will slowly become ubiquitous meaning it will be needed in more places and also it'll be cached more often in the client side so it shouldn't be a big issue.

Can you elaborate on this? For article pages, a subset of Codex's styles are loaded on Minerva and Vector 2022. The entire Codex library is loaded when an interaction occurs e.g. search. This was a strong requirement from the old performance team and T248718 was requested a long time to provide clarity around the why (as efforts to adopt Codex in certain extensions were blocked).

That made sense back then, we might slowly want to revisit that. Let me explain in more depth: This is about cache fragmentation. You're here seeing a page view in isolation, that's rarely the case. A person usually doesn't read wikipedia once every other week so for most of the time, if codex loaded, it won't be fetched. Having one large blob being loaded and cached once is better than having a medium size bundle for search, another medium sized for flaggedrevs, another medium sized for another feature, etc. i.e. if codex is our design system and gets properly adopted, it'll be ubiquitous and thus shared between many functionalities. Or to put it simply, if codex is going to be the look of wikimedia, it should be loaded everywhere.

Of course, where to draw the line between how fragmented the cache should be and how large it's going to, is not an easy question to answer, people might draw the line differently and it heavily depends on the adoption, usage pattern, cache TTL and many more variables. For example, for dewiki the users on average have a much better connection than wikimedia's general readers, so the difference doesn't matter much so I think it's better to move towards "one large cache" instead of fragmented caches but that's just my opinion.

Of course, we could go with "base codex" and "full codex" and load base codex everywhere and full codex on user interaction but that's waiting on codex team work and it's not really what we should change in FR.

My point is that it's not as simple it looks. Hope I explained it better this time.

FWIW I agree with you that we should load Codex on page load, but as I mentioned before web team were specifically asked not to do that which led to T248718 which asks why. We should work towards this goal together.

I think the big time-sensitive issue right now is FlaggedRevisions is loading two versions of Codex - base AND full Codex - which is a mismatch with what the web team has been constrained to do in Vector and Minerva (following a considerate amount of discussion between performance (when it existed)/web/design systems team).

Even though codex-search-styles is on the way out (importantly : it's not deprecated only the Codex npm library is deprecated), my preference would be to either:

  • make FlaggedRevisions have a loading strategy consistent with Minerva and Vector since that seems to cover all the things the FlaggedRevisions interface requires and then work towards codex-styles everywhere.
  • Limit the loading of Codex-styles to only those that users that need it. When I visit https://de.wikipedia.org/wiki/Totalschaden_(Album)?useskin=vector-2022 I see absolutely nothing related to RevisionReviewFormUI in the page, so why is the module loaded in the first place?

A short term solution would avoid the existing performance situation we are in (T350724 is an UBN in my opinion) while not adding unnecessary time pressure on Design Systems Team to make a decision here prior to the web team rolling out Vector 2022 to FlaggedRevisions wikis.

While I can't do anything about Polish, I can't support knowingly rolling out to German Wikipedia in this current state - it just provides a legitimate avenue of criticism comparing Vector and Vector 2022 and creates unnecessary work for our community liaisons.

FWIW I agree with you that we should load Codex on page load, but as I mentioned before web team were specifically asked not to do that which led to T248718 which asks why. We should work towards this goal together.

I think this is about adoption, first we need to start adopting it in places ooui or other stuff are used and then we could make a case for loading of codex. Right now, it makes sense not to load it as it just adds another module to the user alongside jquery-ui (linkitem in wikibase client do that), ooui (many parts use that, like url shortener), wikimedia ui, etc..

I think the big time-sensitive issue right now is FlaggedRevisions is loading two versions of Codex - base AND full Codex - which is a mismatch with what the web team has been constrained to do in Vector and Minerva (following a considerate amount of discussion between performance (when it existed)/web/design systems team).

My grep of codex in FR only brings adding of "codex-styles" and nothing more. Where are you seeing this?

Even though codex-search-styles is on the way out (importantly : it's not deprecated only the Codex npm library is deprecated), my preference would be to either:

  • make FlaggedRevisions have a loading strategy consistent with Minerva and Vector since that seems to cover all the things the FlaggedRevisions interface requires and then work towards codex-styles everywhere.
  • Limit the loading of Codex-styles to only those that users that need it. When I visit https://de.wikipedia.org/wiki/Totalschaden_(Album)?useskin=vector-2022 I see absolutely nothing related to RevisionReviewFormUI in the page, so why is the module loaded in the first place?

This might be fixable, need someone to spend some time to debug it.

Change 973379 had a related patch set uploaded (by Tacsipacsi; author: Tacsipacsi):

[mediawiki/extensions/FlaggedRevs@master] Load codex-styles only for reviewers

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

While changing codex-styles to codex-search-styles may not (or may) be a good idea, not loading the review form styles for users who can’t review definitely won’t hurt, so I uploaded a patch for that. This will avoid loading full Codex in an incognito window, since anons can’t review.

Change 973379 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] Load codex-styles only for reviewers

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

Jdlrobson changed the task status from Open to Stalled.Nov 13 2023, 10:33 PM

Thanks @Tacsipacsi and @Ladsgroup for the helpful discussion here and the quick limitation to reviewers (which seems to fix the immediate concern)! I've folded the highlights of the discussion here into T248718 and I suggest we continue that there for now and stall this ticket until we get those guidelines.

Jdlrobson renamed this task from Projects with FlaggedRevisions installed load the entire Codex library and 10kb more CSS and likely have higher first paint to Projects with FlaggedRevisions installed load the entire Codex library and OOUI and 10kb more CSS, over 500kb of JavaScript and likely have higher first paint.EditedApr 3 2024, 6:57 AM
Jdlrobson triaged this task as High priority.

This is really really bad IMO. Coming from T340705#9682598 I've just noticed FlaggedRevisions also pulls in OOUI as well as Vue.js via this code and this is happening for anonymous page views on big projects like Russian Wikipedia:

[EDIT] I thought it was due to
https://gerrit.wikimedia.org/g/mediawiki/extensions/FlaggedRevs/+/aa366934663d316a4ef13b0522478820332a0041/includes/frontend/FlaggedRevsUIHooks.php#996 but I was wrong - see more recent subtasks.

Are you sure this code runs on article views? It’s in a method called onProtectionFormAddFormFields, a ProtectionFormAddFormFields hook handler, so it should run only on action=protect.

Are you sure this code runs on article views? It’s in a method called onProtectionFormAddFormFields, a ProtectionFormAddFormFields hook handler, so it should run only on action=protect.

After further inspection I am pretty sure it is ext.flaggedRevs.advanced and associated mediawiki.diff code: T361671 . I've clarified my earlier comment.

Are you sure this code runs on article views? It’s in a method called onProtectionFormAddFormFields, a ProtectionFormAddFormFields hook handler, so it should run only on action=protect.

After further inspection I am pretty sure it is ext.flaggedRevs.advanced and associated mediawiki.diff code: T361671 . I've clarified my earlier comment.