Bug 1286464 part.18 ContentEventHandler::OnQueryTextRectArray() should compute line breaker's rect with the last character's node when the start of queried range is a line breaker and it's caused by a block frame r?smaug draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 08 Aug 2016 19:05:35 +0900
changeset 398652 b50a88bb0b46cd08c7b54461e0cc70c9b8063811
parent 398651 876f1008a691ac59f3decd966d004e718db04cf2
child 398653 e5b7ceb6f27e8ebee9136e6a14a9ed629ff116cf
push id25590
push usermasayuki@d-toybox.com
push dateTue, 09 Aug 2016 15:40:38 +0000
reviewerssmaug
bugs1286464
milestone51.0a1
Bug 1286464 part.18 ContentEventHandler::OnQueryTextRectArray() should compute line breaker's rect with the last character's node when the start of queried range is a line breaker and it's caused by a block frame r?smaug Similar to OnQueryTextRect(), OnQueryTextRectArray() should compute a line breaker's rect with the last character when it's the first character in queried range and not caused by <br> frame. MozReview-Commit-ID: CGDHbcrc6zB
dom/events/ContentEventHandler.cpp
dom/events/ContentEventHandler.h
--- a/dom/events/ContentEventHandler.cpp
+++ b/dom/events/ContentEventHandler.cpp
@@ -1683,16 +1683,50 @@ ContentEventHandler::GetLineBreakerRectB
       // left of top-left corner of aFrame.
       result.mRect.x = -mPresContext->AppUnitsPerDevPixel();
       result.mRect.y = 0;
     }
   }
   return result;
 }
 
+ContentEventHandler::FrameRelativeRect
+ContentEventHandler::GuessLineBreakerRectAfter(nsIContent* aTextContent)
+{
+  // aTextContent should be a text node.
+  MOZ_ASSERT(aTextContent->IsNodeOfType(nsINode::eTEXT));
+
+  FrameRelativeRect result;
+  int32_t length = static_cast<int32_t>(aTextContent->Length());
+  if (NS_WARN_IF(length < 0)) {
+    return result;
+  }
+  // Get the last nsTextFrame which is caused by aTextContent.  Note that
+  // a text node can cause multiple text frames, e.g., the text is too long
+  // and wrapped by its parent block or the text has line breakers and its
+  // white-space property respects the line breakers (e.g., |pre|).
+  nsIFrame* lastTextFrame = nullptr;
+  nsresult rv = GetFrameForTextRect(aTextContent, length, true, &lastTextFrame);
+  if (NS_WARN_IF(NS_FAILED(rv)) || NS_WARN_IF(!lastTextFrame)) {
+    return result;
+  }
+  const nsRect kLastTextFrameRect = lastTextFrame->GetRect();
+  if (lastTextFrame->GetWritingMode().IsVertical()) {
+    // Below of the last text frame.
+    result.mRect.SetRect(0, kLastTextFrameRect.height,
+                         kLastTextFrameRect.width, 0);
+  } else {
+    // Right of the last text frame (not bidi-aware).
+    result.mRect.SetRect(kLastTextFrameRect.width, 0,
+                         0, kLastTextFrameRect.height);
+  }
+  result.mBaseFrame = lastTextFrame;
+  return result;
+}
+
 nsresult
 ContentEventHandler::OnQueryTextRectArray(WidgetQueryContentEvent* aEvent)
 {
   nsresult rv = Init(aEvent);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
@@ -1703,18 +1737,19 @@ ContentEventHandler::OnQueryTextRectArra
 
   bool isVertical = false;
   LayoutDeviceIntRect rect;
   uint32_t offset = aEvent->mInput.mOffset;
   const uint32_t kEndOffset = offset + aEvent->mInput.mLength;
   bool wasLineBreaker = false;
   nsRect lastCharRect;
   while (offset < kEndOffset) {
+    nsCOMPtr<nsIContent> lastTextContent;
     rv = SetRangeFromFlatTextOffset(range, offset, 1, lineBreakType, true,
-                                    nullptr);
+                                    nullptr, getter_AddRefs(lastTextContent));
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
 
     // If the range is collapsed, offset has already reached the end of the
     // contents.
     if (range->Collapsed()) {
       break;
@@ -1744,62 +1779,19 @@ ContentEventHandler::OnQueryTextRectArra
 
     bool startsBetweenLineBreaker = false;
     nsAutoString chars;
     // XXX not bidi-aware this class...
     isVertical = firstFrame->GetWritingMode().IsVertical();
 
     AutoTArray<nsRect, 16> charRects;
 
-    if (ShouldBreakLineBefore(firstContent, mRootContent) ||
-        IsMozBR(firstContent)) {
-      nsRect brRect;
-      // If the frame is <br> frame, we can always trust
-      // GetLineBreakerRectBefore().  Otherwise, "after" the last character
-      // rect is better than its result.
-      // TODO: We need to look for the last character rect if non-br frame
-      //       causes a line break at first time of this loop.
-      if (firstFrame->GetType() == nsGkAtoms::brFrame ||
-          aEvent->mInput.mOffset == offset) {
-        FrameRelativeRect relativeBRRect = GetLineBreakerRectBefore(firstFrame);
-        brRect = relativeBRRect.RectRelativeTo(firstFrame);
-      } else {
-        // The frame position in the root widget will be added in the
-        // following for loop but we need the rect in the previous frame.
-        // So, we need to avoid using current frame position.
-        brRect = lastCharRect - frameRect.TopLeft();
-        if (!wasLineBreaker) {
-          if (isVertical) {
-            // Right of the last character.
-            brRect.y = brRect.YMost() + 1;
-            brRect.height = 1;
-          } else {
-            // Under the last character.
-            brRect.x = brRect.XMost() + 1;
-            brRect.width = 1;
-          }
-        }
-      }
-      charRects.AppendElement(brRect);
-      chars.AssignLiteral("\n");
-      if (kBRLength > 1 && offset == aEvent->mInput.mOffset && offset) {
-        // If the first frame for the previous offset of the query range and
-        // the first frame for the start of query range are same, that means
-        // the start offset is between the first line breaker (i.e., the range
-        // starts between "\r" and "\n").
-        rv = SetRangeFromFlatTextOffset(range, aEvent->mInput.mOffset - 1, 1,
-                                        lineBreakType, true, nullptr);
-        if (NS_WARN_IF(NS_FAILED(rv))) {
-          return NS_ERROR_UNEXPECTED;
-        }
-        FrameAndNodeOffset frameForPrevious =
-          GetFirstFrameInRangeForTextRect(range);
-        startsBetweenLineBreaker = frameForPrevious.mFrame == firstFrame.mFrame;
-      }
-    } else {
+    // If the first frame is a text frame, the result should be computed with
+    // the frame's API.
+    if (firstFrame->GetType() == nsGkAtoms::textFrame) {
       rv = firstFrame->GetCharacterRectsInRange(firstFrame.mOffsetInNode,
                                                 kEndOffset - offset, charRects);
       if (NS_WARN_IF(NS_FAILED(rv)) || NS_WARN_IF(charRects.IsEmpty())) {
         return rv;
       }
       // Assign the characters whose rects are computed by the call of
       // nsTextFrame::GetCharacterRectsInRange().
       AppendSubString(chars, firstContent, firstFrame.mOffsetInNode,
@@ -1820,16 +1812,114 @@ ContentEventHandler::OnQueryTextRectArra
         if (NS_WARN_IF(NS_FAILED(rv))) {
           return rv;
         }
         startsBetweenLineBreaker =
           range->GetStartParent() == rangeToPrevOffset->GetStartParent() &&
           range->StartOffset() == rangeToPrevOffset->StartOffset();
       }
     }
+    // Other contents should cause a line breaker rect before it.
+    // Note that moz-<br> element does not cause any text, however,
+    // it represents empty line at the last of current block.  Therefore,
+    // we need to compute its rect too.
+    else if (ShouldBreakLineBefore(firstContent, mRootContent) ||
+             IsMozBR(firstContent)) {
+      nsRect brRect;
+      // If the frame is not a <br> frame, we need to compute the caret rect
+      // with last character's rect before firstContent if there is.
+      // For example, if caret is after "c" of |<p>abc</p><p>def</p>|, IME may
+      // query a line breaker's rect after "c".  Then, if we compute it only
+      // with the 2nd <p>'s block frame, the result will be:
+      //  +-<p>--------------------------------+
+      //  |abc                                 |
+      //  +------------------------------------+
+      //
+      // I+-<p>--------------------------------+
+      //  |def                                 |
+      //  +------------------------------------+
+      // However, users expect popup windows of IME should be positioned at
+      // right-bottom of "c" like this:
+      //  +-<p>--------------------------------+
+      //  |abcI                                |
+      //  +------------------------------------+
+      //
+      //  +-<p>--------------------------------+
+      //  |def                                 |
+      //  +------------------------------------+
+      // Therefore, if the first frame isn't a <br> frame and there is a text
+      // node before the first node in the queried range, we should compute the
+      // first rect with the previous character's rect.
+      // If we already compute a character's rect in the queried range, we can
+      // compute it with the cached last character's rect.  (However, don't
+      // use this path if it's a <br> frame because trusting <br> frame's rect
+      // is better than guessing the rect from the previous character.)
+      if (firstFrame->GetType() != nsGkAtoms::brFrame &&
+          aEvent->mInput.mOffset != offset) {
+        // The frame position in the root widget will be added in the
+        // following for loop but we need the rect in the previous frame.
+        // So, we need to avoid using current frame position.
+        brRect = lastCharRect - frameRect.TopLeft();
+        if (!wasLineBreaker) {
+          if (isVertical) {
+            // Right of the last character.
+            brRect.y = brRect.YMost() + 1;
+            brRect.height = 1;
+          } else {
+            // Under the last character.
+            brRect.x = brRect.XMost() + 1;
+            brRect.width = 1;
+          }
+        }
+      }
+      // If it's not a <br> frame and it's the first character rect at the
+      // queried range, we need to the previous character of the start of
+      // the queried range if there is a text node.
+      else if (firstFrame->GetType() != nsGkAtoms::brFrame && lastTextContent) {
+        FrameRelativeRect brRectRelativeToLastTextFrame =
+          GuessLineBreakerRectAfter(lastTextContent);
+        if (NS_WARN_IF(!brRectRelativeToLastTextFrame.IsValid())) {
+          return NS_ERROR_FAILURE;
+        }
+        brRect = brRectRelativeToLastTextFrame.RectRelativeTo(firstFrame);
+      }
+      // Otherwise, we need to compute the line breaker's rect only with the
+      // first frame's rect.  But this may be unexpected.  For example,
+      // |<div contenteditable>[<p>]abc</p></div>|.  In this case, caret is
+      // before "a", therefore, users expect the rect left of "a".  However,
+      // we don't have enough information about the next character here and
+      // this isn't usual case (e.g., IME typically tries to query the rect
+      // of "a" or caret rect for computing its popup position).  Therefore,
+      // we shouldn't do more complicated hack here unless we'll get some bug
+      // reports actually.
+      else {
+        FrameRelativeRect relativeBRRect = GetLineBreakerRectBefore(firstFrame);
+        brRect = relativeBRRect.RectRelativeTo(firstFrame);
+      }
+      charRects.AppendElement(brRect);
+      chars.AssignLiteral("\n");
+      if (kBRLength > 1 && offset == aEvent->mInput.mOffset && offset) {
+        // If the first frame for the previous offset of the query range and
+        // the first frame for the start of query range are same, that means
+        // the start offset is between the first line breaker (i.e., the range
+        // starts between "\r" and "\n").
+        rv = SetRangeFromFlatTextOffset(range, aEvent->mInput.mOffset - 1, 1,
+                                        lineBreakType, true, nullptr);
+        if (NS_WARN_IF(NS_FAILED(rv))) {
+          return NS_ERROR_UNEXPECTED;
+        }
+        FrameAndNodeOffset frameForPrevious =
+          GetFirstFrameInRangeForTextRect(range);
+        startsBetweenLineBreaker = frameForPrevious.mFrame == firstFrame.mFrame;
+      }
+    } else {
+      NS_WARNING("The frame is neither a text frame nor a frame whose content "
+                 "causes a line break");
+      return NS_ERROR_FAILURE;
+    }
 
     for (size_t i = 0; i < charRects.Length() && offset < kEndOffset; i++) {
       nsRect charRect = charRects[i];
       charRect.x += frameRect.x;
       charRect.y += frameRect.y;
       lastCharRect = charRect;
 
       rect = LayoutDeviceIntRect::FromUnknownRect(
@@ -2007,36 +2097,23 @@ ContentEventHandler::OnQueryTextRect(Wid
   //
   //  +-<p>--------------------------------+
   //  |def                                 |
   //  +------------------------------------+
   // Therefore, if the first frame isn't a <br> frame and there is a text
   // node before the first node in the queried range, we should compute the
   // first rect with the previous character's rect.
   else if (firstFrame->GetType() != nsGkAtoms::brFrame && lastTextContent) {
-    int32_t length = static_cast<int32_t>(lastTextContent->Length());
-    if (NS_WARN_IF(length < 0)) {
-      return NS_ERROR_FAILURE;
-    }
-    nsIFrame* lastTextFrame = nullptr;
-    rv = GetFrameForTextRect(lastTextContent, length, true, &lastTextFrame);
-    if (NS_WARN_IF(NS_FAILED(rv)) || NS_WARN_IF(!lastTextFrame)) {
+    FrameRelativeRect brRectAfterLastChar =
+      GuessLineBreakerRectAfter(lastTextContent);
+    if (NS_WARN_IF(!brRectAfterLastChar.IsValid())) {
       return NS_ERROR_FAILURE;
     }
-    const nsRect kLastTextFrameRect = lastTextFrame->GetRect();
-    if (lastTextFrame->GetWritingMode().IsVertical()) {
-      // Below of the last text frame.
-      rect.SetRect(0, kLastTextFrameRect.height,
-                   kLastTextFrameRect.width, 0);
-    } else {
-      // Right of the last text frame (not bidi-aware).
-      rect.SetRect(kLastTextFrameRect.width, 0,
-                   0, kLastTextFrameRect.height);
-    }
-    rv = ConvertToRootRelativeOffset(lastTextFrame, rect);
+    rect = brRectAfterLastChar.mRect;
+    rv = ConvertToRootRelativeOffset(brRectAfterLastChar.mBaseFrame, rect);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
     frameRect = rect;
   }
   // Otherwise, we need to compute the line breaker's rect only with the
   // first frame's rect.  But this may be unexpected.  For example,
   // |<div contenteditable>[<p>]abc</p></div>|.  In this case, caret is before
--- a/dom/events/ContentEventHandler.h
+++ b/dom/events/ContentEventHandler.h
@@ -381,16 +381,25 @@ protected:
   };
 
   // Returns a rect for line breaker before the node of aFrame (If aFrame is
   // a <br> frame or a block level frame, it causes a line break at its
   // element's open tag, see also ShouldBreakLineBefore()).  Note that this
   // doesn't check if aFrame should cause line break in non-debug build.
   FrameRelativeRect GetLineBreakerRectBefore(nsIFrame* aFrame);
 
+  // Returns a line breaker rect after aTextContent as there is a line breaker
+  // immediately after aTextContent.  This is useful when following block
+  // element causes a line break before it and it needs to compute the line
+  // breaker's rect.  For example, if there is |<p>abc</p><p>def</p>|, the
+  // rect of 2nd <p>'s line breaker should be at right of "c" in the first
+  // <p>, not the start of 2nd <p>.  The result is relative to the last text
+  // frame which represents the last character of aTextContent.
+  FrameRelativeRect GuessLineBreakerRectAfter(nsIContent* aTextContent);
+
   // Make aRect non-empty.  If width and/or height is 0, these methods set them
   // to 1.  Note that it doesn't set nsRect's width nor height to one device
   // pixel because using nsRect::ToOutsidePixels() makes actual width or height
   // to 2 pixels because x and y may not be aligned to device pixels.
   void EnsureNonEmptyRect(nsRect& aRect) const;
   void EnsureNonEmptyRect(LayoutDeviceIntRect& aRect) const;
 };