Bug 1951833 part 1: Support line feed characters in TextLeafPoint::CharBounds. r=morgan

Previously, both literal line feed characters in pre-formatted text and HTMl <br> elements returned a rect with 0 width and/or height.
Because of the way CharBounds() was implemented, this also returned 0 for x and y.
This caused problems for clients such as Windows Text Cursor Indicator which need the rectangle for the character at the caret.
Now, we return the correct x and y coordinates.
We also return a minimum width and height of 1 to ensure clients treat it as an actual rectangle.

As part of this, CharBounds() has been refactored slightly for consistency and readability.
As a bonus, this also fixes character bounds for list item bullets (bug 360003), but a test for that will be added in a subsequent patch.

This patch also removes the special case line feed code added to TextLeafRange::WalkLineRects() in bug 1946552, since CharBounds() now handles this.

Differential Revision: https://phabricator.services.mozilla.com/D249709

accessible/base/TextLeafRange.cpp

@@ -2020,22 +2020,49 @@ TextLeafPoint TextLeafPoint::FindTextAttrsStart(nsDirection aDirection,
 }
 
 LayoutDeviceIntRect TextLeafPoint::CharBounds() {
-  if (mAcc && !mAcc->IsText()) {
-    // If we're dealing with an empty container, return the
-    // accessible's non-text bounds.
-    return mAcc->Bounds();
-  }
-
-  if (!mAcc || (mAcc->IsRemote() && !mAcc->AsRemote()->mCachedFields)) {
+  if (!mAcc) {
     return LayoutDeviceIntRect();
   }
 
-  if (LocalAccessible* local = mAcc->AsLocal()) {
-    if (!local->IsTextLeaf() || nsAccUtils::TextLength(local) == 0) {
-      // Empty content, use our own bounds to at least get x,y coordinates
-      return local->Bounds();
+  if (mAcc->IsHTMLBr()) {
+    // HTML <br> elements don't provide character bounds, but do provide text (a
+    // line feed). They also have 0 width and/or height, depending on the
+    // doctype and writing mode. Expose minimum 1 x 1 so clients treat it as an
+    // actual rectangle; e.g. when the caret is positioned on a <br>.
+    LayoutDeviceIntRect bounds = mAcc->Bounds();
+    if (bounds.width == 0) {
+      bounds.width = 1;
     }
+    if (bounds.height == 0) {
+      bounds.height = 1;
+    }
+    return bounds;
+  }
 
+  if (!mAcc->IsTextLeaf()) {
+    // This could be an empty container. Alternatively, it could be a list
+    // bullet,which does provide text but doesn't support character bounds. In
+    // either case, return the Accessible's bounds.
+    return mAcc->Bounds();
+  }
+
+  auto maybeAdjustLineFeedBounds = [this](LayoutDeviceIntRect& aBounds) {
+    if (!IsLineFeedChar()) {
+      return;
+    }
+    // Line feeds have a 0 width or height, depending on the writing mode.
+    // Use 1 instead so that clients treat it as an actual rectangle; e.g. when
+    // displaying the caret when it is positioned on a line feed.
+    MOZ_ASSERT(aBounds.IsZeroArea());
+    if (aBounds.width == 0) {
+      aBounds.width = 1;
+    }
+    if (aBounds.height == 0) {
+      aBounds.height = 1;
+    }
+  };
+
+  if (LocalAccessible* local = mAcc->AsLocal()) {
     if (mOffset >= 0 &&
         static_cast<uint32_t>(mOffset) >= nsAccUtils::TextLength(local)) {
       // It's valid for a caller to query the length because the caret might be
@@ -2062,6 +2089,7 @@ LayoutDeviceIntRect TextLeafPoint::CharBounds() {
     bounds.MoveBy(-orgRectPixels.X(), -orgRectPixels.Y());
     bounds.ScaleRoundOut(presContext->PresShell()->GetResolution());
     bounds.MoveBy(orgRectPixels.X(), orgRectPixels.Y());
+    maybeAdjustLineFeedBounds(bounds);
     return bounds;
   }
 
@@ -2069,9 +2097,18 @@ LayoutDeviceIntRect TextLeafPoint::CharBounds() {
     return LayoutDeviceIntRect();
   }
   RemoteAccessible* remote = mAcc->AsRemote();
+  if (!remote->mCachedFields) {
+    return LayoutDeviceIntRect();
+  }
+
   nsRect charBounds = remote->GetCachedCharRect(mOffset);
-  if (!charBounds.IsEmpty()) {
-    return remote->BoundsWithOffset(Some(charBounds));
+  // A character can have 0 width, but we still want its other coordinates.
+  // Thus, we explicitly test for an all-0 rect here to determine whether this
+  // is a valid char rect, rather than using IsZeroArea or IsEmpty.
+  if (!charBounds.IsEqualRect(0, 0, 0, 0)) {
+    LayoutDeviceIntRect bounds = remote->BoundsWithOffset(Some(charBounds));
+    maybeAdjustLineFeedBounds(bounds);
+    return bounds;
   }
 
   return LayoutDeviceIntRect();
@@ -2414,15 +2451,6 @@ bool TextLeafRange::WalkLineRects(LineRectCallback aCallback) const {
         nsIAccessibleText::BOUNDARY_CHAR, eDirPrevious);
     MOZ_ASSERT(currPoint <= lastPointInLine);
 
-    if (lastPointInLine != currPoint && lastPointInLine.IsLineFeedChar()) {
-      // The line feed character at the end of a line in pre-formatted text
-      // doesn't have a useful rect. Use the previous character. Otherwise,
-      // the rect we provide won't span the line of text and we'll miss
-      // characters.
-      lastPointInLine = lastPointInLine.FindBoundary(
-          nsIAccessibleText::BOUNDARY_CHAR, eDirPrevious);
-    }
-
     LayoutDeviceIntRect currLineRect = currPoint.CharBounds();
     currLineRect.UnionRect(currLineRect, lastPointInLine.CharBounds());
     // The range we pass must include the last character and range ends are

accessible/tests/browser/e10s/browser_caching_text_bounds.js

@@ -743,3 +743,82 @@ b</p>
   },
   { chrome: true, topLevel: true }
 );
+
+function getCharacterExtents(acc, offset) {
+  const x = {};
+  const y = {};
+  const w = {};
+  const h = {};
+  acc.getCharacterExtents(offset, x, y, w, h, COORDTYPE_SCREEN_RELATIVE);
+  return [x.value, y.value, w.value, h.value];
+}
+
+/**
+ * Test character bounds of line feed characters in a textarea.
+ */
+addAccessibleTask(
+  `
+<textarea id="textarea">a
+
+b</textarea>
+  `,
+  async function testLineFeedTextarea(browser, docAcc) {
+    // We can't use testChar because it doesn't know how to handle line feeds.
+    // We check relative to other characters instead.
+    const textarea = findAccessibleChildByID(docAcc, "textarea", [
+      nsIAccessibleText,
+    ]);
+    const [x0, y0, ,] = getCharacterExtents(textarea, 0);
+    const [x1, y1, w1, h1] = getCharacterExtents(textarea, 1);
+    const [x2, y2, w2, h2] = getCharacterExtents(textarea, 2);
+    const [x3, y3, ,] = getCharacterExtents(textarea, 3);
+    // Character 0 is a letter on the first line.
+    // Character 1 is a line feed at the end of the first line.
+    Assert.greater(x1, x0, "x1 > x0");
+    is(y1, y0, "y1 == y0");
+    Assert.greater(w1, 0, "w1 > 0");
+    Assert.greater(h1, 0, "h1 > 0");
+    // Character 2 is a line feed on a blank line.
+    is(x2, x0, "x2 == x0");
+    Assert.greaterOrEqual(y2, y1 + h1, "y2 >= y1 + h1");
+    Assert.greater(w2, 0, "w2 > 0");
+    Assert.greater(h2, 0, "h2 > 0");
+    // Character 3 is a letter on the final line.
+    is(x3, x0, "x3 == x0");
+    Assert.greaterOrEqual(y3, y2 + h2, "y3 >= y2 + h2");
+  },
+  { chrome: true, topLevel: true }
+);
+
+/**
+ * Test line feed characters in a contentEditable.
+ */
+addAccessibleTask(
+  `
+<div contenteditable role="textbox">
+  <div id="ce0">a</div>
+  <div id="ce1"><br></div>
+  <div id="ce2">b</div>
+</div>
+  `,
+  async function testLineFeedEditable(browser, docAcc) {
+    // We can't use testChar because it doesn't know how to handle line feeds.
+    // We check relative to other characters instead.
+    const ce0 = findAccessibleChildByID(docAcc, "ce0", [nsIAccessibleText]);
+    const [x0, y0, ,] = getCharacterExtents(ce0, 0);
+    const ce1 = findAccessibleChildByID(docAcc, "ce1", [nsIAccessibleText]);
+    const [x1, y1, w1, h1] = getCharacterExtents(ce1, 0);
+    const ce2 = findAccessibleChildByID(docAcc, "ce2", [nsIAccessibleText]);
+    const [x2, y2, ,] = getCharacterExtents(ce2, 0);
+    // Character 0 is a letter on the first line.
+    // Character 1 is a line feed on a blank line.
+    is(x1, x0, "x1 == x0");
+    Assert.greater(y1, y0, "y1 > y0");
+    Assert.greater(w1, 0, "w1 > 0");
+    Assert.greater(h1, 0, "h1 > 0");
+    // Character 2 is a letter on the final line.
+    is(x2, x0, "x2 == x0");
+    Assert.greaterOrEqual(y2, y1 + h1, "y2 >= y1 + h1");
+  },
+  { chrome: true, topLevel: true }
+);