Page MenuHomePhabricator
Create Task
Maniphest T305628

Make all media options use the 'last one wins' strategy
Open, MediumPublic
Actions

Description

From T216003#7836958,

This is a bit of a mess. So, the format (framed, thumbnail, etc) is first one wins. Dimensions (width, height) are last one wins. Horizontal and vertical alignment (left, right, etc) are first one wins. Caption is last one wins. The legacy parser and Parsoid agree on that. However, for any other media option, Parsoid is first one wins and the legacy parser is last one wins. This is where the discrepancy with upright originates.

https://github.com/wikimedia/mediawiki/blob/master/includes/parser/Parser.php#L5353-L5442

Parsoid has been linting bogus media options for quite some time so hopefully this duplication is now rare in practice and making a breaking change so this can all be consistent won't be too disruptive.

There are about 24k lints on enwiki,
https://en.wikipedia.org/wiki/Special:LintErrors/bogus-image-options

Details

SubjectRepoBranchLines +/-
Bump parsoid to 0.18.0-a6mediawiki/vendormaster+186 -73
Ensure the correct dimensions are applied to gallery linesmediawiki/services/parsoidmaster+24 -10
Customize query in gerrit

Related Objects

Event Timeline

Arlolra created this task.Apr 7 2022, 12:24 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 7 2022, 12:24 PM
Arlolra mentioned this in T216003: Linter fails to detect multiple "upright" parameters as a Bogus file option.Apr 7 2022, 12:25 PM
Arlolra triaged this task as Medium priority.Apr 7 2022, 12:28 PM
Arlolra moved this task from Needs Triage to Media Structure on the Parsoid board.
Arlolra moved this task from Backlog to Todo on the Parsoid-Read-Views (Phase 0 - Parsoid-Media-Structure) board.
ssastry subscribed.Apr 9 2022, 3:29 AM

If in Parsoid, we've implemented 'First option wins' all this while, why would we switch it to 'Last option wins' now, i.e. why not just make the consistency fix be 'first option wins'?

JJMC89 subscribed.Apr 9 2022, 4:01 AM

Last would be consistent with template duplicate parameters.

Arlolra added a comment.Apr 9 2022, 4:38 PM

If in Parsoid, we've implemented 'First option wins' all this while, why would we switch it to 'Last option wins' now, i.e. why not just make the consistency fix be 'first option wins'?

See T216003#7837073

• JMcLeod_WMF moved this task from Todo to Backlog on the Parsoid-Read-Views (Phase 0 - Parsoid-Media-Structure) board.May 16 2022, 3:43 PM
Arlolra mentioned this in T214601: Gallery media is erroneously using sub-typeof if format is supplied.Jun 9 2022, 9:13 PM
Jonesey95 subscribed.EditedJun 9 2022, 10:28 PM

This may be a duplicate of T264464. Note that making "last one wins" before flagging the undetected errors in that bug report doesn't give editors a chance to fix the problems before the software's behavior changes.

It is also related to T274382. It may be useful for the MW developers to come up with a project plan for making image processing match a spec instead of trying to make one little change at a time, some of which, like the change that led to T276675, cause new bugs. Linter could then be modified to match the spec so that editors can get their wikis ready for the updated code that matches the spec.

Arlolra added a comment.Jun 9 2022, 11:42 PM

This may be a duplicate of T264464. Note that making "last one wins" before flagging the undetected errors in that bug report doesn't give editors a chance to fix the problems before the software's behavior changes.

Parsoid already lints conflicts formats (frame, frameless, thumb, manualthumb),
https://github.com/wikimedia/parsoid/commit/9023f630aa10e87f972bda9c8f573868a47f8736

But I'll have to look T264464 because border isn't a format. I thought it just added a class.

Arlolra added a comment.Jun 9 2022, 11:55 PM

It is also related to T274382. It may be useful for the MW developers to come up with a project plan for making image processing match a spec instead of trying to make one little change at a time, some of which, like the change that led to T276675, cause new bugs. Linter could then be modified to match the spec so that editors can get their wikis ready for the updated code that matches the spec.

While I fully support the effort to have this specified, my short term goal is to try and reduce the delta as much as possible between Parsoid and the legacy parser's interpretation of the options. And then replace the legacy parser's output structure with Parsoid's.

Bdijkstra renamed this task from Make all media options last one wins to Make all media options use the 'last one wins' strategy.Jun 23 2022, 9:18 AM
Arlolra removed a project: Parsoid-Read-Views (Phase 0 - Parsoid-Media-Structure).Jul 26 2022, 11:20 PM
Arlolra mentioned this in T333907: Address semantic differences from enabling nativeGalleryEnabled.Apr 4 2023, 12:14 AM
gerritbot added a comment.Apr 19 2023, 3:59 PM

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

[mediawiki/services/parsoid@master] Ensure the correct dimensions are applied to gallery lines

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

gerritbot added a project: Patch-For-Review.Apr 19 2023, 3:59 PM
gerritbot added a comment.Apr 19 2023, 4:51 PM

Change 910040 merged by jenkins-bot:

[mediawiki/services/parsoid@master] Ensure the correct dimensions are applied to gallery lines

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

Maintenance_bot removed a project: Patch-For-Review.Apr 19 2023, 5:10 PM
gerritbot added a comment.Apr 24 2023, 8:43 AM

Change 911253 had a related patch set uploaded (by Isabelle Hurbain-Palatin; author: Isabelle Hurbain-Palatin):

[mediawiki/vendor@master] Bump parsoid to 0.18.0-a6

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

gerritbot added a project: Patch-For-Review.Apr 24 2023, 8:43 AM
gerritbot added a comment.Apr 24 2023, 1:08 PM

Change 911253 merged by jenkins-bot:

[mediawiki/vendor@master] Bump parsoid to 0.18.0-a6

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

Maintenance_bot removed a project: Patch-For-Review.Apr 24 2023, 1:17 PM
MSantos added a project: Content-Transform-Team-WIP.Apr 24 2023, 3:44 PM
MSantos moved this task from Backlog to To Deploy on the Content-Transform-Team-WIP board.
Arlolra removed a project: Content-Transform-Team-WIP.Apr 24 2023, 4:25 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL