Page MenuHomePhabricator

WikimediaBadges loads 1.2kb render blocking CSS on page load even for pages which do not have badges
Closed, ResolvedPublicBUG REPORT

Description

This has been flagged by recent tests added in MediaWiki Core

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

What happens?:
The ext.wikimediaBadges module has been loaded on the page despite not being used anywhere in the page

What should have happened instead?:
The module should be loaded conditionally and only if the page has badges

Software version (on Special:Version page; skip for WMF-hosted wikis like Wikipedia):

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

Event Timeline

Jdlrobson renamed this task from WikimediaBadges loads CSS on page load even for pages which do not have badges to WikimediaBadges loads 1.2kb render blocking CSS on page load even for pages which do not have badges.Jan 7 2025, 9:14 PM

I've created a workaround for CI in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikimediaBadges/+/1108836 for now but ideally this would just be fixed as that's 1kB less of render blocking CSS we can load on articles and I'd hope this would be relatively easy to fix.

@awight can you help me route this ticket to the right team? Thanks in advance!

Change #1108836 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Jdlrobson):

[mediawiki/extensions/WikimediaBadges@master] Document that WikimediaBadges unconditionally loads on page load

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

Change #1108836 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Jdlrobson):

[mediawiki/extensions/WikimediaBadges@master] Document that WikimediaBadges unconditionally loads on page load

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

In case it wasn't 100% clear, I didn't associate this Gerrit patch with this ticket, as this is just a short term fix and doesn't fix the issue I'm raising here.
The fix would look something like:

class BeforePageDisplayHookHandler implements BeforePageDisplayHook {
	/** @inheritDoc */
	public function onBeforePageDisplay( $out, $skin ): void {
               // I don't know enough about the extension to know what methjod to use here, but it should check whether badges have been outputted to page.
                if ( WikibaseClass::hasBadges( $out ) {
   		  $out->addModuleStyles( 'ext.wikimediaBadges' );
                 }
	}
}

Change #1108836 merged by jenkins-bot:

[mediawiki/extensions/WikimediaBadges@master] Document that WikimediaBadges unconditionally loads on page load

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

Change #1112266 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/WikimediaBadges@master] Limit ext.wikimediaBadges module to pages that might need it

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

I think the information about the badges should be available in $out->getProperty( 'wikibase_badges' ) (added to the ParserOutput in LanguageLinkBadgeDisplay and copied from there to the OutputPage in SidebarHookHandler), but on my local wiki I haven’t been able to make that return anything other than an empty array yet. Possibly I just don’t have the right test pages set up.

I think this task falls into #wikidata_integrations_team / Wikidata Integration in Wikimedia projects territory, but hopefully the above note helps ^^

Thanks! @Lucas_Werkmeister_WMDE who in that team would be a good person to reach out to?

(I’d like to gently push back against the word “review” here – while the attached change is one possible approach, IMHO we should be able to develop a better / less hacky one. See also my comment above.)

Change #1119736 had a related patch set uploaded (by Joely Rooke WMDE; author: Joely Rooke WMDE):

[mediawiki/extensions/WikimediaBadges@master] Limit ext.wikimediaBadges module to pages whose sitelinks have badges

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

A change conditionally adding the ext.WikimediaBadges module has been reviewed and merged, and will be deployed with the next train, if all goes smoothly. Once it's deployed and tested again on prod, would it make sense to revert change 1108836 since it seemed a temporary workaround for CI breakage?

Change #1119736 merged by jenkins-bot:

[mediawiki/extensions/WikimediaBadges@master] Limit ext.wikimediaBadges module to pages whose sitelinks have badges

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

Change #1112266 abandoned by Jdlrobson:

[mediawiki/extensions/WikimediaBadges@master] Limit ext.wikimediaBadges module to pages that might need it

Reason:

Made obselete by https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikimediaBadges/+/1119736

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

Once it's deployed and tested again on prod, would it make sense to revert change 1108836 since it seemed a temporary workaround for CI breakage?

Probably yes. If CI is happy with it, it should be fine to merge a revert. (If CI isn’t happy with it, then I suppose we can either investigate why the default page used by the CI job appears to use badges and thus pull in the module, or just ignore it and not revert after all.)

Change #1120681 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/WikimediaBadges@master] Revert "Document that WikimediaBadges unconditionally loads on page load"

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

@Danny_Benjafield_WMDE Thanks for adding a proposed entry directly to Tech News. The entry currently says:

  • A conditional gate was added to Extension:WikimediaBadges, reducing up to 1.2 KB of render-blocking CSS on pages without badges. [2]

I wonder if we can clarify that wording for people who are unfamiliar with the ramifications, and why we're telling them/anyone?
I.e. We generally only include entries that contain information that some editors/volunteer-devs will really want to know, or need to take action on.
E.g. Is there a role/demographic that will need/want to be aware of this in particular? (if so, we could write something at the start like "Developers who work with [topic], should be aware that [...]"). -- And, is there anything that anyone could/should do with the information? (E.g. we could point to relevant docs/examples of how to help, or what kind of tools they might need update to compensate, or etc)
If this is purely an informational "we made the sites load a bit faster" announcement, then I'm not sure if it warrants highlighting in Tech News, as those are common tasks that don't each need global awareness?
Thoughts/redrafts would be welcome. Thanks!

@Quiddity , it was an informational addition to celebrate reducing the page load time. I don't think in this case any redraft is necessary. I'll keep in mind the intended audience for any future updates/additions.
Thank you.

Change #1120681 merged by jenkins-bot:

[mediawiki/extensions/WikimediaBadges@master] Revert "Document that WikimediaBadges unconditionally loads on page load"

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

Confirmed this is working on https://en.wikipedia.org/wiki/Main_Page
Note: https://en.wikipedia.org/wiki/Spain (and presumably many pages with good articles) still loads the module because the icons are used in the language dropdown, but these won't be seen by most people.

e.g. with JS disabled:

Screenshot 2025-03-03 at 9.24.43 AM.png (622×408 px, 40 KB)

A good follow up here would be to load these on demand when the language overlay is open on Vector 2022.