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 395433 bcc163bdb168fb84fbb5b246428edbefe75f60f0
parent 395432 b3fae9b204424373262c6646b1a399549bc708fd
child 395434 a5af7ae733c7f479f175026167c62463b6e1ce61
push id24766
push usermasayuki@d-toybox.com
push dateTue, 02 Aug 2016 05:12:01 +0000
reviewerssmaug
bugs1286464
milestone51.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;
     }
@@ -1865,19 +1874,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);
 
@@ -1887,43 +1898,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;
@@ -1961,17 +1993,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;
       }
@@ -1983,21 +2014,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