Skip to content

Commit 9fba50a

Browse files
committed
Fix position instability around highlighted annotations
By checking only nodes that pass the supplied node filter, the position logic can safely ignore out-of-place cursors near highlighted annotations. Note, this can't re-use PositionIterator.previousNode, because the logic requires the previous sibling of the current container's previous sibling.
1 parent 1586b9f commit 9fba50a

2 files changed

Lines changed: 19 additions & 7 deletions

File tree

webodf/lib/ops/TextPositionFilter.js

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,27 @@ ops.TextPositionFilter = function TextPositionFilter() {
3636
/**@const*/FILTER_ACCEPT = core.PositionFilter.FilterResult.FILTER_ACCEPT,
3737
/**@const*/FILTER_REJECT = core.PositionFilter.FilterResult.FILTER_REJECT;
3838

39+
/**
40+
* Find the previous sibling of the specified node that passes the node filter.
41+
* @param {?Node} node
42+
* @param {!function(?Node):!number} nodeFilter
43+
* @return {?Node}
44+
*/
45+
function previousSibling(node, nodeFilter) {
46+
while (node && nodeFilter(node) !== FILTER_ACCEPT) {
47+
node = node.previousSibling;
48+
}
49+
return node;
50+
}
51+
3952
/**
4053
* @param {!Node} container
4154
* @param {?Node} leftNode
4255
* @param {?Node} rightNode
56+
* @param {!function(?Node):!number} nodeFilter
4357
* @return {!core.PositionFilter.FilterResult}
4458
*/
45-
function checkLeftRight(container, leftNode, rightNode) {
59+
function checkLeftRight(container, leftNode, rightNode, nodeFilter) {
4660
var r, firstPos, rightOfChar;
4761
// accept if there is a character immediately to the left
4862
if (leftNode) {
@@ -62,9 +76,7 @@ ops.TextPositionFilter = function TextPositionFilter() {
6276
return FILTER_ACCEPT;
6377
}
6478
} else {
65-
// Note, cant use OdfUtils.previousNode here as that function automatically dives to the previous
66-
// elements first child (if it has one)
67-
if (odfUtils.isInlineRoot(container.previousSibling) && odfUtils.isGroupingElement(container)) {
79+
if (odfUtils.isGroupingElement(container) && odfUtils.isInlineRoot(previousSibling(container.previousSibling, nodeFilter))) {
6880
// Move first position after inline root inside trailing grouping element (part 2)
6981
// Allow the first position inside the first grouping element trailing an annotation
7082
return FILTER_ACCEPT;
@@ -167,13 +179,13 @@ ops.TextPositionFilter = function TextPositionFilter() {
167179
leftNode = iterator.leftNode();
168180
rightNode = container;
169181
container = /**@type{!Node}*/(container.parentNode);
170-
r = checkLeftRight(container, leftNode, rightNode);
182+
r = checkLeftRight(container, leftNode, rightNode, iterator.getNodeFilter());
171183
} else if (!odfUtils.isGroupingElement(container)) {
172184
r = FILTER_REJECT;
173185
} else {
174186
leftNode = iterator.leftNode();
175187
rightNode = iterator.rightNode();
176-
r = checkLeftRight(container, leftNode, rightNode);
188+
r = checkLeftRight(container, leftNode, rightNode, iterator.getNodeFilter());
177189
}
178190
return r;
179191
};

webodf/tests/ops/operationtests.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2135,7 +2135,7 @@
21352135
<office:text><text:p>A<office:annotation office:name="a"><dc:creator e:memberid="Alice">Alice</dc:creator><dc:date>2013-08-05T12:34:07.061Z</dc:date><text:list><text:list-item><text:p/></text:list-item></text:list></office:annotation><text:span text:style-name="auto63350368_0"><c:anchor c:memberId="Alice"/>B<c:cursor c:memberId="Alice"/></text:span><office:annotation-end office:name="a"/>C</text:p></office:text>
21362136
</after>
21372137
</test>
2138-
<test name="RemoveText_AnnotatedRange_MaintainsCorrectCursorPosition" isFailing="true">
2138+
<test name="RemoveText_AnnotatedRange_MaintainsCorrectCursorPosition">
21392139
<before>
21402140
<office:text><text:p>AB<html:span class="webodf-annotationHighlight"/>CD</text:p></office:text>
21412141
</before>

0 commit comments

Comments
 (0)