Bug 1286464 part.15 Rewrite ContentEventHandler::OnQueryTextRect() with new utilities like ContentEventHandler::OnQueryTextRectArray() r?smaug draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 12 Aug 2016 14:11:33 +0900
changeset 399844 38dfe856f4ad7c91a3b14416525cdb23776382a0
parent 399843 f06cb89db2d44619822f900980b174e63d44f81c
child 399845 efab9565c8fed755fbf253ee1d965fe9f2f20f52
push id26007
push usermasayuki@d-toybox.com
push dateFri, 12 Aug 2016 06:30:02 +0000
reviewerssmaug
bugs1286464
milestone51.0a1
Bug 1286464 part.15 Rewrite ContentEventHandler::OnQueryTextRect() with new utilities like ContentEventHandler::OnQueryTextRectArray() r?smaug Although, this patch is pretty big and complicated, but I have no idea to separate this since this rewrites all over the OnQueryTextRect() with new utility methods which are implemented for OnQueryTextRectArray(). Currently, OnQueryTextRect() doesn't support line breaks before block level elements. Therefore, in HTML editors, eQueryTextRect may not work fine if the range includes open tags of block level elements. This causes odd positioning of IME's candidate window in non-e10s mode. This implements ContentEventHandler::GetLastFrameHavingFlatTextInRange() which scans the given range from last to first. However, this hits wrong NS_ASSERTION() in nsContentIterator::Last(). mLast can be nullptr if there is no content but Init() returns NS_OK even in such case. I think that returning NS_OK is fine because it's not illegal case. Therefore this patch fixes the wrong assertion. MozReview-Commit-ID: E6OnIA1eMou
dom/base/nsContentIterator.cpp
dom/events/ContentEventHandler.cpp
dom/events/ContentEventHandler.h
--- a/dom/base/nsContentIterator.cpp
+++ b/dom/base/nsContentIterator.cpp
@@ -940,22 +940,25 @@ nsContentIterator::First()
 
   mIsDone = mFirst == nullptr;
 }
 
 
 void
 nsContentIterator::Last()
 {
-  NS_ASSERTION(mLast, "No last node!");
+  // Note that mLast can be nullptr if MakeEmpty() is called in Init() since
+  // at that time, Init() returns NS_OK.
+  if (!mLast) {
+    MOZ_ASSERT(mIsDone);
+    return;
+  }
 
-  if (mLast) {
-    mozilla::DebugOnly<nsresult> rv = PositionAt(mLast);
-    NS_ASSERTION(NS_SUCCEEDED(rv), "Failed to position iterator!");
-  }
+  mozilla::DebugOnly<nsresult> rv = PositionAt(mLast);
+  NS_ASSERTION(NS_SUCCEEDED(rv), "Failed to position iterator!");
 
   mIsDone = mLast == nullptr;
 }
 
 
 void
 nsContentIterator::Next()
 {
--- a/dom/events/ContentEventHandler.cpp
+++ b/dom/events/ContentEventHandler.cpp
@@ -1490,16 +1490,115 @@ ContentEventHandler::GetFirstFrameHaving
   }
 
   nsIFrame* firstFrame = nullptr;
   GetFrameForTextRect(nodePosition.mNode, nodePosition.mOffset,
                       true, &firstFrame);
   return FrameAndNodeOffset(firstFrame, nodePosition.mOffset);
 }
 
+ContentEventHandler::FrameAndNodeOffset
+ContentEventHandler::GetLastFrameHavingFlatTextInRange(nsRange* aRange)
+{
+  NodePosition nodePosition;
+  nsCOMPtr<nsIContentIterator> iter = NS_NewPreContentIterator();
+  iter->Init(aRange);
+
+  nsINode* endNode = aRange->GetEndParent();
+  uint32_t endOffset = static_cast<uint32_t>(aRange->EndOffset());
+  // If the end point is start of a text node or specified by its parent and
+  // index, the node shouldn't be included into the range.  For example,
+  // with this case, |<p>abc[<br>]def</p>|, the range ends at 3rd children of
+  // <p> (see the range creation rules, "2.4. Cases: <element/>]"). This causes
+  // following frames:
+  // +----+-----+
+  // | abc|[<br>|
+  // +----+-----+
+  // +----+
+  // |]def|
+  // +----+
+  // So, if this method includes the 2nd text frame's rect to its result, the
+  // caller will return too tall rect which includes 2 lines in this case isn't
+  // expected by native IME  (e.g., popup of IME will be positioned at bottom
+  // of "d" instead of right-bottom of "c").  Therefore, this method shouldn't
+  // include the last frame when its content isn't really in aRange.
+  nsINode* nextNodeOfRangeEnd = nullptr;
+  if (endNode->IsNodeOfType(nsINode::eTEXT)) {
+    if (!endOffset) {
+      nextNodeOfRangeEnd = endNode;
+    }
+  } else if (endOffset < endNode->GetChildCount()) {
+    nextNodeOfRangeEnd = endNode->GetChildAt(endOffset);
+  }
+
+  for (iter->Last(); !iter->IsDone(); iter->Prev()) {
+    nsINode* node = iter->GetCurrentNode();
+    if (NS_WARN_IF(!node)) {
+      break;
+    }
+
+    if (!node->IsContent() || node == nextNodeOfRangeEnd) {
+      continue;
+    }
+
+    if (node->IsNodeOfType(nsINode::eTEXT)) {
+      nodePosition.mNode = node;
+      if (node == aRange->GetEndParent()) {
+        nodePosition.mOffset = aRange->EndOffset();
+      } else {
+        nodePosition.mOffset = node->Length();
+      }
+      break;
+    }
+
+    if (ShouldBreakLineBefore(node->AsContent(), mRootContent)) {
+      nodePosition.mNode = node;
+      nodePosition.mOffset = 0;
+      break;
+    }
+  }
+
+  if (!nodePosition.IsValid()) {
+    return FrameAndNodeOffset();
+  }
+
+  nsIFrame* lastFrame = nullptr;
+  GetFrameForTextRect(nodePosition.mNode, nodePosition.mOffset,
+                      true, &lastFrame);
+  if (!lastFrame) {
+    return FrameAndNodeOffset();
+  }
+
+  // If the last frame is a text frame, we need to check if the range actually
+  // includes at least one character in the range.  Therefore, if it's not a
+  // text frame, we need to do nothing anymore.
+  if (lastFrame->GetType() != nsGkAtoms::textFrame) {
+    return FrameAndNodeOffset(lastFrame, nodePosition.mOffset);
+  }
+
+  int32_t start, end;
+  if (NS_WARN_IF(NS_FAILED(lastFrame->GetOffsets(start, end)))) {
+    return FrameAndNodeOffset();
+  }
+
+  // If the start offset in the node is same as the computed offset in the
+  // node, the frame shouldn't be added to the text rect.  So, this should
+  // return previous text frame and its last offset.
+  if (nodePosition.mOffset == start) {
+    MOZ_ASSERT(nodePosition.mOffset);
+    GetFrameForTextRect(nodePosition.mNode, --nodePosition.mOffset,
+                        true, &lastFrame);
+    if (NS_WARN_IF(!lastFrame)) {
+      return FrameAndNodeOffset();
+    }
+  }
+
+  return FrameAndNodeOffset(lastFrame, nodePosition.mOffset);
+}
+
 ContentEventHandler::FrameRelativeRect
 ContentEventHandler::GetLineBreakerRectBefore(nsIFrame* aFrame)
 {
   // Note that this method should be called only with an element's frame whose
   // open tag causes a line break.
   MOZ_ASSERT(ShouldBreakLineBefore(aFrame->GetContent(), mRootContent));
 
   nsIFrame* frameForFontMetrics = aFrame;
@@ -1672,24 +1771,24 @@ ContentEventHandler::OnQueryTextRectArra
         if (NS_WARN_IF(NS_FAILED(rv))) {
           return NS_ERROR_UNEXPECTED;
         }
         FrameAndNodeOffset frameForPrevious =
           GetFirstFrameHavingFlatTextInRange(range);
         startsBetweenLineBreaker = frameForPrevious.mFrame == firstFrame.mFrame;
       }
     } else {
-      rv = firstFrame->GetCharacterRectsInRange(firstFrame.mStartOffsetInNode,
+      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.mStartOffsetInNode,
+      AppendSubString(chars, firstContent, firstFrame.mOffsetInNode,
                       charRects.Length());
       if (NS_WARN_IF(chars.Length() != charRects.Length())) {
         return NS_ERROR_UNEXPECTED;
       }
       if (kBRLength > 1 && chars[0] == '\n' &&
           offset == aEvent->mInput.mOffset && offset) {
         // If start of range starting from previous offset of query range is
         // same as the start of query range, the query range starts from
@@ -1827,107 +1926,164 @@ ContentEventHandler::OnQueryTextRect(Wid
   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);
 
-  // get the starting frame
-  NodePosition startNodePosition(iter->GetCurrentNode(), range->StartOffset());
-  if (!startNodePosition.mNode) {
-    startNodePosition =
-      GetNodePositionHavingFlatText(range->GetStartParent(),
-                                    startNodePosition.mOffset);
-    if (NS_WARN_IF(!startNodePosition.IsValid())) {
+  // Get the first frame which causes some text after the offset.
+  FrameAndNodeOffset firstFrame = GetFirstFrameHavingFlatTextInRange(range);
+
+  // If GetFirstFrameHavingFlatTextInRange() 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;
+    FrameRelativeRect relativeRect = GetLineBreakerRectBefore(firstFrame);
+    if (NS_WARN_IF(!relativeRect.IsValid())) {
       return NS_ERROR_FAILURE;
     }
-  }
-  nsIFrame* firstFrame = nullptr;
-  rv = GetFrameForTextRect(startNodePosition.mNode, startNodePosition.mOffset,
-                           true, &firstFrame);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  // get the starting frame rect
-  nsRect rect(nsPoint(0, 0), firstFrame->GetRect().Size());
-  rv = ConvertToRootRelativeOffset(firstFrame, rect);
-  NS_ENSURE_SUCCESS(rv, rv);
-  nsRect frameRect = rect;
-  nsPoint ptOffset;
-  firstFrame->GetPointFromOffset(startNodePosition.mOffset, &ptOffset);
-  if (firstFrame->GetWritingMode().IsVertical()) {
-    rect.y += ptOffset.y;
-    rect.height -= ptOffset.y;
-  } else {
-    rect.x += ptOffset.x;
-    rect.width -= ptOffset.x;
+    rect = relativeRect.RectRelativeTo(firstFrame);
+    rv = ConvertToRootRelativeOffset(firstFrame, rect);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
+    frameRect = rect;
   }
   // UnionRect() requires non-empty rect.  So, let's make sure to get non-emtpy
   // rect from the first frame.
   EnsureNonEmptyRect(rect);
 
-  // get the ending frame
-  NodePosition endNodePosition =
-    GetNodePositionHavingFlatText(range->GetEndParent(), range->EndOffset());
-  if (NS_WARN_IF(!endNodePosition.IsValid())) {
+  // Get the last frame which causes some text in the range.
+  FrameAndNodeOffset lastFrame = GetLastFrameHavingFlatTextInRange(range);
+  if (NS_WARN_IF(!lastFrame.IsValid())) {
     return NS_ERROR_FAILURE;
   }
-  nsIFrame* lastFrame = nullptr;
-  rv = GetFrameForTextRect(endNodePosition.mNode, endNodePosition.mOffset,
-                           range->Collapsed(), &lastFrame);
-  NS_ENSURE_SUCCESS(rv, rv);
 
   // iterate over all covered frames
   for (nsIFrame* frame = firstFrame; frame != lastFrame;) {
     frame = frame->GetNextContinuation();
     if (!frame) {
       do {
         iter->Next();
         nsINode* node = iter->GetCurrentNode();
         if (!node) {
           break;
         }
         if (!node->IsNodeOfType(nsINode::eCONTENT)) {
           continue;
         }
-        frame = node->AsContent()->GetPrimaryFrame();
+        nsIFrame* primaryFrame = node->AsContent()->GetPrimaryFrame();
+        // The node may be hidden by CSS.
+        if (!primaryFrame) {
+          continue;
+        }
+        // We should take only text frame's rect and br frame's rect.  We can
+        // always use frame rect of text frame and GetLineBreakerRectBefore()
+        // can return exactly correct rect only for <br> frame for now.  On the
+        // other hand, GetLineBreakRectBefore() returns guessed caret rect for
+        // the other frames.  We shouldn't include such odd rect to the result.
+        if (primaryFrame->GetType() == nsGkAtoms::textFrame ||
+            primaryFrame->GetType() == nsGkAtoms::brFrame) {
+          frame = primaryFrame;
+        }
       } while (!frame && !iter->IsDone());
       if (!frame) {
-        // this can happen when the end offset of the range is 0.
-        frame = lastFrame;
+        break;
       }
     }
-    frameRect.SetRect(nsPoint(0, 0), frame->GetRect().Size());
+    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;
+      }
+      frameRect = relativeRect.RectRelativeTo(frame);
+    }
     rv = ConvertToRootRelativeOffset(frame, frameRect);
-    NS_ENSURE_SUCCESS(rv, rv);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
     // UnionRect() requires non-empty rect.  So, let's make sure to get
     // non-emtpy rect from the frame.
     EnsureNonEmptyRect(frameRect);
     if (frame != lastFrame) {
       // not last frame, so just add rect to previous result
       rect.UnionRect(rect, frameRect);
     }
   }
 
-  // get the ending frame rect
-  lastFrame->GetPointFromOffset(endNodePosition.mOffset, &ptOffset);
-  if (lastFrame->GetWritingMode().IsVertical()) {
-    frameRect.height -= lastFrame->GetRect().height - ptOffset.y;
-  } else {
-    frameRect.width -= lastFrame->GetRect().width - ptOffset.x;
+  // 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;
+    }
   }
-  // UnionRect() requires non-empty rect.  So, let's make sure to get non-empty
-  // rect from the last frame.
-  EnsureNonEmptyRect(frameRect);
 
-  if (firstFrame == lastFrame) {
-    rect.IntersectRect(rect, frameRect);
-  } else {
-    rect.UnionRect(rect, frameRect);
+  // Shrink the last frame for cutting off the text after the query range.
+  if (lastFrame->GetType() == nsGkAtoms::textFrame) {
+    lastFrame->GetPointFromOffset(lastFrame.mOffsetInNode, &ptOffset);
+    if (lastFrame->GetWritingMode().IsVertical()) {
+      frameRect.height -= lastFrame->GetRect().height - ptOffset.y;
+    } else {
+      frameRect.width -= lastFrame->GetRect().width - ptOffset.x;
+    }
+    // UnionRect() requires non-empty rect.  So, let's make sure to get
+    // non-empty rect from the last frame.
+    EnsureNonEmptyRect(frameRect);
+
+    if (firstFrame.mFrame == lastFrame.mFrame) {
+      rect.IntersectRect(rect, frameRect);
+    } else {
+      rect.UnionRect(rect, frameRect);
+    }
   }
   aEvent->mReply.mRect = LayoutDeviceIntRect::FromUnknownRect(
       rect.ToOutsidePixels(mPresContext->AppUnitsPerDevPixel()));
   // Returning empty rect may cause native IME confused, let's make sure to
   // return non-empty rect.
   EnsureNonEmptyRect(aEvent->mReply.mRect);
   aEvent->mReply.mWritingMode = lastFrame->GetWritingMode();
   aEvent->mSucceeded = true;
--- a/dom/events/ContentEventHandler.h
+++ b/dom/events/ContentEventHandler.h
@@ -313,42 +313,47 @@ protected:
   struct MOZ_STACK_CLASS FrameAndNodeOffset final
   {
     // mFrame is safe since this can live in only stack class and
     // ContentEventHandler doesn't modify layout after
     // ContentEventHandler::Init() flushes pending layout.  In other words,
     // this struct shouldn't be used before calling
     // ContentEventHandler::Init().
     nsIFrame* mFrame;
-    // Start offset in the node of mFrame
-    int32_t mStartOffsetInNode;
+    // offset in the node of mFrame
+    int32_t mOffsetInNode;
 
     FrameAndNodeOffset()
       : mFrame(nullptr)
-      , mStartOffsetInNode(-1)
+      , mOffsetInNode(-1)
     {
     }
 
     FrameAndNodeOffset(nsIFrame* aFrame, int32_t aStartOffsetInNode)
       : mFrame(aFrame)
-      , mStartOffsetInNode(aStartOffsetInNode)
+      , mOffsetInNode(aStartOffsetInNode)
     {
     }
 
     nsIFrame* operator->() { return mFrame; }
     const nsIFrame* operator->() const { return mFrame; }
     operator nsIFrame*() { return mFrame; }
     operator const nsIFrame*() const { return mFrame; }
-    bool IsValid() const { return mFrame && mStartOffsetInNode >= 0; }
+    bool IsValid() const { return mFrame && mOffsetInNode >= 0; }
   };
   // Get first frame after the start of the given range for computing text rect.
   // This returns invalid FrameAndNodeOffset if there is no content which
-  // causes text after the start of the range.
+  // causes text in the range.  mOffsetInNode is start offset in the frame.
   FrameAndNodeOffset GetFirstFrameHavingFlatTextInRange(nsRange* aRange);
 
+  // Get last frame before the end of the given range for computing text rect.
+  // This returns invalid FrameAndNodeOffset if there is no content which
+  // causes text in the range.  mOffsetInNode is end offset in the frame.
+  FrameAndNodeOffset GetLastFrameHavingFlatTextInRange(nsRange* aRange);
+
   struct MOZ_STACK_CLASS FrameRelativeRect final
   {
     // mRect is relative to the mBaseFrame's position.
     nsRect mRect;
     nsIFrame* mBaseFrame;
 
     FrameRelativeRect()
       : mBaseFrame(nullptr)