Bug 1286464 part.5 Create ContentEventHandler::EnsureNonEmptyRect() for query various rect event handlers r=smaug
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 05 Aug 2016 13:01:17 +0900
changeset 400376 4ddcedb282564f1c1012f5d47fbd88ae18695e52
parent 400375 49ce9d732d5f160d7214bc6e881944ea0c174864
child 400377 909162ca9772e0a2c8486e1af95751716751476c
push id26138
push usergszorc@mozilla.com
push dateSat, 13 Aug 2016 03:04:36 +0000
reviewerssmaug
bugs1286464
milestone51.0a1
Bug 1286464 part.5 Create ContentEventHandler::EnsureNonEmptyRect() for query various rect event handlers r=smaug This can kill the duplicated code in a lot of places in ContentEventHandler. MozReview-Commit-ID: BRpBkbXyeBs
dom/events/ContentEventHandler.cpp
dom/events/ContentEventHandler.h
--- a/dom/events/ContentEventHandler.cpp
+++ b/dom/events/ContentEventHandler.cpp
@@ -390,16 +390,19 @@ ContentEventHandler::QueryContentRect(ns
     nsRect frameRect(nsPoint(0, 0), frame->GetRect().Size());
     rv = ConvertToRootRelativeOffset(frame, frameRect);
     NS_ENSURE_SUCCESS(rv, rv);
     resultRect.UnionRect(resultRect, frameRect);
   }
 
   aEvent->mReply.mRect = LayoutDeviceIntRect::FromUnknownRect(
       resultRect.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->mSucceeded = true;
 
   return NS_OK;
 }
 
 // Editor places a bogus BR node under its root content if the editor doesn't
 // have any text. This happens even for single line editors.
 // When we get text content and when we change the selection,
@@ -1382,16 +1385,32 @@ ContentEventHandler::OnQueryTextContent(
                "Font ranges doesn't match the string");
   }
 
   aEvent->mSucceeded = true;
 
   return NS_OK;
 }
 
+void
+ContentEventHandler::EnsureNonEmptyRect(nsRect& aRect) const
+{
+  // See the comment in ContentEventHandler.h why this doesn't set them to
+  // one device pixel.
+  aRect.height = std::max(1, aRect.height);
+  aRect.width = std::max(1, aRect.width);
+}
+
+void
+ContentEventHandler::EnsureNonEmptyRect(LayoutDeviceIntRect& aRect) const
+{
+  aRect.height = std::max(1, aRect.height);
+  aRect.width = std::max(1, aRect.width);
+}
+
 ContentEventHandler::NodePosition
 ContentEventHandler::GetNodePositionHavingFlatText(
                        const NodePosition& aNodePosition)
 {
   return GetNodePositionHavingFlatText(aNodePosition.mNode,
                                        aNodePosition.mOffset);
 }
 
@@ -1496,20 +1515,19 @@ ContentEventHandler::OnQueryTextRectArra
 
     for (size_t i = 0; i < charRects.Length(); i++) {
       nsRect charRect = charRects[i];
       charRect.x += frameRect.x;
       charRect.y += frameRect.y;
 
       rect = LayoutDeviceIntRect::FromUnknownRect(
                charRect.ToOutsidePixels(mPresContext->AppUnitsPerDevPixel()));
-
-      // Ensure at least 1px width and height for avoiding empty rect.
-      rect.height = std::max(1, rect.height);
-      rect.width = std::max(1, rect.width);
+      // 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++;
     }
   }
   aEvent->mSucceeded = true;
   return NS_OK;
 }
@@ -1552,24 +1570,26 @@ ContentEventHandler::OnQueryTextRect(Wid
 
   // 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);
-  // minus 1 to avoid creating an empty rect
   if (firstFrame->GetWritingMode().IsVertical()) {
-    rect.y += ptOffset.y - 1;
-    rect.height -= ptOffset.y - 1;
+    rect.y += ptOffset.y;
+    rect.height -= ptOffset.y;
   } else {
-    rect.x += ptOffset.x - 1;
-    rect.width -= ptOffset.x - 1;
+    rect.x += ptOffset.x;
+    rect.width -= ptOffset.x;
   }
+  // 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())) {
     return NS_ERROR_FAILURE;
   }
   nsIFrame* lastFrame = nullptr;
@@ -1595,38 +1615,46 @@ ContentEventHandler::OnQueryTextRect(Wid
       if (!frame) {
         // this can happen when the end offset of the range is 0.
         frame = lastFrame;
       }
     }
     frameRect.SetRect(nsPoint(0, 0), frame->GetRect().Size());
     rv = ConvertToRootRelativeOffset(frame, frameRect);
     NS_ENSURE_SUCCESS(rv, 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);
-  // minus 1 to avoid creating an empty rect
   if (lastFrame->GetWritingMode().IsVertical()) {
-    frameRect.height -= lastFrame->GetRect().height - ptOffset.y - 1;
+    frameRect.height -= lastFrame->GetRect().height - ptOffset.y;
   } else {
-    frameRect.width -= lastFrame->GetRect().width - ptOffset.x - 1;
+    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 == lastFrame) {
     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;
   return NS_OK;
 }
 
 nsresult
 ContentEventHandler::OnQueryEditorRect(WidgetQueryContentEvent* aEvent)
 {
@@ -1665,16 +1693,19 @@ ContentEventHandler::OnQueryCaretRect(Wi
       NS_ENSURE_SUCCESS(rv, rv);
       if (offset == aEvent->mInput.mOffset) {
         rv = ConvertToRootRelativeOffset(caretFrame, caretRect);
         NS_ENSURE_SUCCESS(rv, rv);
         nscoord appUnitsPerDevPixel =
           caretFrame->PresContext()->AppUnitsPerDevPixel();
         aEvent->mReply.mRect = LayoutDeviceIntRect::FromUnknownRect(
           caretRect.ToOutsidePixels(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 = caretFrame->GetWritingMode();
         aEvent->mReply.mOffset = aEvent->mInput.mOffset;
         aEvent->mSucceeded = true;
         return NS_OK;
       }
     }
   }
 
@@ -1716,23 +1747,19 @@ ContentEventHandler::OnQueryCaretRect(Wi
     rect.height = fontMetrics->MaxHeight();
   }
 
   rv = ConvertToRootRelativeOffset(frame, rect);
   NS_ENSURE_SUCCESS(rv, rv);
 
   aEvent->mReply.mRect = LayoutDeviceIntRect::FromUnknownRect(
       rect.ToOutsidePixels(mPresContext->AppUnitsPerDevPixel()));
-  // If the caret rect is empty, let's make it non-empty rect.
-  if (!aEvent->mReply.mRect.width) {
-    aEvent->mReply.mRect.width = 1;
-  }
-  if (!aEvent->mReply.mRect.height) {
-    aEvent->mReply.mRect.height = 1;
-  }
+  // Returning empty rect may cause native IME confused, let's make sure to
+  // return non-empty rect.
+  EnsureNonEmptyRect(aEvent->mReply.mRect);
   aEvent->mSucceeded = true;
   return NS_OK;
 }
 
 nsresult
 ContentEventHandler::OnQueryContentState(WidgetQueryContentEvent* aEvent)
 {
   nsresult rv = Init(aEvent);
--- a/dom/events/ContentEventHandler.h
+++ b/dom/events/ContentEventHandler.h
@@ -336,13 +336,20 @@ 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.
   FrameAndNodeOffset GetFirstFrameHavingFlatTextInRange(nsRange* aRange);
+
+  // Make aRect non-empty.  If width and/or height is 0, these methods set them
+  // to 1.  Note that it doesn't set nsRect's width nor height to one device
+  // pixel because using nsRect::ToOutsidePixels() makes actual width or height
+  // to 2 pixels because x and y may not be aligned to device pixels.
+  void EnsureNonEmptyRect(nsRect& aRect) const;
+  void EnsureNonEmptyRect(LayoutDeviceIntRect& aRect) const;
 };
 
 } // namespace mozilla
 
 #endif // mozilla_ContentEventHandler_h_