Page MenuHomePhabricator

SkinModule is missing cache invalidation for feature files
Closed, ResolvedPublic

Description

When changing e.g. logo: true to logo: false the version hash stays the same. The version can be seen in the dev tools via the E-Tag version header in the CSS response, or in the startup manifest, or by evaluating mw.loader.moduleRegistry['skins.vector.styles.legacy'].version from the console.

On a local wiki with no file-level or HTTP-level caching one can still get the change to show up by doing a hard refresh, but there is no such thing as a hard refresh on a production wiki where one has MW's FileCache enabled, a cache proxy, or a CDN.

[…] My guess is that this is another regression from the skin refactoring, T287410: ResourceLoaderSkinModule: Skins should not use the legacy styling feature which recently touched stuff like https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/skins/Vector/+/a50b1db7931610a038197b74ba798fe307e9055e

Also for some reason Timeless has the "content-media" feature set to false, so it's just fully broken there AFAICT.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change 714841 had a related patch set uploaded (by Krinkle; author: Arlolra):

[mediawiki/core@master] resourceloader: Call getStyleFiles from FileModule::getFileHashes

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

This is the before/after for this patch when dumping the stylesheets added in FileModule::getFileHashes():

1$ colordiff -u before.txt after.txt
2--- before.txt 2021-09-02
3+++ after.txt 2021-09-02
4
5-skins.vector.styles: ["mediawiki/skins/Vector/resources/common/common.less","mediawiki/skins/Vector/resources/skins.vector.styles/skin.less"]
6+skins.vector.styles: ["mediawiki/resources/src/mediawiki.skinning/normalize.less","mediawiki/resources/src/mediawiki.skinning/messageBoxes.less","mediawiki/resources/src/mediawiki.skinning/toc/common.css","mediawiki/skins/Vector/resources/common/common.less","mediawiki/skins/Vector/resources/skins.vector.styles/skin.less","mediawiki/resources/src/mediawiki.skinning/content.thumbnails.less","mediawiki/resources/src/mediawiki.skinning/content.links.less","mediawiki/resources/src/mediawiki.skinning/content.externallinks.less","mediawiki/resources/src/mediawiki.skinning/content.body.less","mediawiki/resources/src/mediawiki.skinning/content.tables.less","mediawiki/resources/src/mediawiki.skinning/interface.less","mediawiki/resources/src/mediawiki.skinning/interface.category.less","mediawiki/resources/src/mediawiki.skinning/elements.less","mediawiki/resources/src/mediawiki.skinning/i18n-ordered-lists.less","mediawiki/resources/src/mediawiki.skinning/i18n-all-lists-margins.less","mediawiki/resources/src/mediawiki.skinning/i18n-headings.less","mediawiki/resources/src/mediawiki.skinning/toc/screen.less","mediawiki/resources/src/mediawiki.skinning/content.thumbnails-print.less","mediawiki/resources/src/mediawiki.skinning/content.body-print.less","mediawiki/resources/src/mediawiki.skinning/content.tables-print.less","mediawiki/resources/src/mediawiki.skinning/interface-print.less","mediawiki/resources/src/mediawiki.skinning/interface.category-print.less","mediawiki/resources/src/mediawiki.skinning/elements-print.less","mediawiki/resources/src/mediawiki.skinning/toc/print.css"]
7
8-skins.vector.styles.legacy: ["mediawiki/skins/Vector/resources/common/common.less","mediawiki/skins/Vector/resources/skins.vector.styles.legacy/skin-legacy.less"]
9+skins.vector.styles.legacy: ["mediawiki/resources/src/mediawiki.skinning/normalize.less","mediawiki/resources/src/mediawiki.skinning/logo.less","mediawiki/resources/src/mediawiki.skinning/messageBoxes.less","mediawiki/resources/src/mediawiki.skinning/toc/common.css","mediawiki/skins/Vector/resources/common/common.less","mediawiki/skins/Vector/resources/skins.vector.styles.legacy/skin-legacy.less","mediawiki/resources/src/mediawiki.skinning/logo-print.less","mediawiki/resources/src/mediawiki.skinning/content.thumbnails-print.less","mediawiki/resources/src/mediawiki.skinning/content.body-print.less","mediawiki/resources/src/mediawiki.skinning/content.tables-print.less","mediawiki/resources/src/mediawiki.skinning/interface-print.less","mediawiki/resources/src/mediawiki.skinning/interface.category-print.less","mediawiki/resources/src/mediawiki.skinning/elements-print.less","mediawiki/resources/src/mediawiki.skinning/toc/print.css","mediawiki/resources/src/mediawiki.skinning/content.thumbnails.less","mediawiki/resources/src/mediawiki.skinning/content.links.less","mediawiki/resources/src/mediawiki.skinning/content.externallinks.less","mediawiki/resources/src/mediawiki.skinning/content.body.less","mediawiki/resources/src/mediawiki.skinning/content.tables.less","mediawiki/resources/src/mediawiki.skinning/interface.less","mediawiki/resources/src/mediawiki.skinning/interface.category.less","mediawiki/resources/src/mediawiki.skinning/elements.less","mediawiki/resources/src/mediawiki.skinning/i18n-ordered-lists.less","mediawiki/resources/src/mediawiki.skinning/i18n-all-lists-margins.less","mediawiki/resources/src/mediawiki.skinning/i18n-headings.less","mediawiki/resources/src/mediawiki.skinning/toc/screen.less"]
10
11-skins.monobook.styles: ["mediawiki/skins/MonoBook/resources/skin.less","mediawiki/skins/MonoBook/resources/skin-responsive.less"]
12+skins.monobook.styles: ["mediawiki/resources/src/mediawiki.skinning/normalize.less","mediawiki/resources/src/mediawiki.skinning/logo.less","mediawiki/resources/src/mediawiki.skinning/messageBoxes.less","mediawiki/resources/src/mediawiki.skinning/toc/common.css","mediawiki/skins/MonoBook/resources/skin.less","mediawiki/skins/MonoBook/resources/skin-responsive.less","mediawiki/resources/src/mediawiki.skinning/logo-print.less","mediawiki/resources/src/mediawiki.skinning/content.thumbnails-print.less","mediawiki/resources/src/mediawiki.skinning/content.body-print.less","mediawiki/resources/src/mediawiki.skinning/content.tables-print.less","mediawiki/resources/src/mediawiki.skinning/interface-print.less","mediawiki/resources/src/mediawiki.skinning/interface.category-print.less","mediawiki/resources/src/mediawiki.skinning/elements-print.less","mediawiki/resources/src/mediawiki.skinning/toc/print.css","mediawiki/resources/src/mediawiki.skinning/content.thumbnails.less","mediawiki/resources/src/mediawiki.skinning/content.links.less","mediawiki/resources/src/mediawiki.skinning/content.externallinks.less","mediawiki/resources/src/mediawiki.skinning/content.body.less","mediawiki/resources/src/mediawiki.skinning/content.tables.less","mediawiki/resources/src/mediawiki.skinning/interface.less","mediawiki/resources/src/mediawiki.skinning/interface.category.less","mediawiki/resources/src/mediawiki.skinning/elements.less","mediawiki/resources/src/mediawiki.skinning/i18n-ordered-lists.less","mediawiki/resources/src/mediawiki.skinning/i18n-all-lists-margins.less","mediawiki/resources/src/mediawiki.skinning/i18n-headings.less","mediawiki/resources/src/mediawiki.skinning/toc/screen.less"]
13
14
15+mediawiki.skinning.elements: ["mediawiki/resources/src/mediawiki.skinning/content.body.less","mediawiki/resources/src/mediawiki.skinning/elements.less","mediawiki/resources/src/mediawiki.skinning/legacy.less","mediawiki/resources/src/mediawiki.skinning/toc/screen.less","mediawiki/resources/src/mediawiki.skinning/content.body-print.less","mediawiki/resources/src/mediawiki.skinning/elements-print.less","mediawiki/resources/src/mediawiki.skinning/commonPrint.less","mediawiki/resources/src/mediawiki.skinning/toc/print.css","mediawiki/resources/src/mediawiki.skinning/messageBoxes.less","mediawiki/resources/src/mediawiki.skinning/toc/common.css"]
16+mediawiki.skinning.content: ["mediawiki/resources/src/mediawiki.skinning/content.thumbnails.less","mediawiki/resources/src/mediawiki.skinning/content.body.less","mediawiki/resources/src/mediawiki.skinning/elements.less","mediawiki/resources/src/mediawiki.skinning/legacy.less","mediawiki/resources/src/mediawiki.skinning/toc/screen.less","mediawiki/resources/src/mediawiki.skinning/content.thumbnails-print.less","mediawiki/resources/src/mediawiki.skinning/content.body-print.less","mediawiki/resources/src/mediawiki.skinning/elements-print.less","mediawiki/resources/src/mediawiki.skinning/commonPrint.less","mediawiki/resources/src/mediawiki.skinning/toc/print.css","mediawiki/resources/src/mediawiki.skinning/messageBoxes.less","mediawiki/resources/src/mediawiki.skinning/toc/common.css"]
17+mediawiki.skinning.interface: ["mediawiki/resources/src/mediawiki.skinning/logo.less","mediawiki/resources/src/mediawiki.skinning/messageBoxes.less","mediawiki/resources/src/mediawiki.skinning/toc/common.css","mediawiki/resources/src/mediawiki.skinning/logo-print.less","mediawiki/resources/src/mediawiki.skinning/content.thumbnails-print.less","mediawiki/resources/src/mediawiki.skinning/content.body-print.less","mediawiki/resources/src/mediawiki.skinning/interface-print.less","mediawiki/resources/src/mediawiki.skinning/elements-print.less","mediawiki/resources/src/mediawiki.skinning/commonPrint.less","mediawiki/resources/src/mediawiki.skinning/toc/print.css","mediawiki/resources/src/mediawiki.skinning/content.thumbnails.less","mediawiki/resources/src/mediawiki.skinning/content.body.less","mediawiki/resources/src/mediawiki.skinning/interface.less","mediawiki/resources/src/mediawiki.skinning/elements.less","mediawiki/resources/src/mediawiki.skinning/legacy.less","mediawiki/resources/src/mediawiki.skinning/toc/screen.less"]
18
19+mediawiki.skinning.content.externallinks: ["mediawiki/resources/src/mediawiki.skinning/content.links.less","mediawiki/resources/src/mediawiki.skinning/content.externallinks.less"]
20
21 mediawiki.icon: ["mediawiki/resources/src/mediawiki.icon/icon.less"]
22-mediawiki.notification: ["mediawiki/resources/src/mediawiki.notification/common.css","mediawiki/resources/src/mediawiki.notification/print.css","mediawiki/resources/src/mediawiki.notification/default.css","mediawiki/skins/Vector/skinStyles/mediawiki.notification.less"]
23+mediawiki.notification: ["mediawiki/resources/src/mediawiki.notification/common.css","mediawiki/resources/src/mediawiki.notification/default.css","mediawiki/skins/Vector/skinStyles/mediawiki.notification.less","mediawiki/resources/src/mediawiki.notification/print.css"]
24 mediawiki.toc: ["mediawiki/resources/src/mediawiki.toc/toc.css"]
25+mediawiki.toc.styles: ["mediawiki/resources/src/mediawiki.skinning/toc/common.css","mediawiki/resources/src/mediawiki.skinning/toc/screen.less","mediawiki/resources/src/mediawiki.skinning/toc/print.css"]
26
27+mediawiki.legacy.shared: ["mediawiki/resources/src/mediawiki.skinning/content.body.less","mediawiki/resources/src/mediawiki.skinning/legacy.less","mediawiki/resources/src/mediawiki.skinning/toc/screen.less","mediawiki/resources/src/mediawiki.skinning/content.body-print.less","mediawiki/resources/src/mediawiki.skinning/commonPrint.less","mediawiki/resources/src/mediawiki.skinning/toc/print.css","mediawiki/resources/src/mediawiki.skinning/messageBoxes.less","mediawiki/resources/src/mediawiki.skinning/toc/common.css"]
28

Modules with an added line only, are modules that were effectively untracked completely and had a constant version no matter how much their less files changed.

Modules with a removed and added line, are modules that previously only tracked some of their files and were ignoring any changes to their Less files until/less some files that were tracked also changed.

Krinkle triaged this task as High priority.
Krinkle moved this task from Inbox to Doing: Prio Interrupt on the Performance-Team board.
Krinkle moved this task from Inbox to Confirmed Problem on the MediaWiki-ResourceLoader board.

Change 714841 merged by jenkins-bot:

[mediawiki/core@master] resourceloader: Call getStyleFiles from FileModule::getFileHashes

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

Does this need backporting to 1.36?

Change 719179 had a related patch set uploaded (by Krinkle; author: Arlolra):

[mediawiki/core@REL1_36] resourceloader: Call getStyleFiles from FileModule::getFileHashes

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

Change 719180 had a related patch set uploaded (by Krinkle; author: Arlolra):

[mediawiki/core@REL1_35] resourceloader: Call getStyleFiles from FileModule::getFileHashes

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

Yes, thanks for the reminder. Done now.

Change 719180 merged by jenkins-bot:

[mediawiki/core@REL1_35] resourceloader: Call getStyleFiles from FileModule::getFileHashes

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

Change 719179 merged by jenkins-bot:

[mediawiki/core@REL1_36] resourceloader: Call getStyleFiles from FileModule::getFileHashes

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