Bug 1015550 - Line boundary is incorrect for multi-char bullet. r=surkov
authorXidorn Quan <quanxunzhen@gmail.com>
Tue, 27 May 2014 18:07:00 +0200
changeset 185249 fd6d8a361cee14240eebb7bc56149526cd879892
parent 185248 5db02d29ec3b74c6c8c2b232f052f14c1823d921
child 185250 98644e8a0e128dde6f80f71bc7fac91d240d6a09
push id26846
push usercbook@mozilla.com
push dateWed, 28 May 2014 12:22:26 +0000
treeherdermozilla-central@96838c90ebac [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssurkov
bugs1015550
milestone32.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1015550 - Line boundary is incorrect for multi-char bullet. r=surkov
accessible/src/generic/HyperTextAccessible.cpp
accessible/tests/mochitest/text/test_lineboundary.html
--- a/accessible/src/generic/HyperTextAccessible.cpp
+++ b/accessible/src/generic/HyperTextAccessible.cpp
@@ -424,16 +424,19 @@ HyperTextAccessible::OffsetToDOMPoint(in
     DOMPoint();
 }
 
 uint32_t
 HyperTextAccessible::FindOffset(uint32_t aOffset, nsDirection aDirection,
                                 nsSelectionAmount aAmount,
                                 EWordMovementType aWordMovementType)
 {
+  NS_ASSERTION(aDirection == eDirPrevious || aAmount != eSelectBeginLine,
+               "eSelectBeginLine should only be used with eDirPrevious");
+
   // Find a leaf accessible frame to start with. PeekOffset wants this.
   HyperTextAccessible* text = this;
   Accessible* child = nullptr;
   int32_t innerOffset = aOffset;
 
   do {
     int32_t childIdx = text->GetChildIndexAtOffset(innerOffset);
     NS_ASSERTION(childIdx != -1, "Bad in offset!");
@@ -442,32 +445,42 @@ HyperTextAccessible::FindOffset(uint32_t
 
     child = text->GetChildAt(childIdx);
 
     // HTML list items may need special processing because PeekOffset doesn't
     // work with list bullets.
     if (text->IsHTMLListItem()) {
       HTMLLIAccessible* li = text->AsHTMLListItem();
       if (child == li->Bullet()) {
-        // It works only when the bullet is one single char.
+        // XXX: the logic is broken for multichar bullets in moving by
+        // char/cluster/word cases.
+        if (text != this) {
+          return aDirection == eDirPrevious ?
+            TransformOffset(text, 0, false) :
+            TransformOffset(text, 1, true);
+        }
         if (aDirection == eDirPrevious)
-          return text != this ? TransformOffset(text, 0, false) : 0;
-
-        if (aAmount == eSelectEndLine || aAmount == eSelectLine) {
-          if (text != this)
-            return TransformOffset(text, 1, true);
+          return 0;
 
-          // Ask a text leaf next (if not empty) to the bullet for an offset
-          // since list item may be multiline.
-          return aOffset + 1 < CharacterCount() ?
-            FindOffset(aOffset + 1, aDirection, aAmount, aWordMovementType) : 1;
+        uint32_t nextOffset = GetChildOffset(1);
+        if (nextOffset == 0)
+          return 0;
+
+        switch (aAmount) {
+          case eSelectLine:
+          case eSelectEndLine:
+            // Ask a text leaf next (if not empty) to the bullet for an offset
+            // since list item may be multiline.
+            return nextOffset < CharacterCount() ?
+              FindOffset(nextOffset, aDirection, aAmount, aWordMovementType) :
+              nextOffset;
+
+          default:
+            return nextOffset;
         }
-
-        // Case of word and char boundaries.
-        return text != this ? TransformOffset(text, 1, true) : 1;
       }
     }
 
     innerOffset -= text->GetChildOffset(childIdx);
 
     text = child->AsHyperText();
   } while (text);
 
@@ -516,18 +529,22 @@ HyperTextAccessible::FindOffset(uint32_t
 
   if (aDirection == eDirPrevious) {
     // If we reached the end during search, this means we didn't find the DOM point
     // and we're actually at the start of the paragraph
     if (hyperTextOffset == CharacterCount())
       return 0;
 
     // PeekOffset stops right before bullet so return 0 to workaround it.
-    if (IsHTMLListItem() && aAmount == eSelectBeginLine && hyperTextOffset == 1)
-      return 0;
+    if (IsHTMLListItem() && aAmount == eSelectBeginLine &&
+        hyperTextOffset > 0) {
+      Accessible* prevOffsetChild = GetChildAtOffset(hyperTextOffset - 1);
+      if (prevOffsetChild == AsHTMLListItem()->Bullet())
+        return 0;
+    }
   }
 
   return hyperTextOffset;
 }
 
 uint32_t
 HyperTextAccessible::FindLineBoundary(uint32_t aOffset,
                                       EWhichLineBoundary aWhichLineBoundary)
--- a/accessible/tests/mochitest/text/test_lineboundary.html
+++ b/accessible/tests/mochitest/text/test_lineboundary.html
@@ -121,21 +121,44 @@
                        [ [ 0, 5, kDiscBulletChar + "Item", 0, 5 ] ]);
       testTextAtOffset([ "li2" ], BOUNDARY_LINE_START,
                        [ [ 0, 1, kDiscBulletChar, 0, 1 ] ]);
       testTextAtOffset([ "li3" ], BOUNDARY_LINE_START,
                        [ [ 0, 7, kDiscBulletChar + "a long ", 0, 8 ],
                          [ 8, 11, "and ", 8, 12 ] ]);
       testTextAtOffset([ "li4" ], BOUNDARY_LINE_START,
                        [ [ 0, 6, kDiscBulletChar + "a " + kEmbedChar + " c", 0, 6 ] ]);
+      testTextAtOffset([ "li5" ], BOUNDARY_LINE_START,
+                       [ [ 0, 1, kDiscBulletChar + "\n", 0, 2 ],
+                         [ 2, 6, "hello", 2, 7 ] ]);
       testTextAtOffset([ "ul1" ], BOUNDARY_LINE_START,
                        [ [ 0, 0, kEmbedChar, 0, 1 ],
                          [ 1, 1, kEmbedChar, 1, 2 ],
                          [ 2, 2, kEmbedChar, 2, 3 ],
-                         [ 3, 4, kEmbedChar, 3, 4 ] ]);
+                         [ 3, 3, kEmbedChar, 3, 4 ],
+                         [ 4, 5, kEmbedChar, 4, 5 ] ]);
+
+      testTextAtOffset([ "li6" ], BOUNDARY_LINE_START,
+                       [ [ 0, 6, "1.Item", 0, 6 ] ]);
+      testTextAtOffset([ "li7" ], BOUNDARY_LINE_START,
+                       [ [ 0, 2, "2.", 0, 2 ] ]);
+      testTextAtOffset([ "li8" ], BOUNDARY_LINE_START,
+                       [ [ 0, 8, "3.a long ", 0, 9 ],
+                         [ 9, 12, "and ", 9, 13 ] ]);
+      testTextAtOffset([ "li9" ], BOUNDARY_LINE_START,
+                       [ [ 0, 7, "4.a " + kEmbedChar + " c", 0, 7 ] ]);
+      testTextAtOffset([ "li10" ], BOUNDARY_LINE_START,
+                       [ [ 0, 2, "5.\n", 0, 3 ],
+                         [ 3, 7, "hello", 3, 8 ] ]);
+      testTextAtOffset([ "ol1" ], BOUNDARY_LINE_START,
+                       [ [ 0, 0, kEmbedChar, 0, 1 ],
+                         [ 1, 1, kEmbedChar, 1, 2 ],
+                         [ 2, 2, kEmbedChar, 2, 3 ],
+                         [ 3, 3, kEmbedChar, 3, 4 ],
+                         [ 4, 5, kEmbedChar, 4, 5 ] ]);
 
       //////////////////////////////////////////////////////////////////////////
       // Nested hypertexts
 
       testTextAtOffset(["ht_5" ], BOUNDARY_LINE_START,
                        [ [ 0, 0, kEmbedChar, 0, 1 ] ]);
 
       SimpleTest.finish();
@@ -205,18 +228,27 @@ two words
   <p id="ht_4">Hello world
 </p>
 
   <ul id="ul1">
     <li id="li1">Item</li>
     <li id="li2"></li>
     <li id="li3" style="width:10ex; font-family:monospace; font-size:10pt;">a long and winding road that lead me to your door</li>
     <li id="li4">a <a href=''>b</a> c</li>
+    <li id="li5"><br>hello</li>
   </ul>
 
+  <ol id="ol1">
+    <li id="li6">Item</li>
+    <li id="li7"></li>
+    <li id="li8" style="width:10ex; font-family:monospace; font-size:10pt;">a long and winding road that lead me to your door</li>
+    <li id="li9">a <a href=''>b</a> c</li>
+    <li id="li10"><br>hello</li>
+  </ol>
+
   <div id="ht_5">
     <div>
       <p>sectiounus</p>
       <p>seciofarus</p>
     </div>
   </div>
 
 </body>