Page MenuHomePhabricator

VisualEditor adding nonsense code to file pages
Closed, ResolvedPublicBUG REPORT

Description

Steps to replicate the issue:

  • Open a file page on Commons using the VisualEditor.
  • Make any change to the page.
  • Apply the change.

What happens?:

The change is made but additionally a huge amount of nonsense HTML and Wikitext code is added.

Example: diff

Search for all pages where this is currently not reverted: search for "<mw:slotheader>mediainfo</mw:slotheader>"

What should have happened instead?:

The nonsense code should not become added.

Other information :

I created an AbuseFilter blocking such edits to prevent further damage until this is fixed. Special:AbuseFilter/297

Event Timeline

Pppery triaged this task as Unbreak Now! priority.Nov 11 2023, 9:43 PM

This broke when we rolled out https://gerrit.wikimedia.org/r/c/mediawiki/core/+/953342 about 3 weeks back. Somewhere in the code paths, we forgot to account for the fact that when VE needs to edit a page, it only needs to edit the "main" slot of a page with multiple MCR slots. File pages on commons have multiple slots and with that change, we seem to be giving VE the combined HTML from non-wikitext slots.

We should be able to roll out a fix tomorrow once we narrow down where to fix the API calls.

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

[mediawiki/core@master] Parsoid-VE-MCR hack: Always return main slot output if useParsoid is set

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

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

[mediawiki/core@wmf/1.42.0-wmf.4] Parsoid-VE-MCR hack: Always return main slot output if useParsoid is set

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

Change 973823 merged by jenkins-bot:

[mediawiki/core@master] Parsoid-VE-MCR hack: Always return main slot output if useParsoid is set

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

Change 973797 merged by jenkins-bot:

[mediawiki/core@wmf/1.42.0-wmf.4] Parsoid-VE-MCR hack: Always return main slot output if useParsoid is set

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

Mentioned in SAL (#wikimedia-operations) [2023-11-13T21:35:59Z] <urbanecm@deploy2002> Started scap: Backport for [[gerrit:969401|mobile: Add MobileUrlCallback (T257852)]], [[gerrit:973797|Parsoid-VE-MCR hack: Always return main slot output if useParsoid is set (T351026 T351113)]]

Mentioned in SAL (#wikimedia-operations) [2023-11-13T21:37:17Z] <urbanecm@deploy2002> urbanecm and ssastry and tgr: Backport for [[gerrit:969401|mobile: Add MobileUrlCallback (T257852)]], [[gerrit:973797|Parsoid-VE-MCR hack: Always return main slot output if useParsoid is set (T351026 T351113)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2023-11-13T21:54:33Z] <urbanecm@deploy2002> Finished scap: Backport for [[gerrit:969401|mobile: Add MobileUrlCallback (T257852)]], [[gerrit:973797|Parsoid-VE-MCR hack: Always return main slot output if useParsoid is set (T351026 T351113)]] (duration: 18m 34s)

ssastry claimed this task.

This is now fixed. But it uncovered an issue with the 'Review Changes' output in VE which now incorrectly shows that structured data information will be removed on save, but on save, there is no such deletion happening. We'll track and fix that separately.