Merge pull request #879 from kossebau/addAnnotationWith0Length

In OpAddAnnotation support annotated ranges with 0 length
webodf · Mar 26, 2015 · d91660b · d91660b
2 parents ca7d2e6 + 1d24d46
commit d91660b
Show file tree

Hide file tree

Showing 5 changed files with 93 additions and 25 deletions.
diff --git a/ChangeLog.md b/ChangeLog.md
@@ -6,6 +6,18 @@
 
 * Fix breaking all empty annotations on merging the paragraph they are contained in with the one before ([#877](https://github.com/kogmbh/WebODF/pull/877)))
 
+### Improvements
+
+* In OpAddAnnotation support annotated ranges with 0 length ([#879](https://github.com/kogmbh/WebODF/pull/879)))
+
+### Breaking changes
+
+* OpAddAnnotation spec changed: length=0 no longer means unranged annotation, but a range of 0 length. For unranged annotations now use length=undefined.
+
+
+## Wodo.TextEditor
+See also section about WebODF
+
 
 # Changes between 0.5.5 and 0.5.6
 

diff --git a/webodf/lib/gui/AnnotationController.js b/webodf/lib/gui/AnnotationController.js
@@ -109,8 +109,12 @@ gui.AnnotationController = function AnnotationController(session, sessionConstra
             return;
         }
 
-        position = length >= 0 ? position : position + length;
-        length = Math.abs(length);
+        if (length === 0) {
+            length = undefined;
+        } else {
+            position = length >= 0 ? position : position + length;
+            length = Math.abs(length);
+        }
 
         op.init({
             memberid: inputMemberId,

diff --git a/webodf/lib/ops/OpAddAnnotation.js b/webodf/lib/ops/OpAddAnnotation.js
@@ -34,7 +34,7 @@ ops.OpAddAnnotation = function OpAddAnnotation() {
     var memberid, timestamp,
         /**@type{number}*/
         position,
-        /**@type{number}*/
+        /**@type{!number|undefined}*/
         length,
         /**@type{string}*/
         name,
@@ -48,7 +48,7 @@ ops.OpAddAnnotation = function OpAddAnnotation() {
         memberid = data.memberid;
         timestamp = parseInt(data.timestamp, 10);
         position = parseInt(data.position, 10);
-        length = parseInt(data.length, 10) || 0;
+        length = (data.length !== undefined) ? (parseInt(data.length, 10) || 0) : undefined;
         name = data.name;
     };
 
@@ -149,7 +149,7 @@ ops.OpAddAnnotation = function OpAddAnnotation() {
 
         annotation = createAnnotationNode(odtDocument, new Date(timestamp));
 
-        if (length) {
+        if (length !== undefined) {
             annotationEnd = createAnnotationEnd();
             // link annotation end to start
             annotation.annotationEndElement = annotationEnd;
@@ -197,15 +197,15 @@ ops.OpAddAnnotation = function OpAddAnnotation() {
     memberid:string,
     timestamp:number,
     position:number,
-    length:number,
+    length:(!number|undefined),
     name:string
 }}*/
 ops.OpAddAnnotation.Spec;
 /**@typedef{{
     memberid:string,
     timestamp:(number|undefined),
     position:number,
-    length:number,
+    length:(!number|undefined),
     name:string
 }}*/
 ops.OpAddAnnotation.InitSpec;
diff --git a/webodf/tests/ops/OperationTests.js b/webodf/tests/ops/OperationTests.js
@@ -437,6 +437,18 @@ ops.OperationTests = function OperationTests(runner) {
     };
 
     /*jslint emptyblock: true*/
+    function linkAnnotationEndToStart() {
+            return {
+                setUp: function () {
+                    var rootElement = t.odfContainer.rootElement,
+                        annotation = rootElement.getElementsByTagNameNS(odf.Namespaces.officens, "annotation")[0],
+                        annotationEnd = rootElement.getElementsByTagNameNS(odf.Namespaces.officens, "annotation-end")[0];
+                    annotation.annotationEndElement = annotationEnd;
+                },
+                tearDown: function () {}
+            };
+    }
+
     this.setUps = {
         "ApplyDirectStyling_FixesCursorPositions" : function () {
             // Test specifically requires the cursor node to have a child element of some sort to
@@ -450,17 +462,8 @@ ops.OperationTests = function OperationTests(runner) {
                 tearDown: function () {t.odtDocument.unsubscribe(ops.Document.signalCursorAdded, appendToCursor); }
             };
         },
-        "RemoveAnnotation_ranged" : function () {
-            return {
-                setUp: function () {
-                    var rootElement = t.odfContainer.rootElement,
-                        annotation = rootElement.getElementsByTagNameNS(odf.Namespaces.officens, "annotation")[0],
-                        annotationEnd = rootElement.getElementsByTagNameNS(odf.Namespaces.officens, "annotation-end")[0];
-                    annotation.annotationEndElement = annotationEnd;
-                },
-                tearDown: function () {}
-            };
-        },
+        "RemoveAnnotation_ranged" : linkAnnotationEndToStart,
+        "RemoveAnnotation_rangedZero" : linkAnnotationEndToStart,
         "RemoveText_CopesWithEmptyTextNodes" : function () {
             return {
                 setUp: function () {

diff --git a/webodf/tests/ops/operationtests.xml b/webodf/tests/ops/operationtests.xml
@@ -1421,7 +1421,7 @@
    <office:text><text:p>ABCD</text:p></office:text>
   </before>
   <ops>
-   <op optype="AddAnnotation" memberid="Alice" position="2" length="0" name="alice_1" timestamp="1375706047061">
+   <op optype="AddAnnotation" memberid="Alice" position="2" name="alice_1" timestamp="1375706047061">
    </op>
   </ops>
   <after>
@@ -1447,7 +1447,7 @@
     <setProperties fullName="John Doe">
     </setProperties>
    </op>
-   <op optype="AddAnnotation" memberid="John" position="2" length="0" name="alice_1" timestamp="1375706047061">
+   <op optype="AddAnnotation" memberid="John" position="2" name="alice_1" timestamp="1375706047061">
    </op>
   </ops>
   <after>
@@ -1473,7 +1473,7 @@
     <setProperties fullName="John Doe">
     </setProperties>
    </op>
-   <op optype="AddAnnotation" memberid="John" position="2" length="0" name="alice_1" timestamp="1375706047061">
+   <op optype="AddAnnotation" memberid="John" position="2" name="alice_1" timestamp="1375706047061">
    </op>
    <op optype="UpdateMember" memberid="John">
     <setProperties fullName="Don Joe">
@@ -1520,13 +1520,39 @@
    </office:text>
   </after>
  </test>
+ <test name="AddAnnotation_rangedZero">
+  <before>
+   <office:text>
+    <text:p>ABCD</text:p>
+    <text:p>EFG<text:span>HIJ</text:span></text:p>
+   </office:text>
+  </before>
+  <ops>
+   <op optype="AddAnnotation" memberid="Alice" position="3" length="0" name="alice_2" timestamp="1375706047061">
+   </op>
+  </ops>
+  <after>
+   <office:text>
+    <text:p>ABC<office:annotation office:name="alice_2">
+     <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:p>
+      </text:list-item>
+     </text:list>
+    </office:annotation><office:annotation-end office:name="alice_2"/>D</text:p>
+    <text:p>EFG<text:span>HIJ</text:span></text:p>
+   </office:text>
+  </after>
+ </test>
  <test name="AddAnnotation_rangedMultiple">
   <before>
    <office:text><text:p>ABCD</text:p><text:p>EFG<text:span>HIJ</text:span></text:p></office:text>
   </before>
   <ops>
    <op optype="AddAnnotation" memberid="Alice" position="3" length="6" name="alice_1" timestamp="1375706047061"/>
-   <op optype="AddAnnotation" memberid="Alice" position="3" length="0" name="alice_2" timestamp="1375706047061"/>
+   <op optype="AddAnnotation" memberid="Alice" position="3" name="alice_2" timestamp="1375706047061"/>
   </ops>
   <after>
    <office:text>
@@ -1573,6 +1599,29 @@
    <office:text><text:p>ABCD</text:p><text:p>EFG<text:span>HIJ</text:span></text:p></office:text>
   </after>
  </test>
+ <test name="RemoveAnnotation_rangedZero" hasSetup="true">
+  <before>
+   <office:text>
+    <text:p>ABC<office:annotation office:name="alice_2">
+     <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>xyz</text:p>
+      </text:list-item>
+     </text:list>
+    </office:annotation><office:annotation-end office:name="alice_2"/>D</text:p>
+    <text:p>EFG<text:span>HIJ</text:span></text:p>
+   </office:text>
+  </before>
+  <ops>
+   <op optype="RemoveAnnotation" memberid="Alice" position="4" length="3">
+   </op>
+  </ops>
+  <after>
+   <office:text><text:p>ABCD</text:p><text:p>EFG<text:span>HIJ</text:span></text:p></office:text>
+  </after>
+ </test>
  <test name="RemoveAnnotation_simple">
   <before>
    <office:text>
@@ -1702,7 +1751,7 @@
    <op optype="MoveCursor" memberid="Bob" position="3"/>
    <op optype="AddCursor" memberid="Eve"/>
    <op optype="MoveCursor" memberid="Eve" position="3"/>
-   <op optype="AddAnnotation" memberid="Alice" position="3" length="0" name="alice_1" timestamp="1375706047061"/>
+   <op optype="AddAnnotation" memberid="Alice" position="3" name="alice_1" timestamp="1375706047061"/>
   </ops>
   <after>
    <office:text>
@@ -1725,7 +1774,7 @@
    <op optype="MoveCursor" memberid="Alice" position="3"/>
    <op optype="AddCursor" memberid="Bob"/>
    <op optype="MoveCursor" memberid="Bob" position="3"/>
-   <op optype="AddAnnotation" memberid="Alice" position="3" length="0" name="alice_1" timestamp="1375706047061"/>
+   <op optype="AddAnnotation" memberid="Alice" position="3" name="alice_1" timestamp="1375706047061"/>
    <op optype="AddCursor" memberid="Eve"/>
    <op optype="MoveCursor" memberid="Eve" position="3"/>
   </ops>
@@ -2164,7 +2213,7 @@
   </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="3" 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>