Bug 1286464 part.17 ContentEventHandler::OnQueryTextRect() should compute a line breaker's rect from the last text frame if the queried range starts with a block frame r?smaug draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 29 Jul 2016 13:29:45 +0900
changeset 394879 43eb79cff14baebb79ef8bc02df57b544d876534
parent 394878 f59ccfde8d4cad8d687a37ad30f300cfb378d235
child 394880 c7ef92d5169557ff2fdb2edb897ee82729fe1b52
push id24655
push usermasayuki@d-toybox.com
push dateMon, 01 Aug 2016 08:00:00 +0000
reviewerssmaug
bugs1286464
milestone50.0a1
Bug 1286464 part.17 ContentEventHandler::OnQueryTextRect() should compute a line breaker's rect from the last text frame if the queried range starts with a block frame r?smaug When queried range starts with a block frame, we cannot compute proper line breaker's rect at its open tag only with the frame's rect because the proper rect should be at the end of previous text frame. Therefore, this patch makes SetRangeFromFlatTextOffset() return the last text node found at scanning the range. Then, OnQueryTextRect() can compute the first line breaker's rect with it (if there is at least one text node). However, if the last text node is hidden by CSS or something, this patch won't work. We need to do something in another bug for hidden frame's issue, though. But note that IME typically doesn't request hidden text's rect because neither selection nor composition string is in such range. MozReview-Commit-ID: 2FFzNoubJ1y
dom/events/ContentEventHandler.cpp
dom/events/ContentEventHandler.h
--- a/dom/events/ContentEventHandler.cpp
+++ b/dom/events/ContentEventHandler.cpp
@@ -946,21 +946,25 @@ ContentEventHandler::ExpandToClusterBoun
 }
 
 nsresult
 ContentEventHandler::SetRangeFromFlatTextOffset(nsRange* aRange,
                                                 uint32_t aOffset,
                                                 uint32_t aLength,
                                                 LineBreakType aLineBreakType,
                                                 bool aExpandToClusterBoundaries,
-                                                uint32_t* aNewOffset)
+                                                uint32_t* aNewOffset,
+                                                nsIContent** aLastTextNode)
 {
   if (aNewOffset) {
     *aNewOffset = aOffset;
   }
+  if (aLastTextNode) {
+    *aLastTextNode = nullptr;
+  }
 
   // Special case like <br contenteditable>
   if (!mRootContent->HasChildren()) {
     nsresult rv = aRange->SetStart(mRootContent, 0);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
     rv = aRange->SetEnd(mRootContent, 0);
@@ -984,16 +988,21 @@ ContentEventHandler::SetRangeFromFlatTex
       break;
     }
     // FYI: mRootContent shouldn't cause any text. So, we can skip it simply.
     if (node == mRootContent || !node->IsContent()) {
       continue;
     }
     nsIContent* content = node->AsContent();
 
+    if (aLastTextNode && content->IsNodeOfType(nsINode::eTEXT)) {
+      NS_IF_RELEASE(*aLastTextNode);
+      NS_ADDREF(*aLastTextNode = content);
+    }
+
     uint32_t textLength =
       content->IsNodeOfType(nsINode::eTEXT) ?
         GetTextLength(content, aLineBreakType) :
         (ShouldBreakLineBefore(content, mRootContent) ?
            GetBRLength(aLineBreakType) : 0);
     if (!textLength) {
       continue;
     }
@@ -1862,19 +1871,21 @@ ContentEventHandler::OnQueryTextRect(Wid
   // If mLength is 0 (this may be caused by bug of native IME), we should
   // redirect this event to OnQueryCaretRect().
   if (!aEvent->mInput.mLength) {
     return OnQueryCaretRect(aEvent);
   }
 
   LineBreakType lineBreakType = GetLineBreakType(aEvent);
   RefPtr<nsRange> range = new nsRange(mRootContent);
+  nsCOMPtr<nsIContent> lastTextContent;
   rv = SetRangeFromFlatTextOffset(range, aEvent->mInput.mOffset,
                                   aEvent->mInput.mLength, lineBreakType, true,
-                                  &aEvent->mReply.mOffset);
+                                  &aEvent->mReply.mOffset,
+                                  getter_AddRefs(lastTextContent));
   NS_ENSURE_SUCCESS(rv, rv);
   rv = GenerateFlatTextContent(range, aEvent->mReply.mString, lineBreakType);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // used to iterate over all contents and their frames
   nsCOMPtr<nsIContentIterator> iter = NS_NewContentIterator();
   iter->Init(range);
 
@@ -1884,43 +1895,64 @@ ContentEventHandler::OnQueryTextRect(Wid
   // If GetFirstFrameInRangeForTextRect() does not return valid frame,
   // that means that there is no remaining content which causes text.
   // So, in such case, we must have reached the end of the contents.
   if (!firstFrame.IsValid()) {
     // TODO: Handle this case later.
     return NS_ERROR_FAILURE;
   }
 
-  // For GetLineBreakerRectBefore() doesn't work well with block frame, so,
-  // this should fail if there is only block frame in the queried range for now.
-  // TODO: Fix later.
-  bool hasSetRect = false;
-
   nsRect rect, frameRect;
   nsPoint ptOffset;
 
-  // Get the starting frame rect if it's not a block frame.
   if (firstFrame->GetType() == nsGkAtoms::textFrame) {
-    hasSetRect = true;
     rect.SetRect(nsPoint(0, 0), firstFrame->GetRect().Size());
     rv = ConvertToRootRelativeOffset(firstFrame, rect);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
     frameRect = rect;
     firstFrame->GetPointFromOffset(firstFrame.mOffsetInNode, &ptOffset);
     if (firstFrame->GetWritingMode().IsVertical()) {
       rect.y += ptOffset.y;
       rect.height -= ptOffset.y;
     } else {
       rect.x += ptOffset.x;
       rect.width -= ptOffset.x;
     }
-  } else if (firstFrame->GetType() == nsGkAtoms::brFrame) {
-    hasSetRect = true;
+  } else if (firstFrame->GetType() != nsGkAtoms::brFrame && lastTextContent) {
+    // If the node isn't <br> frame but there is a text node, we should
+    // compute the line break rect with the last character.
+    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)) {
+      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);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
+    frameRect = rect;
+  } else {
+    // If the frame is a <br> frame or there are no text nodes, we should use
+    // the result of GetLineBreakerRectBefore().
     FrameRelativeRect relativeRect = GetLineBreakerRectBefore(firstFrame);
     if (NS_WARN_IF(!relativeRect.IsValid())) {
       return NS_ERROR_FAILURE;
     }
     rect = relativeRect.RectRelativeTo(firstFrame);
     rv = ConvertToRootRelativeOffset(firstFrame, rect);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
@@ -1958,17 +1990,16 @@ ContentEventHandler::OnQueryTextRect(Wid
             primaryFrame->GetType() == nsGkAtoms::brFrame) {
           frame = primaryFrame;
         }
       } while (!frame && !iter->IsDone());
       if (!frame) {
         break;
       }
     }
-    hasSetRect = true;
     if (frame->GetType() == nsGkAtoms::textFrame) {
       frameRect.SetRect(nsPoint(0, 0), frame->GetRect().Size());
     } else {
       MOZ_ASSERT(frame->GetType() == nsGkAtoms::brFrame);
       FrameRelativeRect relativeRect = GetLineBreakerRectBefore(frame);
       if (NS_WARN_IF(!relativeRect.IsValid())) {
         return NS_ERROR_FAILURE;
       }
@@ -1980,21 +2011,16 @@ ContentEventHandler::OnQueryTextRect(Wid
     }
     EnsureNonEmptyRect(frameRect);
     if (frame != lastFrame) {
       // not last frame, so just add rect to previous result
       rect.UnionRect(rect, frameRect);
     }
   }
 
-  // TODO: We need to fix this case later.
-  if (NS_WARN_IF(!hasSetRect)) {
-    return NS_ERROR_FAILURE;
-  }
-
   // Get the ending frame rect.
   // FYI: If first frame and last frame are same, frameRect is already set
   //      to the rect excluding the text before the query range.
   if (firstFrame.mFrame != lastFrame.mFrame) {
     frameRect.SetRect(nsPoint(0, 0), lastFrame->GetRect().Size());
     rv = ConvertToRootRelativeOffset(lastFrame, frameRect);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
--- a/dom/events/ContentEventHandler.h
+++ b/dom/events/ContentEventHandler.h
@@ -268,17 +268,18 @@ protected:
   // Make the DOM range from the offset of FlatText and the text length.
   // If aExpandToClusterBoundaries is true, the start offset and the end one are
   // expanded to nearest cluster boundaries.
   nsresult SetRangeFromFlatTextOffset(nsRange* aRange,
                                       uint32_t aOffset,
                                       uint32_t aLength,
                                       LineBreakType aLineBreakType,
                                       bool aExpandToClusterBoundaries,
-                                      uint32_t* aNewOffset = nullptr);
+                                      uint32_t* aNewOffset = nullptr,
+                                      nsIContent** aLastTextNode = nullptr);
   // If the aRange isn't in text node but next to a text node, this method
   // modifies it in the text node.  Otherwise, not modified.
   nsresult AdjustCollapsedRangeMaybeIntoTextNode(nsRange* aCollapsedRange);
   // Find the first frame for the range and get the start offset in it.
   nsresult GetStartFrameAndOffset(const nsRange* aRange,
                                   nsIFrame*& aFrame,
                                   int32_t& aOffsetInFrame);
   // Convert the frame relative offset to be relative to the root frame of the