Page MenuHomePhabricator

Annotations don't merge properly, because their index hashes include original DOM element contents
Closed, ResolvedPublic8 Estimated Story Points

Description

Steps to reproduce: In VE standalone, edit <h1><i>abc</i>X<i>def</i><h1> , delete the 'X', then switch back to source mode .

Expected value: <h1><i>abcdef</i></h1>
Observed value: <h1><i>abc</i><i>def</i></h1>

This happens because an annotation data element generated by ve.dm.Converter contains an originalDomElementsIndex property, which is the index for the hashStore item that holds the DOM element from which it was generated. Currently, that index is generated using store.index( domElements, domElements.map( ve.getNodeHtml ).join( '' ) , which means the innerHTML content of the DOM element gets included in the hash string -- so <b>foo</b> indexes differently to <b>bar</b>.

Including the innerHTML content doesn't give any reliable guarantees of DOM element uniqueness (because a document can contain <b>foo</b>...<b>foo</b>). So unless I'm missing something, ve.dm.Converter should omit the innerHTML of a ve.dm.Annotation's DOM element from the hash string.

Event Timeline

Change 343458 had a related patch set uploaded (by Divec):
[VisualEditor/VisualEditor] When indexing originalDomElements for ve.dm.Annotations, disregard child nodes

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

Jdforrester-WMF set the point value for this task to 1.
Jdforrester-WMF moved this task from To Triage to TR1: Releases on the VisualEditor board.

Change 343458 merged by jenkins-bot:
[VisualEditor/VisualEditor] When indexing originalDomElements for ve.dm.Annotations, disregard child nodes

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

Jdforrester-WMF changed the point value for this task from 1 to 8.
Jdforrester-WMF removed a project: Patch-For-Review.

Change 344219 had a related patch set uploaded (by Jforrester):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (ce3618d86)

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

Change 344219 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (0cc3ded3d)

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