Another blocker in case the developing team haven’t noticed this − I thought would’ve not been repeated in Wikimedia extension (CSS extension [1] already has it and we all kind of learn on others’ mistakes). Templates such as navbox etc. are repeated for a dozen times sometimes [2], so it is deeply unacceptable to have same declarations in the code of the page.
[1]: https://www.mediawiki.org/wiki/Extension:CSS
[2]: https://ru.wikipedia.org/wiki/Загалло,_Марио
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | Anomie | T168333 CSS is duplicated on each template usage | |||
| Resolved | Anomie | T160563 Provide infrastructure for embedding styles in the content HTML with deduplication |
It was decided in T155813: Decide on storage and delivery method for TemplateStyles CSS that deduplication would not be a blocker. Providing the necessary infrastructure is already filed as T160563: Provide infrastructure for embedding styles in the content HTML with deduplication and patches are already filed as Gerrit 347441, Gerrit 347442, and Gerrit 347444 (and Gerrit 347445).
Change 347444 had a related patch set uploaded (by Anomie; owner: Anomie):
[mediawiki/extensions/TemplateStyles@master] Deduplicate embedded style rules
@Tgr @Anomie is your sense about this patch we can do it with relatively low risk ahead of a deployment to technical type wikis (e.g., wikitech)? Or would it make more sense to deploy what we have and then apply this patch later?
CC @Fjalapeno
The patch is blocked on review of required ResourceLoader changes (which is one of those areas with fuzzy ownership; the Performance team maybe?). I don't think it's risky either way.
Consensus in T155813 was that we'd deploy TemplateStyles as it is now and deploy deduplication at a later date whenever it's ready. For example,
Let's not have that discussion all over again, please.
"Performance Team" is what https://www.mediawiki.org/wiki/Developers/Maintainers says, although in practice it might really be "@Krinkle".
Thanks for asking. Aligning resources for code review is on the project manager's task list.
Change 347444 abandoned by Anomie:
Deduplicate embedded style rules
Reason:
Per abandonment of Ibc3fc372, going to do this differently
Change 393285 had a related patch set uploaded (by Anomie; owner: Anomie):
[mediawiki/extensions/TemplateStyles@master] Deduplicate embedded style rules
Change 393285 merged by jenkins-bot:
[mediawiki/extensions/TemplateStyles@master] Deduplicate embedded style rules
Fixed for desktop views. Parsoid (for apps) and the mobileview API (for mobile web) still need deduplication, not sure if we want to track that in this task.
Nothing more is going to need to be done in TemplateStyles for Parsoid or mobileview. They'll need to do the equivalent of T160563: Provide infrastructure for embedding styles in the content HTML with deduplication / rMW9b2b375fcef1: ParserOutput: Add 'deduplicateStyles' post-cache transformation, for which separate tasks should be filed if necessary.
For the deduplicated styles, do we really need the <link> tags? Couldn't we completely remove the tag?
It doesn’t do anything bad to keep them, the only thing that might be interested in removing them is probably mobile version (since every byte seems to count there).
I don't see any particular reason to do this specifically in MobileFrontend. More generally, this is a matter of distinguishing between canonical HTML and presentation mode.
When editing in VisualEditor, or when rendering partials of pages on mobile web or mobile apps, it is important that the client can keep the stylesheet around, even if it removed the section it was a part of, as long as there is another section that had a reference to it. Likewise, when a section if viewed without any references to that template, it can be omitted.
However, for page views as output by a skin (Vector, Minerva, MobileFrontend), they are indeed not needed. It seems like this falls in the same category as a lot of Parsoid attributes not needed that we strip on rendering. Except this is the first (obvious) instance of such concept for the PHP parser output.
I am starting to look at this from the Parsoid angle. Does the core parser de-duping rely on serialized text order to determine which one is the first occurrence ... i.e. depth-first pre-order traversal of the tree?
Alternatively, if a dom tree T with child subtrees S1 and S2 both have the same template style used in S1 and S2, ideally, both of those will be hoisted out and a single reference inserted in the root of T with link references in the original locations of S1 and S2. This ensures that styles for S1 and S2 are loaded before S1 and S2 are rendered.
Is one better / preferred compared to the other?
Change 452964 had a related patch set uploaded (by Subramanya Sastry; owner: Subramanya Sastry):
[mediawiki/services/parsoid@master] WIP: Dedupe template style occurrences
Depth-first pre-order probably, as that loads the style as late as possible (while still avoiding flash of unstyled content) so it does not delay first paint if the elements are way down below the initial viewport. OTOH with hoisting you could easily get style tags just before the main content block if different paragraphs or sections of the article use the same template.