| Quiddity | |
| May 15 2024, 6:14 PM |
| F53971708: Screenshot 2024-05-20 at 17.59.23.png | |
| May 20 2024, 5:07 PM |
| F53362157: image.png | |
| May 15 2024, 6:14 PM |
Steps to replicate the issue:
What happens?:
What should have happened instead?:
I can reproduce: https://en.wikipedia.org/w/index.php?title=User%3APPelberg_%28WMF%29%2FSandbox&diff=1224018033&oldid=1224017935
(Great spot and thank you for filing, @Quiddity)
That file only handles the rendering of the ContentEditable (CE) table node.
The problem here is how we are observing changes in the surface in general, and generating transactions.
Change #1032418 had a related patch set uploaded (by Esanders; author: Esanders):
[VisualEditor/VisualEditor@master] Failing test case for table cell conversion
Looks like our MutationObserver code can't handle more complex removal with nested nodes. Another way to trigger this bug is to try and convert a bullet list to a numbered list. The whole list is deleted.
Change #1032431 had a related patch set uploaded (by Esanders; author: Esanders):
[VisualEditor/VisualEditor@master] Revert "Use mutation observer to detect structural deletions"
Change #1032431 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] Disable MutationObserver in ve.ce.Surface
Change #1032516 had a related patch set uploaded (by Esanders; author: Esanders):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (27296e0e3)
Change #1032516 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (27296e0e3)
Change #1032571 had a related patch set uploaded (by Esanders; author: Esanders):
[mediawiki/extensions/VisualEditor@wmf/1.43.0-wmf.5] Update VE core submodule to master (27296e0e3)
Change #1032571 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@wmf/1.43.0-wmf.5] Update VE core submodule to master (27296e0e3)
Mentioned in SAL (#wikimedia-operations) [2024-05-16T21:06:07Z] <jsn@deploy1002> Started scap: Backport for [[gerrit:1032571|Update VE core submodule to master (27296e0e3) (T230323 T365052)]]
Mentioned in SAL (#wikimedia-operations) [2024-05-16T21:08:49Z] <jsn@deploy1002> jsn and esanders: Backport for [[gerrit:1032571|Update VE core submodule to master (27296e0e3) (T230323 T365052)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)
Mentioned in SAL (#wikimedia-operations) [2024-05-16T21:31:18Z] <jsn@deploy1002> Finished scap: Backport for [[gerrit:1032571|Update VE core submodule to master (27296e0e3) (T230323 T365052)]] (duration: 25m 10s)
This happens when the transaction swaps out the DOM node (from <td> to <th>), but only changes an attribute on the DM tableCell node (from style=data to style=header).
The same happens when changing a bullet list to a numbered list: the DOM node gets swapped out (from <ul> to <ol>), but the DM node only gets an attribute change (from style=bullet to style=number).
The mutation observer sees the removal of the <p> inside the <td> and deletes the corresponding DM paragraph, or sees the removal of the <li> inside the <ul> and deletes the corresponding DM listItem. It does not realise the nodes are being reused.
Change #1033718 had a related patch set uploaded (by Divec; author: Divec):
[VisualEditor/VisualEditor@master] Make ve.ce.Surface#afterMutations understand updateTagName
@Esanders, Is the double modal an expected experience?
Great spot, @AEakinloose. Per today's discussion. Behavior you spotted is expected even if not ideal.
Potential solve (out of scope for this ticket) @nayoub shared: add some kind of "global" affordance so that row/cell specific context item and global table context items are not shown simultaneously.
Change #1032418 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] Run TableAction/ListAction tests with a full view
Change #1035815 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (e3298c4a5)
Change #1035815 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (e3298c4a5)
Change #1033718 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] Make ve.ce.Surface#afterMutations understand updateTagName
Change #1051174 had a related patch set uploaded (by DLynch; author: DLynch):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (84477b4b9)
Change #1051174 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (84477b4b9)
