Skip to content

Commit

Permalink
Merge pull request #824 from peitschie/position-stability
Browse files Browse the repository at this point in the history 
Fix position instability around highlighted annotation
  • Loading branch information
@peitschie
peitschie committed Sep 30, 2014
2 parents b9044f8 + 9fba50a commit 556e58e
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 6 deletions.
24 changes: 18 additions & 6 deletions webodf/lib/ops/TextPositionFilter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,27 @@ ops.TextPositionFilter = function TextPositionFilter() {
/**@const*/FILTER_ACCEPT = core.PositionFilter.FilterResult.FILTER_ACCEPT,
/**@const*/FILTER_REJECT = core.PositionFilter.FilterResult.FILTER_REJECT;

/**
* Find the previous sibling of the specified node that passes the node filter.
* @param {?Node} node
* @param {!function(?Node):!number} nodeFilter
* @return {?Node}
*/
function previousSibling(node, nodeFilter) {
while (node && nodeFilter(node) !== FILTER_ACCEPT) {
node = node.previousSibling;
}
return node;
}

/**
* @param {!Node} container
* @param {?Node} leftNode
* @param {?Node} rightNode
* @param {!function(?Node):!number} nodeFilter
* @return {!core.PositionFilter.FilterResult}
*/
function checkLeftRight(container, leftNode, rightNode) {
function checkLeftRight(container, leftNode, rightNode, nodeFilter) {
var r, firstPos, rightOfChar;
// accept if there is a character immediately to the left
if (leftNode) {
Expand All @@ -62,9 +76,7 @@ ops.TextPositionFilter = function TextPositionFilter() {
return FILTER_ACCEPT;
}
} else {
// Note, cant use OdfUtils.previousNode here as that function automatically dives to the previous
// elements first child (if it has one)
if (odfUtils.isInlineRoot(container.previousSibling) && odfUtils.isGroupingElement(container)) {
if (odfUtils.isGroupingElement(container) && odfUtils.isInlineRoot(previousSibling(container.previousSibling, nodeFilter))) {
// Move first position after inline root inside trailing grouping element (part 2)
// Allow the first position inside the first grouping element trailing an annotation
return FILTER_ACCEPT;
Expand Down Expand Up @@ -167,13 +179,13 @@ ops.TextPositionFilter = function TextPositionFilter() {
leftNode = iterator.leftNode();
rightNode = container;
container = /**@type{!Node}*/(container.parentNode);
r = checkLeftRight(container, leftNode, rightNode);
r = checkLeftRight(container, leftNode, rightNode, iterator.getNodeFilter());
} else if (!odfUtils.isGroupingElement(container)) {
r = FILTER_REJECT;
} else {
leftNode = iterator.leftNode();
rightNode = iterator.rightNode();
r = checkLeftRight(container, leftNode, rightNode);
r = checkLeftRight(container, leftNode, rightNode, iterator.getNodeFilter());
}
return r;
};
Expand Down
2 changes: 2 additions & 0 deletions webodf/tests/ops/OdtDocumentTests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,8 @@ ops.OdtDocumentTests = function OdtDocumentTests(runner) {
'<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>',
'<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>',
'<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>',
'<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>',
'<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>',


// *************** Annotations with highlighting *************** //
Expand Down
38 changes: 38 additions & 0 deletions webodf/tests/ops/operationtests.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2119,4 +2119,42 @@
<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>
</after>
</test>
<test name="ApplyDirectStyling_AnnotatedRange_MaintainsCorrectCursorPosition" isFailing="true">
<before>
<office:text><text:p>ABC</text:p></office:text>
</before>
<ops>
<op optype="AddCursor" memberid="Alice"/>
<op optype="AddAnnotation" memberid="Alice" position="1" length="1" name="a" timestamp="1375706047061"/>
<op optype="MoveCursor" memberid="Alice" position="3" length="1"/>
<op optype="ApplyDirectStyling" memberid="Alice" position="3" length="1">
<setProperties><style:text-properties fo:font-weight="bold" /></setProperties>
</op>
</ops>
<after>
<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>
</after>
</test>
<test name="RemoveText_AnnotatedRange_MaintainsCorrectCursorPosition">
<before>
<office:text><text:p>AB<html:span class="webodf-annotationHighlight"/>CD</text:p></office:text>
</before>
<ops>
<op optype="AddCursor" memberid="Alice"/>
<op optype="AddAnnotation" memberid="Alice" position="3" length="0" name="b" timestamp="1375706047061"/>
<op optype="AddAnnotation" memberid="Alice" position="1" length="2" name="a" timestamp="1375706047061"/>
<op optype="ApplyDirectStyling" memberid="Alice" position="4" length="1">
<setProperties><style:text-properties fo:font-style="italic" /></setProperties>
</op>
<op optype="ApplyDirectStyling" memberid="Alice" position="3" length="2">
<setProperties><style:text-properties fo:font-weight="bold" /></setProperties>
</op>
<op optype="MoveCursor" memberid="Alice" position="3" length="2"/>
<op optype="RemoveText" memberid="Alice" position="3" length="2"/>
<op optype="InsertText" memberid="Alice" position="3" text="1" moveCursor="true"/>
</ops>
<after>
<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>
</after>
</test>
</tests>

0 comments on commit 556e58e

Please sign in to comment.