Page MenuHomePhabricator

Re-applying transactions to internal nodes can throw exceptions
Closed, ResolvedPublic8 Story Points

Description

Create the following WT document:

<ref>'''B'''</ref>
<references/>

Run the following commands to delete the 'B' from the reference body:

var sm = ve.init.target.surface.model;
sm.change( ve.dm.Transaction.newFromRemoval( sm.documentModel, new ve.Range( 9, 10 ) ) );

Turn on 'pause on uncaught exceptions' then press undo, then redo. Observe that an exception is thrown because the ContentBranchNode has no root.

Event Timeline

Esanders created this task.May 12 2016, 1:12 PM
Restricted Application added subscribers: Zppix, Aklapper. · View Herald TranscriptMay 12 2016, 1:12 PM
Jdforrester-WMF triaged this task as High priority.May 12 2016, 5:50 PM
Jdforrester-WMF set the point value for this task to 8.
Elitre added a subscriber: Elitre.May 12 2016, 5:52 PM

Change 289831 had a related patch set uploaded (by Esanders):
Workaround for T135127: Generate replace transactions in MWTransclusionModel

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

Change 289831 merged by jenkins-bot:
Workaround for T135127: Generate replace transactions in MWTransclusionModel

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

Deskana lowered the priority of this task from High to Low.Sep 20 2017, 12:29 PM
Deskana added a subscriber: Deskana.

Lowering in priority since there is a workaround.

Deskana moved this task from To Triage to Freezer on the VisualEditor board.Sep 20 2017, 12:40 PM
matmarex closed this task as Resolved.Sep 5 2018, 8:55 PM
matmarex added a subscriber: matmarex.

This seems to be fixed, although I have no idea which patch fixed it. (I briefly tried looking for one, but I don't even know if it would be a patch in VE, VE-MW, Cite or Citoid.)

I can't reproduce the minimal test case from the description, and I can't reproduce with the Citoid reference "Convert" button after reverting the changes from https://gerrit.wikimedia.org/r/289831.

Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptSep 5 2018, 8:55 PM

Change 458296 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] ve.dm.MWTransclusionModel: Remove workaround for T135127

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

Change 458296 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] ve.dm.MWTransclusionModel: Remove workaround for T135127

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