Bug 1296217 ContentEventHandler::OnQueryTextRectArray() should apply CSS transform to each character or line breaker rect r=smaug
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 06 Sep 2016 11:46:31 +0900
changeset 312887 8a0e581059ccfb773238873d4c218c9d07e137bb
parent 312886 cfd5f330f1158682b0b109de50b679d3329f12fc
child 312888 66324402a18bb01a39f533aaabff0f202d89cc94
push id30663
push usercbook@mozilla.com
push dateWed, 07 Sep 2016 15:12:31 +0000
treeherdermozilla-central@3d0b41fdd93b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1296217
milestone51.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1296217 ContentEventHandler::OnQueryTextRectArray() should apply CSS transform to each character or line breaker rect r=smaug Currently, ContentEventHandler::OnQueryTextRectArray() is used only in e10s mode (at caching necessary character rects in ContentCacheInChild). Therefore, this bug occurs only in e10s mode. ContentEventHandler::OnQueryTextRectArray() applies CSS transform only to each frame rect. Therefore, character rect's width and height are not applied. This patch makes the loop apply CSS transform to each character or line breaker rect (i.e., each item of charRects). Then, we need to rewrite the lastCharRect hack. It stores the last charRect value for computing next line breaker rect if next line breaker is caused by a block level element or something, i.e., not caused by a <br> frame. So, when brRect is computed with lastCharRect, the loop needs to apply CSS transform of the last text frame to the following brRect because it tries to compute a caret rect immediately after the last character. For doing this, this patch adds lastFrame which stores the last frame for lastCharRect and set it to baseFrame. Then, at applying CSS transform to each charRect, it can apply CSS transform of expected frame. Similarly, when brRect is computed with last text frame, this patch looks for the last text frame from lastTextContent and use it as base frame to apply to CSS transform. MozReview-Commit-ID: 5Yr2HMrooHd
dom/events/ContentEventHandler.cpp
--- a/dom/events/ContentEventHandler.cpp
+++ b/dom/events/ContentEventHandler.cpp
@@ -1805,17 +1805,21 @@ ContentEventHandler::OnQueryTextRectArra
 
   RefPtr<nsRange> range = new nsRange(mRootContent);
 
   bool isVertical = false;
   LayoutDeviceIntRect rect;
   uint32_t offset = aEvent->mInput.mOffset;
   const uint32_t kEndOffset = offset + aEvent->mInput.mLength;
   bool wasLineBreaker = false;
+  // lastCharRect stores the last charRect value (see below for the detail of
+  // charRect).
   nsRect lastCharRect;
+  // lastFrame is base frame of lastCharRect.
+  nsIFrame* lastFrame;
   while (offset < kEndOffset) {
     nsCOMPtr<nsIContent> lastTextContent;
     rv = SetRangeFromFlatTextOffset(range, offset, 1, lineBreakType, true,
                                     nullptr, getter_AddRefs(lastTextContent));
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
 
@@ -1848,28 +1852,24 @@ ContentEventHandler::OnQueryTextRectArra
       break;
     }
 
     nsIContent* firstContent = firstFrame.mFrame->GetContent();
     if (NS_WARN_IF(!firstContent)) {
       return NS_ERROR_FAILURE;
     }
 
-    // get the starting frame rect
-    nsRect frameRect(nsPoint(0, 0), firstFrame->GetRect().Size());
-    rv = ConvertToRootRelativeOffset(firstFrame, frameRect);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
-    }
-
     bool startsBetweenLineBreaker = false;
     nsAutoString chars;
     // XXX not bidi-aware this class...
     isVertical = firstFrame->GetWritingMode().IsVertical();
 
+    nsIFrame* baseFrame = firstFrame;
+    // charRect should have each character rect or line breaker rect relative
+    // to the base frame.
     AutoTArray<nsRect, 16> charRects;
 
     // If the first frame is a text frame, the result should be computed with
     // the frame's API.
     if (firstFrame->GetType() == nsGkAtoms::textFrame) {
       rv = firstFrame->GetCharacterRectsInRange(firstFrame.mOffsetInNode,
                                                 kEndOffset - offset, charRects);
       if (NS_WARN_IF(NS_FAILED(rv)) || NS_WARN_IF(charRects.IsEmpty())) {
@@ -1932,20 +1932,18 @@ ContentEventHandler::OnQueryTextRectArra
       // node before the first node in the queried range, we should compute the
       // first rect with the previous character's rect.
       // If we already compute a character's rect in the queried range, we can
       // compute it with the cached last character's rect.  (However, don't
       // use this path if it's a <br> frame because trusting <br> frame's rect
       // is better than guessing the rect from the previous character.)
       if (firstFrame->GetType() != nsGkAtoms::brFrame &&
           aEvent->mInput.mOffset != offset) {
-        // 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();
+        baseFrame = lastFrame;
+        brRect = lastCharRect;
         if (!wasLineBreaker) {
           if (isVertical) {
             // Right of the last character.
             brRect.y = brRect.YMost() + 1;
             brRect.height = 1;
           } else {
             // Under the last character.
             brRect.x = brRect.XMost() + 1;
@@ -1957,17 +1955,26 @@ ContentEventHandler::OnQueryTextRectArra
       // queried range, we need to the previous character of the start of
       // the queried range if there is a text node.
       else if (firstFrame->GetType() != nsGkAtoms::brFrame && lastTextContent) {
         FrameRelativeRect brRectRelativeToLastTextFrame =
           GuessLineBreakerRectAfter(lastTextContent);
         if (NS_WARN_IF(!brRectRelativeToLastTextFrame.IsValid())) {
           return NS_ERROR_FAILURE;
         }
-        brRect = brRectRelativeToLastTextFrame.RectRelativeTo(firstFrame);
+        // Look for the last text frame for lastTextContent.
+        nsIFrame* primaryFrame = lastTextContent->GetPrimaryFrame();
+        if (NS_WARN_IF(!primaryFrame)) {
+          return NS_ERROR_FAILURE;
+        }
+        baseFrame = primaryFrame->LastContinuation();
+        if (NS_WARN_IF(!baseFrame)) {
+          return NS_ERROR_FAILURE;
+        }
+        brRect = brRectRelativeToLastTextFrame.RectRelativeTo(baseFrame);
       }
       // Otherwise, we need to compute the line breaker's rect only with the
       // first frame's rect.  But this may be unexpected.  For example,
       // |<div contenteditable>[<p>]abc</p></div>|.  In this case, caret is
       // before "a", therefore, users expect the rect left of "a".  However,
       // we don't have enough information about the next character here and
       // this isn't usual case (e.g., IME typically tries to query the rect
       // of "a" or caret rect for computing its popup position).  Therefore,
@@ -1996,19 +2003,27 @@ ContentEventHandler::OnQueryTextRectArra
     } else {
       NS_WARNING("The frame is neither a text frame nor a frame whose content "
                  "causes a line break");
       return NS_ERROR_FAILURE;
     }
 
     for (size_t i = 0; i < charRects.Length() && offset < kEndOffset; i++) {
       nsRect charRect = charRects[i];
-      charRect.x += frameRect.x;
-      charRect.y += frameRect.y;
+      // Store lastCharRect before applying CSS transform because it may be
+      // used for computing a line breaker rect.  Then, the computed line
+      // breaker rect will be applied CSS transform again.  Therefore,
+      // the value of lastCharRect should be raw rect value relative to the
+      // base frame.
       lastCharRect = charRect;
+      lastFrame = baseFrame;
+      rv = ConvertToRootRelativeOffset(baseFrame, charRect);
+      if (NS_WARN_IF(NS_FAILED(rv))) {
+        return rv;
+      }
 
       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);