@Jdlrobson https://versions.toolforge.org/ says 25 is out (since this morning), so if you're still seeing signs of the bug then there might be something else going on...

I've purged the pages to see if the change gets reflected in Grafana.
It seems we've only tackled the JS part of this. I see various styles in PhonosButton.less are unused on page load. Does this module need to be render blocking? It only seems to style the button when hovered or style the popup which is not shown on page load?

In T345414#9150691, @Jdlrobson wrote:

I've purged the pages to see if the change gets reflected in Grafana.
It seems we've only tackled the JS part of this. I see various styles in PhonosButton.less are unused on page load. Does this module need to be render blocking? It only seems to style the button when hovered or style the popup which is not shown on page load?

Some of the styles are needed on page load. We could move those to a separate module (ext.phonos.init.styles) and load ext.phonos.styles when the button is clicked, but this optimization would only save us less than 1 KB and possibly cause a flash of unstyled content. Is that worth it?

The JS payload is now significantly smaller on page load: https://grafana.wikimedia.org/goto/pgLnJjzSz?orgId=1

I've been experimenting to reduce the size of the CSS, but haven't really made any headway I'm afraid. :-( Splitting the PhonosButton.less styles makes not much difference (e.g. down to 118.92 KB from 119.75 KB uncompressed).

It seems that the largest (stylesheet) part is oojs-ui-core.styles at 8.24 KB, although there are lots of other ways we can reduce this. The image SVG data is 0.73 KB, and although it contains some redundancy — it includes three versions (normal, invert, and progressive) of three icons (reload, volumeOff, and volumeUp), I'm not sure that's where the biggest savings are.

Really, it seems to come down to OOUI being big, and I'm not really sure how to go about picking out just the bits we need because there's lots included in various individual files.

I wonder if we should be looking at getting rid of OOUI for Phonos, and building it from scratch. We're already overriding lots of OOUI's spacings, colours, animations, etc., and the additional work of handling events etc. ourselves might not be that large. Of course, usually we'd say that all UI should be built in a standard way, but perhaps Phonos should be thought more as wiki content than UI?

I might be missing something but if your need is only for the icon, you can actually use the icon directly from ooui. This is what we did in FlaggedRevs when we were using icons to show the user the status of the page. Check out:

		"ext.flaggedRevs.icons": {
			"class": "ResourceLoaderOOUIIconPackModule",
			"variants": {},
			"icons": [
				"block",
				"check",
				"eye",
				"articleSearch",
				"articleCheck"
			]
		}

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

[mediawiki/extensions/Phonos@master] Only load volume up icon on page load.

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

@Samwilson following @Ladsgroup suggestion would be great. The above patch does just that. Please feel free to amend and please backport that to production as soon as you can.

While here, I also noticed you are using wikimedia base, so please see https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Phonos/+/956469 as a follow up (not urgent).

@Jdlrobson I tested the patch where we only include the icons, we would see something like:

VS with OOUI styles:

We depend on some of the OOUI styles, like oo-ui-buttonWidget styles, that we are sometimes overriding in the extension and in different skins as well.

Like @Samwilson mentioned, maybe we should get rid off OOUI for the phonos button and build it from scratch.

I wonder if we should be looking at getting rid of OOUI for Phonos, and building it from scratch. We're already overriding lots of OOUI's spacings, colours, animations, etc., and the additional work of handling events etc. ourselves might not be that large. Of course, usually we'd say that all UI should be built in a standard way, but perhaps Phonos should be thought more as wiki content than UI?

@Jdlrobson I tested the patch where we only include the icons, we would see something like:

The patch is https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Phonos/+/956468/1. The patch you tested has some follow up changes that I didn't test properly. The patch only rearranges when styles are loaded, not what is loaded so there should be no visual changes. If styles inside oojs-ui-widgets.styles are needed, I would strongly recommend copying any render blocking styles to ext.phonos.styles.

We depend on some of the OOUI styles, like oo-ui-buttonWidget styles, that we are sometimes overriding in the extension and in different skins as well.

These are still loaded but would be pulled down only when the button is clicked.

Like @Samwilson mentioned, maybe we should get rid off OOUI for the phonos button and build it from scratch.

Seems like that is worth considering. I would personally recommending using the Codex icon only button CSS markup/mixin here:
https://doc.wikimedia.org/codex/latest/components/demos/button.html#css-only-version

But on the short term should we roll back Phonos or go with something like https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Phonos/+/956469 even if it causes a slight visual change? Other ideas welcome!

In my testing, r956468/3 saves us 6K uncompressed of CSS, but removes ext.phonos.styles and thus things look off. Adding that back, it looks okay, but only saves 3K uncompressed versus the master branch. Perhaps that will do as an iterative improvement, and then we can explore T345414#9152893 and/or r956469 to further reduce the size (though I'm confused how the latter helps)?

For later, we can explore getting rid of the OOUI dependency altogether, but I'm assuming that's not going to be a quick fix. The above might be a quick way just to get us out of UBN territory.

My understanding is the only styles we need on page load are to render this loudspeaker icon:

If so, then https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Phonos/+/956468 (amended) should be sufficient to get this out of UBN status without changing initial appearance. Let me know if this is simplifying things.

r956469 to further reduce the size (though I'm confused how the latter helps)?

This is not related to this ticket. Please ignore for now. Will create a new ticket once the dust has settled!

For later, we can explore getting rid of the OOUI dependency altogether, but I'm assuming that's not going to be a quick fix. The above might be a quick way just to get us out of UBN territory.

Yes long term this seems like a good idea - given that's the direction Vector and Minerva have taken for server side rendered content. We can arrange a chat with Design System Team and Web team to talk you through what that would look like.

+2'd https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Phonos/+/956468, will backport to wmf.26 today

Change 956468 merged by jenkins-bot:

[mediawiki/extensions/Phonos@master] Reduce initial payload of Phonos styles

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

Change 956809 had a related patch set uploaded (by Samtar; author: Jdlrobson):

[mediawiki/extensions/Phonos@wmf/1.41.0-wmf.26] Reduce initial payload of Phonos styles

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

Change 956809 merged by jenkins-bot:

[mediawiki/extensions/Phonos@wmf/1.41.0-wmf.26] Reduce initial payload of Phonos styles

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

Mentioned in SAL (#wikimedia-operations) [2023-09-12T13:36:35Z] <samtar@deploy1002> Started scap: Backport for [[gerrit:956809|Reduce initial payload of Phonos styles (T345414)]]

Mentioned in SAL (#wikimedia-operations) [2023-09-12T13:38:06Z] <samtar@deploy1002> samtar: Backport for [[gerrit:956809|Reduce initial payload of Phonos styles (T345414)]] synced to the testservers mwdebug1002.eqiad.wmnet, mwdebug2001.codfw.wmnet, mwdebug2002.codfw.wmnet, mwdebug1001.eqiad.wmnet, and mw-debug kubernetes deployment (accessible via k8s-experimental XWD option)

Mentioned in SAL (#wikimedia-operations) [2023-09-12T13:45:35Z] <samtar@deploy1002> Finished scap: Backport for [[gerrit:956809|Reduce initial payload of Phonos styles (T345414)]] (duration: 08m 59s)

Would it be okay for me to backport this to wmf.25 as well? I'd like to verify the fix addresses the problem (and we only run tests against enwiki).

In T345414#9160662, @Jdlrobson wrote:

Would it be okay for me to backport this to wmf.25 as well? I'd like to verify the fix addresses the problem (and we only run tests against enwiki).

Sure, feel free :-) I've cherry picked https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Phonos/+/956811 for .25

Change 956811 had a related patch set uploaded (by Samtar; author: Jdlrobson):

[mediawiki/extensions/Phonos@wmf/1.41.0-wmf.25] Reduce initial payload of Phonos styles

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

Change 956811 merged by jenkins-bot:

[mediawiki/extensions/Phonos@wmf/1.41.0-wmf.25] Reduce initial payload of Phonos styles

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

Mentioned in SAL (#wikimedia-operations) [2023-09-12T20:13:48Z] <cjming@deploy1002> Started scap: Backport for [[gerrit:956811|Reduce initial payload of Phonos styles (T345414)]]

Mentioned in SAL (#wikimedia-operations) [2023-09-12T20:15:25Z] <cjming@deploy1002> cjming and samtar: Backport for [[gerrit:956811|Reduce initial payload of Phonos styles (T345414)]] synced to the testservers mwdebug2001.codfw.wmnet, mwdebug1002.eqiad.wmnet, mwdebug1001.eqiad.wmnet, mwdebug2002.codfw.wmnet, and mw-debug kubernetes deployment (accessible via k8s-experimental XWD option)

Mentioned in SAL (#wikimedia-operations) [2023-09-12T20:25:54Z] <cjming@deploy1002> Finished scap: Backport for [[gerrit:956811|Reduce initial payload of Phonos styles (T345414)]] (duration: 12m 06s)

Change 956971 had a related patch set uploaded (by HMonroy; author: HMonroy):

[mediawiki/extensions/Phonos@master] Do not enable entire OOUI in PHP on page load

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

I submitted the patch above. I removed the line and everything seemed to be working as expected. Hopefully this helps!

Change 956814 had a related patch set uploaded (by Jdlrobson; author: HMonroy):

[mediawiki/extensions/Phonos@wmf/1.41.0-wmf.25] Do not enable entire OOUI in PHP on page load

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

Change 956815 had a related patch set uploaded (by Jdlrobson; author: HMonroy):

[mediawiki/extensions/Phonos@wmf/1.41.0-wmf.26] Do not enable entire OOUI in PHP on page load

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

Change 956971 merged by jenkins-bot:

[mediawiki/extensions/Phonos@master] Do not enable entire OOUI in PHP on page load

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

Would someone be able to backport this before I start work tomorrow (PST time zone?)

In T345414#9162099, @Jdlrobson wrote:

Would someone be able to backport this before I start work tomorrow (PST time zone?)

Sure, from T345414#9160662 and the subsequent confirmation, we thought you were handling it!

@MusikAnimal this is a backport for the new patch
(https://gerrit.wikimedia.org/r/956814). The old patch sadly didn't resolve the issue.

Change 956814 merged by jenkins-bot:

[mediawiki/extensions/Phonos@wmf/1.41.0-wmf.25] Do not enable entire OOUI in PHP on page load

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

Change 956815 merged by jenkins-bot:

[mediawiki/extensions/Phonos@wmf/1.41.0-wmf.26] Do not enable entire OOUI in PHP on page load

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

Mentioned in SAL (#wikimedia-operations) [2023-09-13T04:16:11Z] <hmonroy@deploy1002> Started scap: Backport for [[gerrit:956815|Do not enable entire OOUI in PHP on page load (T345414)]]

Mentioned in SAL (#wikimedia-operations) [2023-09-13T04:17:46Z] <hmonroy@deploy1002> hmonroy and jdlrobson: Backport for [[gerrit:956815|Do not enable entire OOUI in PHP on page load (T345414)]] synced to the testservers mwdebug1002.eqiad.wmnet, mwdebug2002.codfw.wmnet, mwdebug1001.eqiad.wmnet, mwdebug2001.codfw.wmnet, and mw-debug kubernetes deployment (accessible via k8s-experimental XWD option)

Mentioned in SAL (#wikimedia-operations) [2023-09-13T04:26:07Z] <hmonroy@deploy1002> Finished scap: Backport for [[gerrit:956815|Do not enable entire OOUI in PHP on page load (T345414)]] (duration: 09m 56s)

Mentioned in SAL (#wikimedia-operations) [2023-09-13T04:27:50Z] <hmonroy@deploy1002> Started scap: Backport for [[gerrit:956814|Do not enable entire OOUI in PHP on page load (T345414)]]

Mentioned in SAL (#wikimedia-operations) [2023-09-13T04:29:26Z] <hmonroy@deploy1002> hmonroy and jdlrobson: Backport for [[gerrit:956814|Do not enable entire OOUI in PHP on page load (T345414)]] synced to the testservers mwdebug1001.eqiad.wmnet, mwdebug2001.codfw.wmnet, mwdebug1002.eqiad.wmnet, mwdebug2002.codfw.wmnet, and mw-debug kubernetes deployment (accessible via k8s-experimental XWD option)

Mentioned in SAL (#wikimedia-operations) [2023-09-13T04:35:48Z] <hmonroy@deploy1002> Finished scap: Backport for [[gerrit:956814|Do not enable entire OOUI in PHP on page load (T345414)]] (duration: 07m 58s)

@Jdlrobson I'll hold off optimistically dropping the priority this time, but hopefully this has resolved the issue — if not, we can revert deployment (could this be limited to "just" undeploying from the most impacted projects, i.e. enwiki?, even though this is where the most usage is currently..)

Oops, forgot to FYI Language-Team on this — I know @UOzurumba is aware :-)

@Jdlrobson Thank you for your help!

Indeed, thank you Jon! This incident has reminded me of T202154 and the importance of monitoring performance when deploying new features. 9KB or what have you is not a lot, but multiplied times millions of requests, it is quite significant. We will be more diligent about this moving forward :)

Currently the icon appears larger than the container and clipped until you click on it. Is this just going to be fixed with the next deployment?

In T345414#9168804, @Nardog wrote:

Currently the icon appears larger than the container and clipped until you click on it. Is this just going to be fixed with the next deployment?

Happening even on Beta Cluster, so I assume not. Filed T346592.