Page MenuHomePhabricator

TreeModifier: ensureNotText can return the wrong position
Closed, ResolvedPublic

Description

Steps to reproduce:

  1. In VE standalone, edit <p>abc<img src='x'>def</p> .
  2. Select just the image (so selected range is 4-6).
  3. Use the Bold action in the menu.

Expected behaviour:

The image gets annotated as bold.

Observed behaviour:

TreeModifier throws an exception with the following stacktrace:

TypeError: a.node.splice is not a function
    at Object.ve.dm.TreeModifier.static.applyTreeOperation (file:///home/ubuntu/src/ve/src/dm/ve.dm.TreeModifier.js:323:11)
    at Object.ve.dm.TreeModifier.static.applyTreeOperations (file:///home/ubuntu/src/ve/src/dm/ve.dm.TreeModifier.js:106:8)
    at VeDmTreeModifier.ve.dm.TreeModifier.process (file:///home/ubuntu/src/ve/src/dm/ve.dm.TreeModifier.js:365:26)
    at VeDmTransactionProcessor.ve.dm.TransactionProcessor.process (file:///home/ubuntu/src/ve/src/dm/ve.dm.TransactionProcessor.js:111:22)
    at VeDmDocument.ve.dm.Document.commit (file:///home/ubuntu/src/ve/src/dm/ve.dm.Document.js:341:65)
    at VeDmSurface.ve.dm.Surface.changeInternal (file:///home/ubuntu/src/ve/src/dm/ve.dm.Surface.js:950:25)
    at VeDmSurface.ve.dm.Surface.change (file:///home/ubuntu/src/ve/src/dm/ve.dm.Surface.js:918:7)
    at VeDmSurfaceFragment.ve.dm.SurfaceFragment.change (file:///home/ubuntu/src/ve/src/dm/ve.dm.SurfaceFragment.js:126:15)
    at VeDmSurfaceFragment.ve.dm.SurfaceFragment.annotateContent (file:///home/ubuntu/src/ve/src/dm/ve.dm.SurfaceFragment.js:767:7)
    at VeUiAnnotationAction.ve.ui.AnnotationAction.setInternal (file:///home/ubuntu/src/ve/src/ui/actions/ve.ui.AnnotationAction.js:154:11)

This is because when ensureNotText splits the text node, it returns the newly created text node, not the position between the two text nodes.

Event Timeline

Change 541066 had a related patch set uploaded (by Divec; owner: Divec):
[VisualEditor/VisualEditor@master] TreeModifier: return correct position in ensureNotText

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

Change 541066 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] TreeModifier: return correct position in ensureNotText

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

Change 541272 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (2c8e48194)

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

Change 541272 merged by Bartosz Dziewoński:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (2c8e48194)

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

Change 541296 had a related patch set uploaded (by Bartosz Dziewoński; owner: Divec):
[VisualEditor/VisualEditor@wmf/1.34.0-wmf.25] TreeModifier: return correct position in ensureNotText

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

Change 541300 had a related patch set uploaded (by Bartosz Dziewoński; owner: Divec):
[VisualEditor/VisualEditor@REL1_34] TreeModifier: return correct position in ensureNotText

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

Change 541303 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@wmf/1.34.0-wmf.25] Update VE core submodule to 2ffb699eb (TreeModifier fixes)

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

Change 541304 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@REL1_34] Update VE core submodule to 74bcce48e (TreeModifier fixes)

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

Change 541300 merged by jenkins-bot:
[VisualEditor/VisualEditor@REL1_34] TreeModifier: return correct position in ensureNotText

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

Change 541296 merged by jenkins-bot:
[VisualEditor/VisualEditor@wmf/1.34.0-wmf.25] TreeModifier: return correct position in ensureNotText

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

Change 541304 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@REL1_34] Update VE core submodule to 74bcce48e (TreeModifier fixes)

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

Change 541303 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@wmf/1.34.0-wmf.25] Update VE core submodule to 2ffb699eb (TreeModifier fixes)

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

Mentioned in SAL (#wikimedia-operations) [2019-10-07T18:25:08Z] <urbanecm@deploy1001> Synchronized php-1.34.0-wmf.25/extensions/VisualEditor/: SWAT: rEVED011b6ebd6286: rEVED11033b7d2ac4: Update VE core submodule to 2ffb699eb (TreeModifier fixes), T234489, T234742 + ve.ui.MWDefinedTransclusionContextItem: Fix handling of template names (T234817) (duration: 00m 53s)

matmarex added a subscriber: matmarex.

This problem is now fixed on all Wikimedia wikis.

(The problem also affected VE-MW, with wikitext like abc[[File:Example.jpg|20x20px]]def).