Skip to content

Commit 556e58e

Browse files
peitschiepeitschie
committed
Merge pull request #824 from peitschie/position-stability
Fix position instability around highlighted annotation
2 parents b9044f8 + 9fba50a commit 556e58e

3 files changed

Lines changed: 58 additions & 6 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/OdtDocumentTests.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -624,6 +624,8 @@ ops.OdtDocumentTests = function OdtDocumentTests(runner) {
624624
'<text:p>|a|b|<office:annotation><text:list><text:list-item><text:p>|</text:p></text:list-item></text:list></office:annotation>|c|d|<office:annotation-end></office:annotation-end>1|2|</text:p>',
625625
'<text:p>|a|<office:annotation><text:list><text:list-item><text:p>|b|</text:p></text:list-item></text:list></office:annotation>|c|<office:annotation-end></office:annotation-end>1|2|</text:p>',
626626
'<text:p>|a|<office:annotation><text:list><text:list-item><text:p>|b|</text:p></text:list-item></text:list></office:annotation>|<office:annotation-end></office:annotation-end>1|2|</text:p>',
627+
'<text:p>|<office:annotation><text:list><text:list-item><text:p>|</text:p></text:list-item></text:list></office:annotation><text:span>|</text:span></text:p>',
628+
'<text:p>|<office:annotation><text:list><text:list-item><text:p>|</text:p></text:list-item></text:list></office:annotation><e:editinfo></e:editinfo><text:span>|</text:span></text:p>',
627629

628630

629631
// *************** Annotations with highlighting *************** //

‎webodf/tests/ops/operationtests.xml‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2119,4 +2119,42 @@
21192119
<office:text><text:p><text:span>A<c:anchor c:memberId="Joe"/></text:span><text:span text:style-name="auto63350368_0">B</text:span><text:span text:style-name="auto63350368_0">C<text:s> </text:s><text:s> </text:s>D<text:s> </text:s>E<c:cursor c:memberId="Joe"/></text:span><text:s> </text:s></text:p></office:text>
21202120
</after>
21212121
</test>
2122+
<test name="ApplyDirectStyling_AnnotatedRange_MaintainsCorrectCursorPosition" isFailing="true">
2123+
<before>
2124+
<office:text><text:p>ABC</text:p></office:text>
2125+
</before>
2126+
<ops>
2127+
<op optype="AddCursor" memberid="Alice"/>
2128+
<op optype="AddAnnotation" memberid="Alice" position="1" length="1" name="a" timestamp="1375706047061"/>
2129+
<op optype="MoveCursor" memberid="Alice" position="3" length="1"/>
2130+
<op optype="ApplyDirectStyling" memberid="Alice" position="3" length="1">
2131+
<setProperties><style:text-properties fo:font-weight="bold" /></setProperties>
2132+
</op>
2133+
</ops>
2134+
<after>
2135+
<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>
2136+
</after>
2137+
</test>
2138+
<test name="RemoveText_AnnotatedRange_MaintainsCorrectCursorPosition">
2139+
<before>
2140+
<office:text><text:p>AB<html:span class="webodf-annotationHighlight"/>CD</text:p></office:text>
2141+
</before>
2142+
<ops>
2143+
<op optype="AddCursor" memberid="Alice"/>
2144+
<op optype="AddAnnotation" memberid="Alice" position="3" length="0" name="b" timestamp="1375706047061"/>
2145+
<op optype="AddAnnotation" memberid="Alice" position="1" length="2" name="a" timestamp="1375706047061"/>
2146+
<op optype="ApplyDirectStyling" memberid="Alice" position="4" length="1">
2147+
<setProperties><style:text-properties fo:font-style="italic" /></setProperties>
2148+
</op>
2149+
<op optype="ApplyDirectStyling" memberid="Alice" position="3" length="2">
2150+
<setProperties><style:text-properties fo:font-weight="bold" /></setProperties>
2151+
</op>
2152+
<op optype="MoveCursor" memberid="Alice" position="3" length="2"/>
2153+
<op optype="RemoveText" memberid="Alice" position="3" length="2"/>
2154+
<op optype="InsertText" memberid="Alice" position="3" text="1" moveCursor="true"/>
2155+
</ops>
2156+
<after>
2157+
<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><html:span class="webodf-annotationHighlight">1<c:cursor c:memberId="Alice"/></html:span><office:annotation-end office:name="a"/><office:annotation office:name="b"><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>D</text:p></office:text>
2158+
</after>
2159+
</test>
21222160
</tests>

0 commit comments

Comments
 (0)