jenkins-bot has submitted this change and it was merged.
Change subject: Fix TextState#getChangeTransaction bug
......................................................................
Fix TextState#getChangeTransaction bug
Also document the algorithm more clearly
Bug: T141573
Change-Id: I3cfca36c289a6936810665233547d37ef504905f
---
M src/ce/ve.ce.TextState.js
M tests/ce/ve.ce.TextState.test.js
2 files changed, 52 insertions(+), 11 deletions(-)
Approvals:
Esanders: Looks good to me, approved
jenkins-bot: Verified
diff --git a/src/ce/ve.ce.TextState.js b/src/ce/ve.ce.TextState.js
index 1663f74..5aa464b 100644
--- a/src/ce/ve.ce.TextState.js
+++ b/src/ce/ve.ce.TextState.js
@@ -213,7 +213,11 @@
// During typical typing, there is a single changed chunk with matching
start/end chars.
textStart = 0;
textEnd = 0;
- if ( change.start < Math.min( oldChunks.length, newChunks.length ) ) {
+ if ( change.start + change.end < Math.min( oldChunks.length,
newChunks.length ) ) {
+ // Both oldChunks and newChunks include a changed chunk.
Therefore the first changed
+ // chunk of oldChunks and newChunks is respectively oldChunks[
change.start ] and
+ // newChunks[ change.start ] . If they have matching
annotations, then matching
+ // characters at their start are also part of the unchanged
start region.
if ( oldChunks[ change.start ].hasEqualElements( newChunks[
change.start ] ) ) {
oldChunk = oldChunks[ change.start ];
newChunk = newChunks[ change.start ];
@@ -226,19 +230,22 @@
textStart = i;
}
- if (
- change.end < Math.min( oldChunks.length,
newChunks.length ) &&
- oldChunks[ oldChunks.length - 1 - change.end
].hasEqualElements(
+ // Likewise, the last changed chunk of oldChunks and newChunks
is respectively
+ // oldChunks[ oldChunks.length - 1 - change.end ] and
+ // newChunks[ newChunks.length - 1 - change.end ] , and if they
have matching
+ // annotations, then matching characters at their end
potentially form part of
+ // the unchanged end region.
+ if ( oldChunks[ oldChunks.length - 1 - change.end
].hasEqualElements(
newChunks[ newChunks.length - 1 - change.end ]
- )
- ) {
+ ) ) {
oldChunk = oldChunks[ oldChunks.length - 1 - change.end
];
newChunk = newChunks[ newChunks.length - 1 - change.end
];
- // For oldChunks/newChunks/both, it's possible that
only one chunk
- // changed, in which case textStart has already eaten
into that chunk;
- // so take care not to overlap it. (For example, for
'ana'->'anna',
- // textStart will be 2 so we want to limit textEnd to
1, else the 'n'
- // of 'ana' will be counted twice).
+ // However, if only one chunk has changed in
oldChunks/newChunks, then
+ // oldChunk/newChunk is also the *first* changed chunk,
in which case
+ // textStart has already eaten into that chunk; so take
care not to
+ // overlap it. (For example, for 'ana'->'anna',
textStart will be 2 so
+ // we want to limit textEnd to 1, else the 'n' of 'ana'
will be counted
+ // twice).
iLen = Math.min(
oldChunk.text.length -
( change.start + change.end ===
oldChunks.length - 1 ? textStart : 0 ),
diff --git a/tests/ce/ve.ce.TextState.test.js b/tests/ce/ve.ce.TextState.test.js
index 593c987..57e3342 100644
--- a/tests/ce/ve.ce.TextState.test.js
+++ b/tests/ce/ve.ce.TextState.test.js
@@ -317,6 +317,40 @@
},
{ type: 'retain', length: 3 }
]
+ },
+ {
+ msg: 'Insert new chunk whose annotations match end
chunk\'s',
+ oldRawHtml: '<u>x</u>yz',
+ oldInnerHtml: '<u class="ve-ce-textStyleAnnotation
ve-ce-underlineAnnotation">x</u>yz',
+ newInnerHtml: '<u class="ve-ce-textStyleAnnotation
ve-ce-underlineAnnotation">x</u>y<u class="ve-ce-textStyleAnnotation
ve-ce-underlineAnnotation">w</u>yz',
+ operations: [
+ { type: 'retain', length: 2 },
+ {
+ type: 'replace',
+ remove: [],
+ insert: [ 'y', [ 'w', [ 0 ] ] ],
+ insertedDataOffset: 0,
+ insertedDataLength: 2
+ },
+ { type: 'retain', length: 5 }
+ ]
+ },
+ {
+ msg: 'Ambiguous insert with start and end both
identical to original',
+ oldRawHtml: 'ab',
+ oldInnerHtml: 'ab',
+ newInnerHtml: 'ab<u class="ve-ce-textStyleAnnotation
ve-ce-underlineAnnotation">x</u>ab',
+ operations: [
+ { type: 'retain', length: 3 },
+ {
+ type: 'replace',
+ remove: [],
+ insert: [ [ 'x', [ 0 ] ], 'a', 'b' ],
+ insertedDataOffset: 0,
+ insertedDataLength: 3
+ },
+ { type: 'retain', length: 3 }
+ ]
}
];
--
To view, visit https://gerrit.wikimedia.org/r/304135
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I3cfca36c289a6936810665233547d37ef504905f
Gerrit-PatchSet: 2
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Divec <da...@troi.org>
Gerrit-Reviewer: Esanders <esand...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
