Page MenuHomePhabricator

Insertions with references too easily cause rebase conflicts
Closed, ResolvedPublic1 Story Points

Description

Steps to reproduce:

plainRemoval = new ve.dm.Transaction( [
    { type: 'replace', remove: [ 'x' ], insert: [] },
    { type: 'retain', length: 10 }
] );
insertionWithRef = new ve.dm.Transaction( [
    { type: 'replace', remove: [], insert: [ 'y' ] },
    { type: 'retain', length: 10 },
    { type: 'replace', remove: [], insert: [ 'z' ] },
    { type: 'retain', length: 1 }
] );
x = ve.dm.Change.static.rebaseTransactions( plainRemoval, insertionWithRef );

Desired behaviour: x is a successful rebase: the transactions 'obviously' don't conflict.
Observed behaviour: x is [ null, null ] - the transactions conflict.

But we can't just solve this algorithmically on the linearized form by allowing transactions of that "shape", because if we form tx3 from insertionWithRef by replacing 'y' and 'z' with <p> and </o> respectively, then we need the rebase to fail, else plainRemoval.concatRebased(tx3).reversed() is a transaction that removes paragraph tags then adds text where they used to be, which violates VE HTML validity. This shows we need the "entirely before or entirely after" property for rebasing: it is what guarantees the tree branch context at each replace operation cannot be changed by rebasing.

Therefore, we may need to change the use of newFromDocumentInsertion so that references are inserted in a separate transaction.

Event Timeline

dchan created this task.Sep 5 2017, 7:04 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 5 2017, 7:04 PM

Change 376077 had a related patch set uploaded (by Divec; owner: Divec):
[mediawiki/extensions/ContentTranslation@master] Fix crashes when placeholder replacement contains references

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

dchan renamed this task from Insertions with references too easily causes rebase conflicts to Insertions with references too easily cause rebase conflicts.Sep 5 2017, 7:12 PM
dchan updated the task description. (Show Details)
Deskana triaged this task as Normal priority.Sep 5 2017, 7:15 PM
Deskana moved this task from To Triage to TR0: Interrupt on the VisualEditor board.

Change 376077 merged by jenkins-bot:
[mediawiki/extensions/ContentTranslation@master] Fix crashes when placeholder replacement contains references

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

Deskana closed this task as Resolved.Sep 6 2017, 9:55 AM
Deskana set the point value for this task to 1.
Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptSep 6 2017, 9:55 AM
This comment was removed by Esanders.