Page MenuHomePhabricator

Parsoid clients should handle inline media HTML change from <figure-inline> to <span>
Closed, ResolvedPublic

Description

As part of the project to use Parsoid everywhere, we are bridging HTML output differences between Parsoid and core parser. One of these is in the HTML generated for media output.

As part of T251641: RFC: HTML element for inline media from wikitext, where we initially wanted to switch core output to <figure-inline> tag for inline media, after TechCom review and discussion, we decided to switch Parsoid back to <span> tag instead. The change in Parsoid will come with a version bump as well.

Before we roll out this change in Parsoid, we need all Parsoid clients to be prepared to handle either <span> or <figure-inline> tags for a brief period. Same with any necessary CSS selectors. Presumably, this will be a simple change.

I've created a common task and tagged all known Parsoid clients. Feel free to create a specific subtask for your own client, if necessary.

Event Timeline

ssastry created this task.Oct 21 2020, 2:36 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
ssastry renamed this task from Parsoid clients can handle inline media HTML change from <figure-inline> to <span> to Parsoid clients should handle inline media HTML change from <figure-inline> to <span>.Oct 21 2020, 2:36 PM
ssastry triaged this task as High priority.
ssastry updated the task description. (Show Details)

VE still has backwards compatibility for <span>, but is treating <figure-inline> as canonical (for new images). We can change this back.

Does this affect gallery output? I note that that uses figure-inline too.

Change 641765 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/VisualEditor@master] Switch back to using <span> for inline images

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

Does this affect gallery output? I note that that uses figure-inline too.

Yes, it does

Change 641800 had a related patch set uploaded (by Arlolra; owner: Arlolra):
[mediawiki/services/parsoid@master] Shore up serializing spans for inline media

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

Yes, it does

What are galleries switching to, figure or span? Has that been implemented yet, should we update our code now?

Change 641800 merged by jenkins-bot:
[mediawiki/services/parsoid@master] Shore up serializing spans for inline media

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

@ssastry, When VE updates the code, our frontend will function as expected, but we need to do some code changes and test updates in CX-cxserver , espcially when we do image adaptation and sanitization of machine translation. Do you have any timeline for deployment?

There are a series of code deployments to unify core and Parsoid output. The first of those is to change Parsoid output for inline media.

Ideally, we would like this first Parsoid change from 'figure-inline' -> 'span' for inline media to ride the next train (Dec 1 - 3), but it depends on whether all Parsoid clients can get their switchover code merged by then or not.

What are galleries switching to, figure or span? Has that been implemented yet, should we update our code now?

Sorry for being unclear. For the purpose of this task, we'd just like to do s/figure-inline/span/ everywhere. Since galleries are currently using <figure-inline>, they'd switch to using <span> too. That's what I glibly meant above.

Parsoid should already be backwards compatible with this change (we started with <span>s before T118520), so feel free to update your code to start producing them for new inline images whenever you're ready. But note that Parsoid won't be outputting <span>s until all the clients are ready to handle them (this task) so make sure your code continues to be backwards compatible in accepting <figure-inline> for the time being.

However, you bring up a good point in that the outcome of the RFC ( T251641#6212416 ) specified that galleries would switch to using <figure>.
I think that's a separate task from what's being asked here so I opened T268250 to see how we'd like to proceed there.

Change 644393 had a related patch set uploaded (by Subramanya Sastry; owner: Subramanya Sastry):
[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.13.0-a18

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

Change 644221 had a related patch set uploaded (by C. Scott Ananian; owner: Subramanya Sastry):
[mediawiki/vendor@wmf/1.36.0-wmf.20] Bump wikimedia/parsoid to 0.13.0-a18

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

Change 644393 merged by jenkins-bot:
[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.13.0-a18

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

Change 644221 merged by jenkins-bot:
[mediawiki/vendor@wmf/1.36.0-wmf.20] Bump wikimedia/parsoid to 0.13.0-a18

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

ppelberg moved this task from To Triage to Triaged on the VisualEditor board.Dec 1 2020, 4:14 PM

Change 645139 had a related patch set uploaded (by Arlolra; owner: Arlolra):
[mediawiki/extensions/Flow@master] Switch some BadImageRemover tests back to using spans

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

Arlolra added a comment.EditedDec 3 2020, 7:41 PM

It looks like StructuredDiscussions is backwards compatible for T173972. The above patch in T266143#6667623 just updates the test suite.

Change 645189 had a related patch set uploaded (by Arlolra; owner: Arlolra):
[mediawiki/services/mobileapps@master] [WIP] Switch back to spans for inline media

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

T266143#6668090 starts on looking at the changes Mobile-Content-Service made in T177301

Change 645139 merged by jenkins-bot:
[mediawiki/extensions/Flow@master] Switch some BadImageRemover tests back to using spans

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

Change 645189 merged by jenkins-bot:
[mediawiki/services/mobileapps@master] Switch back to spans for inline media

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

Change 647806 had a related patch set uploaded (by Arlolra; owner: Arlolra):
[mediawiki/extensions/VisualEditor@master] Preserve the passed in inline media tag name

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

Change 647860 had a related patch set uploaded (by Arlolra; owner: Arlolra):
[mediawiki/extensions/VisualEditor@master] [WIP] Preserve the passed in inline media tag name in gallery

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

Change 647806 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Preserve the passed in inline media tag name

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

Change 648353 had a related patch set uploaded (by Arlolra; owner: Arlolra):
[mediawiki/extensions/VisualEditor@master] Switch back to using <span> for gallery images

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

Change 641765 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Switch back to using <span> for inline images

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

Change 647860 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Preserve the passed in inline media tag name in gallery

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

Change 648353 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Switch back to using <span> for gallery images

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

Arlolra closed this task as Resolved.Dec 16 2020, 9:06 PM
Arlolra claimed this task.

It looks like this is just waiting on QA in T268309 so I'm going to close it but please reopen if that proves problematic.

Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptDec 16 2020, 9:08 PM