Page MenuHomePhabricator

Disable wgParserEnableLegacyMediaDOM on all wikis
Closed, ResolvedPublic

Description

The current state is,
https://github.com/wikimedia/operations-mediawiki-config/blob/master/wmf-config/InitialiseSettings.php#L28379-L28388

'wgParserEnableLegacyMediaDOM' => [
	'default' => true,
	'group0' => false,
	'test2wiki' => false,
	'wikitech' => false,
	// Careful with these, since they can partake in cross-wiki inclusion.
	// See T51097#7401266
	'metawiki' => true,
	'commonswiki' => true,
],

Initial notice in T113258#7209988

Roughs steps would be,

but we'd like to get some wikis to opt-in beforehand


More information about what this flag enables at https://www.mediawiki.org/wiki/Parsoid/Parser_Unification/Media_structure/FAQ

Details

SubjectRepoBranchLines +/-
mediawiki/coremaster+18 -0
operations/mediawiki-configmaster+1 -18
mediawiki/coreREL1_40+8 -3
mediawiki/skins/VectorREL1_40+2 -2
mediawiki/coremaster+8 -3
mediawiki/skins/Vectormaster+2 -2
mediawiki/extensions/PageImagesmaster+7 -3
mediawiki/extensions/TimedMediaHandlermaster+32 -12
mediawiki/vendormaster+2 K -528
mediawiki/coremaster+330 -0
mediawiki/services/parsoidmaster+14 -6
operations/mediawiki-configmaster+1 -0
operations/mediawiki-configmaster+1 -0
operations/mediawiki-configmaster+1 -0
operations/mediawiki-configmaster+1 -0
operations/mediawiki-configmaster+1 -0
operations/mediawiki-configmaster+1 -0
Show related patches Customize query in gerrit

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Arlo is on vacation till end of next week, but I think it is okay to mark the FAQ for translation. I expect it is not too disruptive to make tweaks as long as there are no wholesale changes? But, I leave that decision to you since you know better what is best practice.

My sense is that we should add all links to the FAQ and have the Tech News entry be simply a heads up about the changes and a call to action to look at the FAQ for detailed techincal guidance for required changes.

Sounds good, I agree that centralizing it all would be best. Thanks!
I've marked the existing FAQ content for translation (including leaving the localizable parameters like center available for translation (not tvar'd), which I'm relatively sure I've gotten correct).

Overall, I believe it's best if we can include the details from those other 2 tasks in the FAQ directly, and to announce this when Arlo is around to answer any possible questions. Therefore, we will plan for including it in 28 Nov edition (drafted by 24/25th).

  1. Are there any other examples (from those 2 new phab tasks) that could/should be added into the FAQ page, to provide a 'one-stop-location' for editors to find all the most common tips?

What's there pretty much covers the issues that came up on the opt-in wikis.

  1. Within the Tech News entry, and the FAQ, what else needs to communicated most crucially to the local technical editors? -- with the assumption that some of them might not read any English, but can still follow "search&replace" style guidance.

I've added to the "How can I help?" section a call to action for local technical editors to update gadgets and site specific assets for issues that are known and fixes provided,
https://www.mediawiki.org/w/index.php?title=Parsoid%2FParser_Unification%2FMedia_structure%2FFAQ&type=revision&diff=5613463&oldid=5585748

That's probably the most crucial thing to communicate since it should ideally be done before we enable on those wikis.

  1. Let me know when the current FAQ text is 'stable' (i.e. additions might arrive, but the existing text won't change), so that we can mark the page as "ready for translation".

It seems to already have been marked for translation, which is fine

Coming over from [[mw:Parsoid/Parser Unification/Media structure/FAQ]], I still do not understand why you chose such convoluted CSS for this instead of agreeing on one class that would describe all of these thumbnailed media files equally. Just a baffling decision, especially given that there are classes used in the layout modifiers for this new syntax.

Also, this new syntax means that every template that tries to replicate how .thumb looks would break without a bunch of skin-specific CSS that could and would break. Frankly, you need to provide more justification for why the CSS uses figure[typeof~="…"] syntax and not something as clear as .mw-imageblock.

insource:"class thumb" prefix:template: lends a rough estimate of 70 templates in enWP and 20 in ruWP, for example.

dewiki also uses some of the thumb classes (thumbinner, tleft, tright, thumbcaption) in templates and article text. Will they be preserved for a migration period? And is it possible to give a complete list of classes that will be changed/renamed?

Coming over from [[mw:Parsoid/Parser Unification/Media structure/FAQ]], I still do not understand why you chose such convoluted CSS for this instead of agreeing on one class that would describe all of these thumbnailed media files equally. Just a baffling decision, especially given that there are classes used in the layout modifiers for this new syntax.

Also, this new syntax means that every template that tries to replicate how .thumb looks would break without a bunch of skin-specific CSS that could and would break. Frankly, you need to provide more justification for why the CSS uses figure[typeof~="…"] syntax and not something as clear as .mw-imageblock.

Thank you. Yes, your position was made quite clear in T304010,

figure[typeof~="…"] CSS needs fixing to stop triggering on random elements inside a <figure>. This is why CSS authors use classes, and why the old thumb layout actually used classes. Please just use classes instead, or even data attributes if this is some unexplainable author preference.

The observation was not lost and resulted in the proposal in T314097.

As in the FAQ, the new structure originated in Parsoid. Parsoid's specified output was designed around RDFa to convey meaning to editing clients (like VE) so the HTML it produces can be roundtripped back to serialize wikitext. In other words, the design was around editing needs, not necessarily read views.

Now, the same information could be surfaced with classes, as you say, but it would be redundant with the RDFa already present in the HTML and that's something we've tried to avoid. We could rethink the choice of RDFa but fixing that would mean an awful lot of work to the current consumers of Parsoid's HTML.

Ultimately, though, the strongest argument I can think of for having both is outlined in T318433. As you say, template authors often mimic the parser output around media and would like to reuse the classes. However, adding the RDFa to the template output might not necessarily be semantically correct (not to mention that it would be sanitized away). In other words, it might not be wrapping the output of a file.

Note though, at present, the styling rules targeting the old classes are not being deleted so none of the templates mimicking the old parser output should break. Parsoid does extensive visual difference testing and we don’t expect to see rendering differences with this change. As the FAQ mentions, we’ve already enabled the output on a number of larger wikis (itwiki, viwiki, cswiki) with some success.

That’s not to say no templates will need updating (for example see T287963#8234537) but we’ve tried to cover the general cases to look out for in FAQ.

In summary: Hopefully this provides some historical context for why the styling rules target the RDFa, lends some assurance that the templates outputting the older classes should, for the mostpart, continue to work, and that we're probably in agreement that classes should be added so template authors can make use of them, which is T318433.

I think there's a valid engineering tradeoff in "don't want to add more to the emitted HTML" but at the now-quite-obvious cost of both "potentially adding selectors to the CSS and forcing client machines to digest the attribute version, which was known to be more processing expensive at some point in the past decade". (Which you allude to with your link from the above but which I'm not totally sure you were cognizant that attribute selectors weren't optimized relatively recently and I still don't honestly know if they are today.)

I'm just not sure this particularly was the point on which it was necessary or wise to try to reduce Parsoid's thicc HTML reading footprint especially with tasks like T54091: The read HTML should have hinting to allow full DOM copying (as opposed to just rich copying) from read mode into VE surfaces on the books. Classes are nice to deal with. Attributes a lot less so. :)

Just remembered that Kartographer also has thumb classes in use (both in HTML generation and in JS). So that probably also needs a rewrite.

Just remembered that Kartographer also has thumb classes in use (both in HTML generation and in JS). So that probably also needs a rewrite.

Thanks, that was note in T271114#8247024 and should be covered by T318433

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

[operations/mediawiki-config@master] Disable wgParserEnableLegacyMediaDOM on group1 wikis

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

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

[operations/mediawiki-config@master] Disable wgParserEnableLegacyMediaDOM on cawiki

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

Change 866653 merged by jenkins-bot:

[operations/mediawiki-config@master] Disable wgParserEnableLegacyMediaDOM on cawiki

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

Mentioned in SAL (#wikimedia-operations) [2022-12-15T14:18:31Z] <lucaswerkmeister-wmde@deploy1002> Started scap: Backport for [[gerrit:866653|Disable wgParserEnableLegacyMediaDOM on cawiki (T297984 T314318)]]

Mentioned in SAL (#wikimedia-operations) [2022-12-15T14:20:15Z] <lucaswerkmeister-wmde@deploy1002> lucaswerkmeister-wmde and arlolra: Backport for [[gerrit:866653|Disable wgParserEnableLegacyMediaDOM on cawiki (T297984 T314318)]] synced to the testservers: mwdebug1001.eqiad.wmnet, mwdebug2002.codfw.wmnet, mwdebug2001.codfw.wmnet, mwdebug1002.eqiad.wmnet

Mentioned in SAL (#wikimedia-operations) [2022-12-15T14:27:49Z] <lucaswerkmeister-wmde@deploy1002> Finished scap: Backport for [[gerrit:866653|Disable wgParserEnableLegacyMediaDOM on cawiki (T297984 T314318)]] (duration: 09m 18s)

Change 865214 merged by jenkins-bot:

[operations/mediawiki-config@master] Disable wgParserEnableLegacyMediaDOM on group1 wikis

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

Mentioned in SAL (#wikimedia-operations) [2023-02-01T21:18:16Z] <kindrobot@deploy1002> Started scap: Backport for [[gerrit:865214|Disable wgParserEnableLegacyMediaDOM on group1 wikis (T314318)]]

Mentioned in SAL (#wikimedia-operations) [2023-02-01T21:20:08Z] <kindrobot@deploy1002> arlolra and kindrobot: Backport for [[gerrit:865214|Disable wgParserEnableLegacyMediaDOM on group1 wikis (T314318)]] synced to the testservers: mwdebug1002.eqiad.wmnet, mwdebug1001.eqiad.wmnet, mwdebug2001.codfw.wmnet, mwdebug2002.codfw.wmnet

Mentioned in SAL (#wikimedia-operations) [2023-02-01T21:32:13Z] <kindrobot@deploy1002> Finished scap: Backport for [[gerrit:865214|Disable wgParserEnableLegacyMediaDOM on group1 wikis (T314318)]] (duration: 13m 56s)

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

[mediawiki/core@master] Add explicit wgParserEnableLegacyMediaDOM config to legacyMedia.txt

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

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

[mediawiki/services/parsoid@master] Add explicit wgParserEnableLegacyMediaDOM config to parserTests

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

Change 887817 merged by jenkins-bot:

[mediawiki/services/parsoid@master] Add explicit wgParserEnableLegacyMediaDOM config to parserTests

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

Change 887816 merged by jenkins-bot:

[mediawiki/core@master] Add explicit wgParserEnableLegacyMediaDOM config to legacyMedia.txt

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

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

[mediawiki/core@master] [WIP] Change the default of wgParserEnableLegacyMediaDOM to false

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

Change 888790 had a related patch set uploaded (by Subramanya Sastry; author: Subramanya Sastry):

[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.17.0-a15

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

Change 888790 merged by jenkins-bot:

[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.17.0-a15

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

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

[mediawiki/skins/Vector@master] [WIP] Bump legacy styles size for content-media feature

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

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

[mediawiki/extensions/TimedMediaHandler@master] Add explicit wgParserEnableLegacyMediaDOM config to parserTests.txt

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

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

[mediawiki/extensions/PageImages@master] Add explicit wgParserEnableLegacyMediaDOM config to pageImagesParserTests.txt

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

Change 889223 merged by jenkins-bot:

[mediawiki/extensions/TimedMediaHandler@master] Add explicit wgParserEnableLegacyMediaDOM config to parserTests.txt

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

Change 889228 merged by jenkins-bot:

[mediawiki/extensions/PageImages@master] Add explicit wgParserEnableLegacyMediaDOM config to pageImagesParserTests.txt

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

Change 888810 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Bump styles sizes for content-media feature

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

Change 888127 merged by jenkins-bot:

[mediawiki/core@master] Change the default of wgParserEnableLegacyMediaDOM to false

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

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

[mediawiki/core@REL1_40] Change the default of wgParserEnableLegacyMediaDOM to false

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

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

[mediawiki/skins/Vector@REL1_40] Bump styles sizes for content-media feature

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

Change 901295 merged by jenkins-bot:

[mediawiki/skins/Vector@REL1_40] Bump styles sizes for content-media feature

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

Change 901294 merged by jenkins-bot:

[mediawiki/core@REL1_40] Change the default of wgParserEnableLegacyMediaDOM to false

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

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

[operations/mediawiki-config@master] Disable wgParserEnableLegacyMediaDOM on group2 wikis

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

Change 936322 merged by jenkins-bot:

[operations/mediawiki-config@master] Disable wgParserEnableLegacyMediaDOM on group2 wikis

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

Mentioned in SAL (#wikimedia-operations) [2023-07-10T13:05:59Z] <lucaswerkmeister-wmde@deploy1002> Started scap: Backport for [[gerrit:936322|Disable wgParserEnableLegacyMediaDOM on group2 wikis (T314318)]]

Mentioned in SAL (#wikimedia-operations) [2023-07-10T13:07:36Z] <lucaswerkmeister-wmde@deploy1002> lucaswerkmeister-wmde and arlolra: Backport for [[gerrit:936322|Disable wgParserEnableLegacyMediaDOM on group2 wikis (T314318)]] synced to the testservers: mwdebug1002.eqiad.wmnet, mwdebug2002.codfw.wmnet, mwdebug1001.eqiad.wmnet, mwdebug2001.codfw.wmnet

Mentioned in SAL (#wikimedia-operations) [2023-07-10T13:16:26Z] <lucaswerkmeister-wmde@deploy1002> Finished scap: Backport for [[gerrit:936322|Disable wgParserEnableLegacyMediaDOM on group2 wikis (T314318)]] (duration: 10m 26s)

This possibly broke QICvote on Commons - please have a look at T341736.

In May 2023 I’ve tried to switch Panorama template in Russian Wikipedia to a syntax that leveraged CSS and default media markup:
https://ru.wikipedia.org/wiki/Шаблон:Панорама

Currently, I do not see a way to actually use TemplateStyles in there because the media structure has changed too much in comparison with the previous markup. I would appreciate information on whether my assessment is incorrect, since using default media markup brought benefits to the template.

Arlo is on vacation till end of week and we'll chat once he is back next week and we can provide some guidance.

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

[mediawiki/core@master] Deprecate wgParserEnableLegacyMediaDOM config

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

Change 952895 merged by jenkins-bot:

[mediawiki/core@master] Deprecate wgParserEnableLegacyMediaDOM config

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