Bug 1286464 part.12 ContentEventHandler::GetFirstFrameHavingFlatTextInRange() should return only frames whose content causes text r=smaug draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 21 Jul 2016 17:45:17 +0900
changeset 400168 423c7b57355d659e1a5bd96dac35060cae38801c
parent 400167 97653f1ed04a6a03373cd53613d92aba45fbe38b
child 400169 14807e98cc9a6be22e6168570a299abf0ecaf267
push id26081
push usermasayuki@d-toybox.com
push dateFri, 12 Aug 2016 17:11:29 +0000
reviewerssmaug
bugs1286464
milestone51.0a1
Bug 1286464 part.12 ContentEventHandler::GetFirstFrameHavingFlatTextInRange() should return only frames whose content causes text r=smaug If it returns frames which don't cause text, the caller needs complicated loop. So, it should return only frames which causes some text. MozReview-Commit-ID: 9gny0w1PUMa
dom/events/ContentEventHandler.cpp
dom/events/ContentEventHandler.h
--- a/dom/events/ContentEventHandler.cpp
+++ b/dom/events/ContentEventHandler.cpp
@@ -1447,30 +1447,53 @@ ContentEventHandler::GetNodePositionHavi
 
   NS_WARNING("aNodeOffset is invalid value");
   return NodePosition();
 }
 
 ContentEventHandler::FrameAndNodeOffset
 ContentEventHandler::GetFirstFrameHavingFlatTextInRange(nsRange* aRange)
 {
-  // used to iterate over all contents and their frames
-  nsCOMPtr<nsIContentIterator> iter = NS_NewContentIterator();
-  iter->Init(aRange);
+  NodePosition nodePosition;
+  nsCOMPtr<nsIContentIterator> iter = NS_NewPreContentIterator();
+  for (iter->Init(aRange); !iter->IsDone(); iter->Next()) {
+    nsINode* node = iter->GetCurrentNode();
+    if (NS_WARN_IF(!node)) {
+      break;
+    }
+
+    if (!node->IsContent()) {
+      continue;
+    }
 
-  // get the starting frame
-  NodePosition nodePosition(iter->GetCurrentNode(), aRange->StartOffset());
-  if (!nodePosition.mNode) {
-    nodePosition =
-      GetNodePositionHavingFlatText(aRange->GetStartParent(),
-                                    nodePosition.mOffset);
-    if (NS_WARN_IF(!nodePosition.IsValid())) {
-      return FrameAndNodeOffset();
+    if (node->IsNodeOfType(nsINode::eTEXT)) {
+      // If the range starts at the end of a text node, we need to find
+      // next node which causes text.
+      int32_t offsetInNode =
+        node == aRange->GetStartParent() ? aRange->StartOffset() : 0;
+      if (static_cast<uint32_t>(offsetInNode) < node->Length()) {
+        nodePosition.mNode = node;
+        nodePosition.mOffset = offsetInNode;
+        break;
+      }
+      continue;
+    }
+
+    // If the element node causes a line break before it, it's the first
+    // node causing text.
+    if (ShouldBreakLineBefore(node->AsContent(), mRootContent)) {
+      nodePosition.mNode = node;
+      nodePosition.mOffset = 0;
     }
   }
+
+  if (!nodePosition.IsValid()) {
+    return FrameAndNodeOffset();
+  }
+
   nsIFrame* firstFrame = nullptr;
   GetFrameForTextRect(nodePosition.mNode, nodePosition.mOffset,
                       true, &firstFrame);
   return FrameAndNodeOffset(firstFrame, nodePosition.mOffset);
 }
 
 ContentEventHandler::FrameRelativeRect
 ContentEventHandler::GetLineBreakerRectBefore(nsIFrame* aFrame)
@@ -1569,20 +1592,24 @@ ContentEventHandler::OnQueryTextRectArra
                                     nullptr);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
 
     // TODO: If the range is collapsed, that means offset reaches to the end
     //       of the contents.  We need to do something here.
 
-    // get the starting frame
+    // Get the first frame which causes some text after the offset.
     FrameAndNodeOffset firstFrame = GetFirstFrameHavingFlatTextInRange(range);
-    if (NS_WARN_IF(!firstFrame.IsValid())) {
-      return NS_ERROR_FAILURE;
+
+    // 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()) {
+      break;
     }
 
     nsIContent* firstContent = firstFrame.mFrame->GetContent();
     if (NS_WARN_IF(!firstContent)) {
       return NS_ERROR_FAILURE;
     }
 
     // get the starting frame rect
@@ -1616,18 +1643,16 @@ ContentEventHandler::OnQueryTextRectArra
         FrameAndNodeOffset frameForPrevious =
           GetFirstFrameHavingFlatTextInRange(range);
         startsBetweenLineBreaker = frameForPrevious.mFrame == firstFrame.mFrame;
       }
     } else {
       rv = firstFrame->GetCharacterRectsInRange(firstFrame.mStartOffsetInNode,
                                                 kEndOffset - offset, charRects);
       if (NS_WARN_IF(NS_FAILED(rv)) || NS_WARN_IF(charRects.IsEmpty())) {
-        // XXX: If the node isn't a text node and does not cause a line break,
-        //      we need to recompute with new range, but how?
         return rv;
       }
       // Assign the characters whose rects are computed by the call of
       // nsTextFrame::GetCharacterRectsInRange().
       AppendSubString(chars, firstContent, firstFrame.mStartOffsetInNode,
                       charRects.Length());
       if (NS_WARN_IF(chars.Length() != charRects.Length())) {
         return NS_ERROR_UNEXPECTED;
--- a/dom/events/ContentEventHandler.h
+++ b/dom/events/ContentEventHandler.h
@@ -334,17 +334,19 @@ protected:
     }
 
     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; }
   };
-  // Get first frame in the given range for computing text rect.
+  // 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.
   FrameAndNodeOffset GetFirstFrameHavingFlatTextInRange(nsRange* aRange);
 
   struct MOZ_STACK_CLASS FrameRelativeRect final
   {
     // mRect is relative to the mBaseFrame's position.
     nsRect mRect;
     nsIFrame* mBaseFrame;