Multi-rect TextIndicators are vertically flipped in WebKit1
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 12 Jan 2015 20:22:10 +0000 (20:22 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 12 Jan 2015 20:22:10 +0000 (20:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=140350
<rdar://problem/19441243>

Reviewed by Beth Dakin.

* page/TextIndicator.cpp:
(WebCore::TextIndicator::createWithSelectionInFrame):
(WebCore::TextIndicator::TextIndicator):
* page/TextIndicator.h:
(WebCore::TextIndicator::selectionRectInRootViewCoordinates):
(WebCore::TextIndicator::textBoundingRectInRootViewCoordinates):
(WebCore::TextIndicator::selectionRectInWindowCoordinates): Deleted.
(WebCore::TextIndicator::textBoundingRectInWindowCoordinates): Deleted.
* page/mac/TextIndicatorWindow.mm:
(-[WebTextIndicatorView initWithFrame:textIndicator:margin:]):
(WebCore::TextIndicatorWindow::setTextIndicator):
Compute, store, and use TextIndicator's selectionRect and textBoundingRect
in root view coordinates instead of window coordinates; this way, each
WebKit can do the conversion itself, and the rootView vs. window flipping
isn't wrongly factored into textRectsInBoundingRectCoordinates.

* Shared/WebCoreArgumentCoders.cpp:
(IPC::ArgumentCoder<TextIndicatorData>::encode):
(IPC::ArgumentCoder<TextIndicatorData>::decode):
Adjust to the field name changes.

* UIProcess/API/mac/WKView.mm:
(-[WKView _setTextIndicator:fadeOut:]):
Convert the textBoundingRect from root view to screen coordinates.

* WebProcess/WebPage/FindController.cpp:
(WebKit::FindController::updateFindIndicator):
(WebKit::FindController::drawRect):
Adjust to the new name, and use contentsToRootView when comparing against
the stored m_findIndicatorRect.

* WebView/WebView.mm:
(-[WebView _setTextIndicator:fadeOut:]):
Convert the textBoundingRect from root view to screen coordinates.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@178290 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/page/TextIndicator.cpp
Source/WebCore/page/TextIndicator.h
Source/WebCore/page/mac/TextIndicatorWindow.mm
Source/WebKit/mac/ChangeLog
Source/WebKit/mac/WebView/WebView.mm
Source/WebKit2/ChangeLog
Source/WebKit2/Shared/WebCoreArgumentCoders.cpp
Source/WebKit2/UIProcess/API/mac/WKView.mm
Source/WebKit2/WebProcess/WebPage/FindController.cpp

index a484e92..cb9c279 100644 (file)
@@ -1,3 +1,27 @@
+2015-01-12  Timothy Horton  <timothy_horton@apple.com>
+
+        Multi-rect TextIndicators are vertically flipped in WebKit1
+        https://bugs.webkit.org/show_bug.cgi?id=140350
+        <rdar://problem/19441243>
+
+        Reviewed by Beth Dakin.
+
+        * page/TextIndicator.cpp:
+        (WebCore::TextIndicator::createWithSelectionInFrame):
+        (WebCore::TextIndicator::TextIndicator):
+        * page/TextIndicator.h:
+        (WebCore::TextIndicator::selectionRectInRootViewCoordinates):
+        (WebCore::TextIndicator::textBoundingRectInRootViewCoordinates):
+        (WebCore::TextIndicator::selectionRectInWindowCoordinates): Deleted.
+        (WebCore::TextIndicator::textBoundingRectInWindowCoordinates): Deleted.
+        * page/mac/TextIndicatorWindow.mm:
+        (-[WebTextIndicatorView initWithFrame:textIndicator:margin:]):
+        (WebCore::TextIndicatorWindow::setTextIndicator):
+        Compute, store, and use TextIndicator's selectionRect and textBoundingRect
+        in root view coordinates instead of window coordinates; this way, each
+        WebKit can do the conversion itself, and the rootView vs. window flipping
+        isn't wrongly factored into textRectsInBoundingRectCoordinates.
+
 2015-01-12  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r178281.
index b1d131b..86ae46b 100644 (file)
@@ -146,7 +146,7 @@ PassRefPtr<TextIndicator> TextIndicator::createWithSelectionInFrame(Frame& frame
 
     // Store the selection rect in window coordinates, to be used subsequently
     // to determine if the indicator and selection still precisely overlap.
-    IntRect selectionRectInWindowCoordinates = frame.view()->contentsToWindow(selectionRect);
+    IntRect selectionRectInRootViewCoordinates = frame.view()->contentsToRootView(selectionRect);
 
     Vector<FloatRect> textRects;
     frame.selection().getClippedVisibleTextRectangles(textRects);
@@ -154,23 +154,23 @@ PassRefPtr<TextIndicator> TextIndicator::createWithSelectionInFrame(Frame& frame
     // The bounding rect of all the text rects can be different than the selection
     // rect when the selection spans multiple lines; the indicator doesn't actually
     // care where the selection highlight goes, just where the text actually is.
-    FloatRect textBoundingRectInWindowCoordinates;
-    Vector<FloatRect> textRectsInWindowCoordinates;
+    FloatRect textBoundingRectInRootViewCoordinates;
+    Vector<FloatRect> textRectsInRootViewCoordinates;
     for (const FloatRect& textRect : textRects) {
-        FloatRect textRectInWindowCoordinates = frame.view()->contentsToWindow(enclosingIntRect(textRect));
-        textRectsInWindowCoordinates.append(textRectInWindowCoordinates);
-        textBoundingRectInWindowCoordinates.unite(textRectInWindowCoordinates);
+        FloatRect textRectInRootViewCoordinates = frame.view()->contentsToRootView(enclosingIntRect(textRect));
+        textRectsInRootViewCoordinates.append(textRectInRootViewCoordinates);
+        textBoundingRectInRootViewCoordinates.unite(textRectInRootViewCoordinates);
     }
 
     Vector<FloatRect> textRectsInBoundingRectCoordinates;
-    for (auto rect : textRectsInWindowCoordinates) {
-        rect.moveBy(-textBoundingRectInWindowCoordinates.location());
+    for (auto rect : textRectsInRootViewCoordinates) {
+        rect.moveBy(-textBoundingRectInRootViewCoordinates.location());
         textRectsInBoundingRectCoordinates.append(rect);
     }
 
     TextIndicatorData data;
-    data.selectionRectInWindowCoordinates = selectionRectInWindowCoordinates;
-    data.textBoundingRectInWindowCoordinates = textBoundingRectInWindowCoordinates;
+    data.selectionRectInRootViewCoordinates = selectionRectInRootViewCoordinates;
+    data.textBoundingRectInRootViewCoordinates = textBoundingRectInRootViewCoordinates;
     data.textRectsInBoundingRectCoordinates = textRectsInBoundingRectCoordinates;
     data.contentImageScaleFactor = frame.page()->deviceScaleFactor();
     data.contentImage = indicatorBitmap;
@@ -183,7 +183,7 @@ PassRefPtr<TextIndicator> TextIndicator::createWithSelectionInFrame(Frame& frame
 TextIndicator::TextIndicator(const TextIndicatorData& data)
     : m_data(data)
 {
-    ASSERT(m_data.contentImageScaleFactor != 1 || m_data.contentImage->size() == enclosingIntRect(m_data.selectionRectInWindowCoordinates).size());
+    ASSERT(m_data.contentImageScaleFactor != 1 || m_data.contentImage->size() == enclosingIntRect(m_data.selectionRectInRootViewCoordinates).size());
 
     if (textIndicatorsForTextRectsOverlap(m_data.textRectsInBoundingRectCoordinates)) {
         m_data.textRectsInBoundingRectCoordinates[0] = unionRect(m_data.textRectsInBoundingRectCoordinates);
index 4ea197d..a36a9b6 100644 (file)
@@ -57,8 +57,8 @@ enum class TextIndicatorPresentationTransition {
 };
 
 struct TextIndicatorData {
-    FloatRect selectionRectInWindowCoordinates;
-    FloatRect textBoundingRectInWindowCoordinates;
+    FloatRect selectionRectInRootViewCoordinates;
+    FloatRect textBoundingRectInRootViewCoordinates;
     Vector<FloatRect> textRectsInBoundingRectCoordinates;
     float contentImageScaleFactor;
     RefPtr<Image> contentImageWithHighlight;
@@ -74,8 +74,8 @@ public:
 
     WEBCORE_EXPORT ~TextIndicator();
 
-    FloatRect selectionRectInWindowCoordinates() const { return m_data.selectionRectInWindowCoordinates; }
-    FloatRect textBoundingRectInWindowCoordinates() const { return m_data.textBoundingRectInWindowCoordinates; }
+    FloatRect selectionRectInRootViewCoordinates() const { return m_data.selectionRectInRootViewCoordinates; }
+    FloatRect textBoundingRectInRootViewCoordinates() const { return m_data.textBoundingRectInRootViewCoordinates; }
     const Vector<FloatRect>& textRectsInBoundingRectCoordinates() const { return m_data.textRectsInBoundingRectCoordinates; }
     float contentImageScaleFactor() const { return m_data.contentImageScaleFactor; }
     Image *contentImageWithHighlight() const { return m_data.contentImageWithHighlight.get(); }
index 075c082..be3bf66 100644 (file)
@@ -116,7 +116,7 @@ using namespace WebCore;
     RetainPtr<CGColorRef> gradientDarkColor = [NSColor colorWithDeviceRed:.929 green:.8 blue:0 alpha:1].CGColor;
     RetainPtr<CGColorRef> gradientLightColor = [NSColor colorWithDeviceRed:.949 green:.937 blue:0 alpha:1].CGColor;
 
-    for (auto& textRect : _textIndicator->textRectsInBoundingRectCoordinates()) {
+    for (const auto& textRect : _textIndicator->textRectsInBoundingRectCoordinates()) {
         FloatRect bounceLayerRect = textRect;
         bounceLayerRect.move(_margin.width, _margin.height);
         bounceLayerRect.inflateX(horizontalBorder);
@@ -176,7 +176,7 @@ using namespace WebCore;
         [textLayer setContents:(id)contentsImage.get()];
 
         FloatRect imageRect = textRect;
-        imageRect.move(_textIndicator->textBoundingRectInWindowCoordinates().location() - _textIndicator->selectionRectInWindowCoordinates().location());
+        imageRect.move(_textIndicator->textBoundingRectInRootViewCoordinates().location() - _textIndicator->selectionRectInRootViewCoordinates().location());
         [textLayer setContentsRect:CGRectMake(imageRect.x() / contentsImageLogicalSize.width(), imageRect.y() / contentsImageLogicalSize.height(), imageRect.width() / contentsImageLogicalSize.width(), imageRect.height() / contentsImageLogicalSize.height())];
         [textLayer setContentsGravity:kCAGravityCenter];
         [textLayer setContentsScale:_textIndicator->contentImageScaleFactor()];
@@ -369,14 +369,13 @@ void TextIndicatorWindow::setAnimationProgress(float progress)
     [m_textIndicatorView setAnimationProgress:progress];
 }
 
-void TextIndicatorWindow::setTextIndicator(PassRefPtr<TextIndicator> textIndicator, CGRect contentRect, bool fadeOut)
+void TextIndicatorWindow::setTextIndicator(PassRefPtr<TextIndicator> textIndicator, CGRect textBoundingRectInScreenCoordinates, bool fadeOut)
 {
     if (m_textIndicator == textIndicator)
         return;
 
     m_textIndicator = textIndicator;
 
-    // Get rid of the old window.
     closeWindow();
 
     if (!m_textIndicator)
@@ -386,11 +385,11 @@ void TextIndicatorWindow::setTextIndicator(PassRefPtr<TextIndicator> textIndicat
     CGFloat verticalMargin = dropShadowBlurRadius * 2 + verticalBorder;
     
     if (m_textIndicator->wantsBounce()) {
-        horizontalMargin = std::max(horizontalMargin, contentRect.size.width * (midBounceScale - 1) + horizontalMargin);
-        verticalMargin = std::max(verticalMargin, contentRect.size.height * (midBounceScale - 1) + verticalMargin);
+        horizontalMargin = std::max(horizontalMargin, textBoundingRectInScreenCoordinates.size.width * (midBounceScale - 1) + horizontalMargin);
+        verticalMargin = std::max(verticalMargin, textBoundingRectInScreenCoordinates.size.height * (midBounceScale - 1) + verticalMargin);
     }
 
-    contentRect = CGRectInset(contentRect, -horizontalMargin, -verticalMargin);
+    CGRect contentRect = CGRectInset(textBoundingRectInScreenCoordinates, -horizontalMargin, -verticalMargin);
     NSRect windowContentRect = [NSWindow contentRectForFrameRect:NSIntegralRect(NSRectFromCGRect(contentRect)) styleMask:NSBorderlessWindowMask];
     m_textIndicatorWindow = adoptNS([[NSWindow alloc] initWithContentRect:windowContentRect styleMask:NSBorderlessWindowMask backing:NSBackingStoreBuffered defer:NO]);
 
index 22a7f39..52a337d 100644 (file)
@@ -1,3 +1,15 @@
+2015-01-12  Timothy Horton  <timothy_horton@apple.com>
+
+        Multi-rect TextIndicators are vertically flipped in WebKit1
+        https://bugs.webkit.org/show_bug.cgi?id=140350
+        <rdar://problem/19441243>
+
+        Reviewed by Beth Dakin.
+
+        * WebView/WebView.mm:
+        (-[WebView _setTextIndicator:fadeOut:]):
+        Convert the textBoundingRect from root view to screen coordinates.
+
 2015-01-12  Anders Carlsson  <andersca@apple.com>
 
         Make WebResourceDelegate a formal protocol as well
index f6dc18d..dc39677 100644 (file)
@@ -8653,8 +8653,9 @@ static void glibContextIterationCallback(CFRunLoopObserverRef, CFRunLoopActivity
     if (!_private->textIndicatorWindow)
         _private->textIndicatorWindow = std::make_unique<TextIndicatorWindow>(self);
 
-    NSRect contentRect = [self.window convertRectToScreen:textIndicator->textBoundingRectInWindowCoordinates()];
-    _private->textIndicatorWindow->setTextIndicator(textIndicator, NSRectToCGRect(contentRect), fadeOut);
+    NSRect textBoundingRectInWindowCoordinates = [self convertRect:[self _convertRectFromRootView:textIndicator->textBoundingRectInRootViewCoordinates()] toView:nil];
+    NSRect textBoundingRectInScreenCoordinates = [self.window convertRectToScreen:textBoundingRectInWindowCoordinates];
+    _private->textIndicatorWindow->setTextIndicator(textIndicator, NSRectToCGRect(textBoundingRectInScreenCoordinates), fadeOut);
 }
 
 - (void)_clearTextIndicator
index 04981e2..858d444 100644 (file)
@@ -1,3 +1,26 @@
+2015-01-12  Timothy Horton  <timothy_horton@apple.com>
+
+        Multi-rect TextIndicators are vertically flipped in WebKit1
+        https://bugs.webkit.org/show_bug.cgi?id=140350
+        <rdar://problem/19441243>
+
+        Reviewed by Beth Dakin.
+
+        * Shared/WebCoreArgumentCoders.cpp:
+        (IPC::ArgumentCoder<TextIndicatorData>::encode):
+        (IPC::ArgumentCoder<TextIndicatorData>::decode):
+        Adjust to the field name changes.
+
+        * UIProcess/API/mac/WKView.mm:
+        (-[WKView _setTextIndicator:fadeOut:]):
+        Convert the textBoundingRect from root view to screen coordinates.
+
+        * WebProcess/WebPage/FindController.cpp:
+        (WebKit::FindController::updateFindIndicator):
+        (WebKit::FindController::drawRect):
+        Adjust to the new name, and use contentsToRootView when comparing against
+        the stored m_findIndicatorRect.
+
 2015-01-11  Ryuan Choi  <ryuan.choi@navercorp.com>
 
         [CoordinatedGraphics] Suspend or resume when visibility is changed
index 2b3fb56..ca040f6 100644 (file)
@@ -1963,8 +1963,8 @@ bool ArgumentCoder<BlobPart>::decode(ArgumentDecoder& decoder, BlobPart& blobPar
 
 void ArgumentCoder<TextIndicatorData>::encode(ArgumentEncoder& encoder, const TextIndicatorData& textIndicatorData)
 {
-    encoder << textIndicatorData.selectionRectInWindowCoordinates;
-    encoder << textIndicatorData.textBoundingRectInWindowCoordinates;
+    encoder << textIndicatorData.selectionRectInRootViewCoordinates;
+    encoder << textIndicatorData.textBoundingRectInRootViewCoordinates;
     encoder << textIndicatorData.textRectsInBoundingRectCoordinates;
     encoder << textIndicatorData.contentImageScaleFactor;
     encoder.encodeEnum(textIndicatorData.presentationTransition);
@@ -1982,10 +1982,10 @@ void ArgumentCoder<TextIndicatorData>::encode(ArgumentEncoder& encoder, const Te
 
 bool ArgumentCoder<TextIndicatorData>::decode(ArgumentDecoder& decoder, TextIndicatorData& textIndicatorData)
 {
-    if (!decoder.decode(textIndicatorData.selectionRectInWindowCoordinates))
+    if (!decoder.decode(textIndicatorData.selectionRectInRootViewCoordinates))
         return false;
 
-    if (!decoder.decode(textIndicatorData.textBoundingRectInWindowCoordinates))
+    if (!decoder.decode(textIndicatorData.textBoundingRectInRootViewCoordinates))
         return false;
 
     if (!decoder.decode(textIndicatorData.textRectsInBoundingRectCoordinates))
index ff08d1f..f896950 100644 (file)
@@ -3107,8 +3107,8 @@ static void* keyValueObservingContext = &keyValueObservingContext;
     if (!_data->_textIndicatorWindow)
         _data->_textIndicatorWindow = std::make_unique<TextIndicatorWindow>(self);
 
-    NSRect contentRect = [self.window convertRectToScreen:[self convertRect:textIndicator->textBoundingRectInWindowCoordinates() toView:nil]];
-    _data->_textIndicatorWindow->setTextIndicator(textIndicator, NSRectToCGRect(contentRect), fadeOut);
+    NSRect textBoundingRectInScreenCoordinates = [self.window convertRectToScreen:[self convertRect:textIndicator->textBoundingRectInRootViewCoordinates() toView:nil]];
+    _data->_textIndicatorWindow->setTextIndicator(textIndicator, NSRectToCGRect(textBoundingRectInScreenCoordinates), fadeOut);
 }
 
 - (void)_setTextIndicatorAnimationProgress:(float)progress
index 46d3462..2cd7a37 100644 (file)
@@ -321,7 +321,7 @@ bool FindController::updateFindIndicator(Frame& selectedFrame, bool isShowingOve
     if (!indicator)
         return false;
 
-    m_findIndicatorRect = enclosingIntRect(indicator->selectionRectInWindowCoordinates());
+    m_findIndicatorRect = enclosingIntRect(indicator->selectionRectInRootViewCoordinates());
     m_webPage->send(Messages::WebPageProxy::SetTextIndicator(indicator->data(), !isShowingOverlay));
     m_isShowingFindIndicator = true;
 
@@ -453,7 +453,7 @@ void FindController::drawRect(PageOverlay&, GraphicsContext& graphicsContext, co
         return;
 
     if (Frame* selectedFrame = frameWithSelection(m_webPage->corePage())) {
-        IntRect findIndicatorRect = selectedFrame->view()->contentsToWindow(enclosingIntRect(selectedFrame->selection().selectionBounds()));
+        IntRect findIndicatorRect = selectedFrame->view()->contentsToRootView(enclosingIntRect(selectedFrame->selection().selectionBounds()));
 
         if (findIndicatorRect != m_findIndicatorRect)
             hideFindIndicator();