Bug 961737 - getTextAtOffset is broken w.r.t. embedded object characters, part3 (list items), r=tbsaunde

accessible/src/generic/HyperTextAccessible.cpp

@@ -10,6 +10,7 @@
 #include "nsAccessibilityService.h"
 #include "nsIAccessibleTypes.h"
 #include "DocAccessible.h"
+#include "HTMLListAccessible.h"
 #include "Role.h"
 #include "States.h"
 #include "TextAttrs.h"
@@ -238,7 +239,7 @@ HyperTextAccessible::DOMPointToOffset(nsINode* aNode, int32_t aNodeOffset,
   if (!aNode)
     return 0;
 
-  uint32_t addTextOffset = 0;
+  uint32_t offset = 0;
   nsINode* findNode = nullptr;
 
   if (aNodeOffset == -1) {
@@ -251,7 +252,7 @@ HyperTextAccessible::DOMPointToOffset(nsINode* aNode, int32_t aNodeOffset,
     nsIFrame* frame = aNode->AsContent()->GetPrimaryFrame();
     NS_ENSURE_TRUE(frame, 0);
 
-    nsresult rv = ContentToRenderedOffset(frame, aNodeOffset, &addTextOffset);
+    nsresult rv = ContentToRenderedOffset(frame, aNodeOffset, &offset);
     NS_ENSURE_SUCCESS(rv, 0);
     // Get the child node and 
     findNode = aNode;
@@ -304,14 +305,20 @@ HyperTextAccessible::DOMPointToOffset(nsINode* aNode, int32_t aNodeOffset,
     descendant = GetFirstAvailableAccessible(findNode);
   }
 
-  // From the descendant, go up and get the immediate child of this hypertext
-  Accessible* childAtOffset = nullptr;
+  return TransformOffset(descendant, offset, aIsEndOffset);
+}
+
+int32_t
+HyperTextAccessible::TransformOffset(Accessible* aDescendant,
+                                     int32_t aOffset, bool aIsEndOffset) const
+{
+  // From the descendant, go up and get the immediate child of this hypertext.
+  int32_t offset = aOffset;
+  Accessible* descendant = aDescendant;
   while (descendant) {
     Accessible* parent = descendant->Parent();
-    if (parent == this) {
-      childAtOffset = descendant;
-      break;
-    }
+    if (parent == this)
+      return GetChildOffset(descendant) + offset;
 
     // This offset no longer applies because the passed-in text object is not
     // a child of the hypertext. This happens when there are nested hypertexts,
@@ -321,17 +328,16 @@ HyperTextAccessible::DOMPointToOffset(nsINode* aNode, int32_t aNodeOffset,
     // is not at 0 offset then the returned offset should be after an embedded
     // character the original point belongs to.
     if (aIsEndOffset)
-      addTextOffset = (addTextOffset > 0 || descendant->IndexInParent() > 0) ? 1 : 0;
+      offset = (offset > 0 || descendant->IndexInParent() > 0) ? 1 : 0;
     else
-      addTextOffset = 0;
+      offset = 0;
 
     descendant = parent;
   }
 
-  // If the given DOM point cannot be mapped into offset relative this hypertext
+  // If the given a11y point cannot be mapped into offset relative this hypertext
   // offset then return length as fallback value.
-  return childAtOffset ?
-    GetChildOffset(childAtOffset) + addTextOffset : CharacterCount();
+  return CharacterCount();
 }
 
 bool
@@ -420,6 +426,31 @@ HyperTextAccessible::FindOffset(int32_t aOffset, nsDirection aDirection,
       return -1;
 
     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.
+        if (aDirection == eDirPrevious)
+          return text != this ? TransformOffset(text, 0, false) : 0;
+
+        if (aAmount == eSelectEndLine || aAmount == eSelectLine) {
+          if (text != this)
+            return TransformOffset(text, 1, true);
+
+          // 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;
+        }
+
+        // Case of word and char boundaries.
+        return text != this ? TransformOffset(text, 1, true) : 1;
+      }
+    }
+
     innerOffset -= text->GetChildOffset(childIdx);
 
     text = child->AsHyperText();
@@ -463,10 +494,16 @@ HyperTextAccessible::FindOffset(int32_t aOffset, nsDirection aDirection,
                                              pos.mContentOffset,
                                              aDirection == eDirNext);
 
-  // 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() && aDirection == eDirPrevious)
-    return 0;
+  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;
+  }
 
   return hyperTextOffset;
 }

accessible/src/generic/HyperTextAccessible.h

@@ -120,6 +120,12 @@ public:
   int32_t DOMPointToOffset(nsINode* aNode, int32_t aNodeOffset,
                            bool aIsEndOffset = false) const;
 
+  /**
+   * Transform the given a11y point into the offset relative this hypertext.
+   */
+  int32_t TransformOffset(Accessible* aDescendant, int32_t aOffset,
+                          bool aIsEndOffset) const;
+
   /**
    * Convert start and end hypertext offsets into DOM range.
    *
@@ -435,9 +441,9 @@ protected:
    * Return an offset corresponding to the given direction and selection amount
    * relative the given offset. A helper used to find word or line boundaries.
    */
-  virtual int32_t FindOffset(int32_t aOffset, nsDirection aDirection,
-                             nsSelectionAmount aAmount,
-                             EWordMovementType aWordMovementType = eDefaultBehavior);
+  int32_t FindOffset(int32_t aOffset, nsDirection aDirection,
+                     nsSelectionAmount aAmount,
+                     EWordMovementType aWordMovementType = eDefaultBehavior);
 
   /**
    * Return the boundaries of the substring in case of textual frame or

accessible/src/html/HTMLListAccessible.cpp

@@ -98,33 +98,6 @@ HTMLLIAccessible::GetBounds(int32_t* aX, int32_t* aY,
   return NS_OK;
 }
 
-int32_t
-HTMLLIAccessible::FindOffset(int32_t aOffset, nsDirection aDirection,
-                             nsSelectionAmount aAmount,
-                             EWordMovementType aWordMovementType)
-{
-  Accessible* child = GetChildAtOffset(aOffset);
-  if (!child)
-    return -1;
-
-  if (child != mBullet) {
-    if (aDirection == eDirPrevious &&
-        (aAmount == eSelectBeginLine || aAmount == eSelectLine))
-      return 0;
-
-    return HyperTextAccessible::FindOffset(aOffset, aDirection,
-                                           aAmount, aWordMovementType);
-  }
-
-  if (aDirection == eDirPrevious)
-    return 0;
-
-  if (aAmount == eSelectEndLine || aAmount == eSelectLine)
-    return CharacterCount();
-
-  return nsAccUtils::TextLength(child);
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // HTMLLIAccessible: public
 

accessible/src/html/HTMLListAccessible.h

@@ -55,12 +55,8 @@ public:
   virtual a11y::role NativeRole();
   virtual uint64_t NativeState();
 
-  // HyperTextAccessible
-  virtual int32_t FindOffset(int32_t aOffset, nsDirection aDirection,
-                             nsSelectionAmount aAmount,
-                             EWordMovementType aWordMovementType) MOZ_OVERRIDE;
-
   // HTMLLIAccessible
+  HTMLListBulletAccessible* Bullet() const { return mBullet; }
   void UpdateBullet(bool aHasBullet);
 
 protected:

accessible/tests/mochitest/text/test_lineboundary.html

@@ -119,6 +119,18 @@
 
       testTextAtOffset([ "li1" ], BOUNDARY_LINE_START,
                        [ [ 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([ "ul1" ], BOUNDARY_LINE_START,
+                       [ [ 0, 0, kEmbedChar, 0, 1 ],
+                         [ 1, 1, kEmbedChar, 1, 2 ],
+                         [ 2, 2, kEmbedChar, 2, 3 ],
+                         [ 3, 4, kEmbedChar, 3, 4 ] ]);
 
       //////////////////////////////////////////////////////////////////////////
       // Nested hypertexts
@@ -193,8 +205,11 @@ two words
   <p id="ht_4">Hello world
 </p>
 
-  <ul>
+  <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>
   </ul>
 
   <div id="ht_5">
@@ -203,5 +218,6 @@ two words
       <p>seciofarus</p>
     </div>
   </div>
+
 </body>
 </html>