Bug 1286464 part.13 ContentEventHandler::OnQueryTextRectArray() should guess a line breaker's rect with previous character's rect if it's available r=smaug draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 05 Aug 2016 14:31:37 +0900
changeset 400169 14807e98cc9a6be22e6168570a299abf0ecaf267
parent 400168 423c7b57355d659e1a5bd96dac35060cae38801c
child 400170 c7adc65ccf0fefa7cb1c3bd0b056dc04f8a85b3e
push id26081
push usermasayuki@d-toybox.com
push dateFri, 12 Aug 2016 17:11:29 +0000
reviewerssmaug
bugs1286464
milestone51.0a1
Bug 1286464 part.13 ContentEventHandler::OnQueryTextRectArray() should guess a line breaker's rect with previous character's rect if it's available r=smaug This is hack for returning better rect for a line breaker caused by non-<br> element. E.g., if a block frame causes a line break, after the last visible character is the best position, i.e., there is |<p>ABC</p><p>DEF</p>| and the caret is after "C", querying next character's rect should be computed with "C"'s rect, rather than computed with next <p> element's border rect. MozReview-Commit-ID: 7nXt74GGpNf
dom/events/ContentEventHandler.cpp
--- a/dom/events/ContentEventHandler.cpp
+++ b/dom/events/ContentEventHandler.cpp
@@ -1582,16 +1582,18 @@ ContentEventHandler::OnQueryTextRectArra
   LineBreakType lineBreakType = GetLineBreakType(aEvent);
   const uint32_t kBRLength = GetBRLength(lineBreakType);
 
   RefPtr<nsRange> range = new nsRange(mRootContent);
 
   LayoutDeviceIntRect rect;
   uint32_t offset = aEvent->mInput.mOffset;
   const uint32_t kEndOffset = offset + aEvent->mInput.mLength;
+  bool wasLineBreaker = false;
+  nsRect lastCharRect;
   while (offset < kEndOffset) {
     rv = SetRangeFromFlatTextOffset(range, offset, 1, lineBreakType, true,
                                     nullptr);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
 
     // TODO: If the range is collapsed, that means offset reaches to the end
@@ -1619,21 +1621,44 @@ ContentEventHandler::OnQueryTextRectArra
       return rv;
     }
 
     bool startsBetweenLineBreaker = false;
     nsAutoString chars;
     AutoTArray<nsRect, 16> charRects;
 
     if (ShouldBreakLineBefore(firstContent, mRootContent)) {
-      // TODO: If the frame isn't <br> and there was previous text frame or
-      //       <br>, we can set the rect to caret rect at the end.  Currently,
-      //       this sets the rect to caret rect at the start of the node.
-      FrameRelativeRect brRect = GetLineBreakerRectBefore(firstFrame);
-      charRects.AppendElement(brRect.RectRelativeTo(firstFrame));
+      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 (firstFrame->GetWritingMode().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);
@@ -1675,29 +1700,31 @@ ContentEventHandler::OnQueryTextRectArra
           range->StartOffset() == rangeToPrevOffset->StartOffset();
       }
     }
 
     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(
                charRect.ToOutsidePixels(mPresContext->AppUnitsPerDevPixel()));
       // Returning empty rect may cause native IME confused, let's make sure to
       // return non-empty rect.
       EnsureNonEmptyRect(rect);
 
       aEvent->mReply.mRectArray.AppendElement(rect);
       offset++;
 
       // If it's not a line breaker or the line breaker length is same as
       // XP line breaker's, we need to do nothing for current character.
-      if (chars[i] != '\n' || kBRLength == 1) {
+      wasLineBreaker = chars[i] == '\n';
+      if (!wasLineBreaker || kBRLength == 1) {
         continue;
       }
 
       MOZ_ASSERT(kBRLength == 2);
 
       // If it's already reached the end of query range, we don't need to do
       // anymore.
       if (offset == kEndOffset) {