From 14190c2b26d717e5c822c6b0bafd8127e9adc734 Mon Sep 17 00:00:00 2001
From: Philip Peitsch <philip.peitsch@gmail.com>
Date: Wed, 13 Aug 2014 17:35:58 +1000
Subject: [PATCH 1/3] Add test showing position stability issue near
 annotations

---
 webodf/tests/ops/OdtDocumentTests.js | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/webodf/tests/ops/OdtDocumentTests.js b/webodf/tests/ops/OdtDocumentTests.js
index 4f6f4a37d..5182b2189 100644
--- a/webodf/tests/ops/OdtDocumentTests.js
+++ b/webodf/tests/ops/OdtDocumentTests.js
@@ -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 *************** //

From 1586b9f36cd27d7a8cc900ee0dfab66074a20150 Mon Sep 17 00:00:00 2001
From: Philip Peitsch <philip.peitsch@gmail.com>
Date: Tue, 19 Aug 2014 19:41:43 +1000
Subject: [PATCH 2/3] Add additional tests showing position stability problems

---
 webodf/tests/ops/operationtests.xml | 38 +++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/webodf/tests/ops/operationtests.xml b/webodf/tests/ops/operationtests.xml
index 8cb328a4f..fd44a92da 100644
--- a/webodf/tests/ops/operationtests.xml
+++ b/webodf/tests/ops/operationtests.xml
@@ -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" isFailing="true">
+  <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>

From 9fba50ac6998ebce802c97020dfb339c429c5a80 Mon Sep 17 00:00:00 2001
From: Philip Peitsch <philip.peitsch@gmail.com>
Date: Fri, 19 Sep 2014 00:36:35 +1000
Subject: [PATCH 3/3] 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.
---
 webodf/lib/ops/TextPositionFilter.js | 24 ++++++++++++++++++------
 webodf/tests/ops/operationtests.xml  |  2 +-
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/webodf/lib/ops/TextPositionFilter.js b/webodf/lib/ops/TextPositionFilter.js
index 670857559..04a643509 100644
--- a/webodf/lib/ops/TextPositionFilter.js
+++ b/webodf/lib/ops/TextPositionFilter.js
@@ -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) {
@@ -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;
@@ -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;
     };
diff --git a/webodf/tests/ops/operationtests.xml b/webodf/tests/ops/operationtests.xml
index fd44a92da..bbe901e4b 100644
--- a/webodf/tests/ops/operationtests.xml
+++ b/webodf/tests/ops/operationtests.xml
@@ -2135,7 +2135,7 @@
     <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" isFailing="true">
+ <test name="RemoveText_AnnotatedRange_MaintainsCorrectCursorPosition">
   <before>
     <office:text><text:p>AB<html:span class="webodf-annotationHighlight"/>CD</text:p></office:text>
   </before>