Changeset 246948 in webkit

Timestamp:

Jun 28, 2019, 8:48:18 PM (6 years ago)

Author:

[email protected]

Message:

iOS WebKit2 find-in-page indicator doesn't move with 'overflow: scroll'
​https://bugs.webkit.org/show_bug.cgi?id=175032
<rdar://problem/29346482>

Reviewed by Wenson Hsieh.

Source/WebCore:

• editing/FrameSelection.cpp:

(WebCore::FrameSelection::selectionBounds const):
(WebCore::FrameSelection::revealSelection):

• editing/FrameSelection.h:

Make selectionBounds' clipToVisibleContent param an enum class.

• page/TextIndicator.cpp:

(WebCore::initializeIndicator):
Save the un-clipped selection rect; otherwise we'll frequently save 0, 0
here when finding a match that is off-screen.

Source/WebKit:

• WebProcess/WebPage/FindController.cpp:

(WebKit::FindController::drawRect):
(WebKit::FindController::didScrollAffectingFindIndicatorPosition):
Adopt the macOS code that notices that the find highlight doesn't match
its original position, but instead of hiding the highlight like we do on macOS,
update it. We do this asynchronously to avoid mutating the layer tree
in the middle of painting, which is not /truly/ unsafe, but definitely
non-ideal and causes fun flashes.

• WebProcess/WebPage/FindController.h:
• WebProcess/WebPage/ios/FindControllerIOS.mm:

(WebKit::FindController::updateFindIndicator):
Store m_findIndicatorRect when updating the indicator, just like we do on macOS.

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/editing/FrameSelection.cpp (modified) (3 diffs)
• WebCore/editing/FrameSelection.h (modified) (1 diff)
• WebCore/page/TextIndicator.cpp (modified) (1 diff)
• WebKit/ChangeLog (modified) (1 diff)
• WebKit/WebProcess/WebPage/FindController.cpp (modified) (1 diff)
• WebKit/WebProcess/WebPage/FindController.h (modified) (1 diff)
• WebKit/WebProcess/WebPage/ios/FindControllerIOS.mm (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-06-28  Tim Horton  <[email protected]>
+
+        iOS WebKit2 find-in-page indicator doesn't move with 'overflow: scroll'
+        https://bugs.webkit.org/show_bug.cgi?id=175032
+        <rdar://problem/29346482>
+
+        Reviewed by Wenson Hsieh.
+
+        * editing/FrameSelection.cpp:
+        (WebCore::FrameSelection::selectionBounds const):
+        (WebCore::FrameSelection::revealSelection):
+        * editing/FrameSelection.h:
+        Make selectionBounds' clipToVisibleContent param an enum class.
+
+        * page/TextIndicator.cpp:
+        (WebCore::initializeIndicator):
+        Save the un-clipped selection rect; otherwise we'll frequently save 0, 0
+        here when finding a match that is off-screen.
+
 2019-06-28  Zalan Bujtas  <[email protected]>
 

trunk/Source/WebCore/editing/FrameSelection.cpp

@@ -2289 +2289 @@
 }
 
-FloatRect FrameSelection::selectionBounds(bool clipToVisibleContent) const
+FloatRect FrameSelection::selectionBounds(ClipToVisibleContent clipToVisibleContent) const
 {
     if (!m_frame->document())
@@ -2300 +2300 @@
 
     auto& selection = renderView->selection();
-    auto selectionRect = clipToVisibleContent ? selection.boundsClippedToVisibleContent() : selection.bounds();
-    return clipToVisibleContent ? intersection(selectionRect, renderView->frameView().visibleContentRect(ScrollableArea::LegacyIOSDocumentVisibleRect)) : selectionRect;
+
+    if (clipToVisibleContent == ClipToVisibleContent::Yes) {
+        auto selectionRect = selection.boundsClippedToVisibleContent();
+        return intersection(selectionRect, renderView->frameView().visibleContentRect(ScrollableArea::LegacyIOSDocumentVisibleRect));
+    }
+
+    return selection.bounds();
 }
 
@@ -2392 +2397 @@
         break;
     case VisibleSelection::RangeSelection:
-        rect = revealExtentOption == RevealExtent ? VisiblePosition(m_selection.extent()).absoluteCaretBounds() : enclosingIntRect(selectionBounds(false));
+        rect = revealExtentOption == RevealExtent ? VisiblePosition(m_selection.extent()).absoluteCaretBounds() : enclosingIntRect(selectionBounds(ClipToVisibleContent::No));
         break;
     }

trunk/Source/WebCore/editing/FrameSelection.h

@@ -270 +270 @@
     void clearTypingStyle();
 
-    WEBCORE_EXPORT FloatRect selectionBounds(bool clipToVisibleContent = true) const;
+    enum class ClipToVisibleContent : uint8_t { No, Yes };
+    WEBCORE_EXPORT FloatRect selectionBounds(ClipToVisibleContent = ClipToVisibleContent::Yes) const;
 
     enum class TextRectangleHeight { TextHeight, SelectionHeight };

trunk/Source/WebCore/page/TextIndicator.cpp

@@ -381 +381 @@
     // Store the selection rect in window coordinates, to be used subsequently
     // to determine if the indicator and selection still precisely overlap.
-    data.selectionRectInRootViewCoordinates = frame.view()->contentsToRootView(enclosingIntRect(frame.selection().selectionBounds()));
+    data.selectionRectInRootViewCoordinates = frame.view()->contentsToRootView(enclosingIntRect(frame.selection().selectionBounds(FrameSelection::ClipToVisibleContent::No)));
     data.textBoundingRectInRootViewCoordinates = textBoundingRectInRootViewCoordinates;
     data.textRectsInBoundingRectCoordinates = textRectsInBoundingRectCoordinates;

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-06-28  Tim Horton  <[email protected]>
+
+        iOS WebKit2 find-in-page indicator doesn't move with 'overflow: scroll'
+        https://bugs.webkit.org/show_bug.cgi?id=175032
+        <rdar://problem/29346482>
+
+        Reviewed by Wenson Hsieh.
+
+        * WebProcess/WebPage/FindController.cpp:
+        (WebKit::FindController::drawRect):
+        (WebKit::FindController::didScrollAffectingFindIndicatorPosition):
+        Adopt the macOS code that notices that the find highlight doesn't match
+        its original position, but instead of hiding the highlight like we do on macOS,
+        update it. We do this asynchronously to avoid mutating the layer tree
+        in the middle of painting, which is not /truly/ unsafe, but definitely
+        non-ideal and causes fun flashes.
+
+        * WebProcess/WebPage/FindController.h:
+        * WebProcess/WebPage/ios/FindControllerIOS.mm:
+        (WebKit::FindController::updateFindIndicator):
+        Store m_findIndicatorRect when updating the indicator, just like we do on macOS.
+
 2019-06-28  Ryan Haddad  <[email protected]>
 

trunk/Source/WebKit/WebProcess/WebPage/FindController.cpp

@@ -500 +500 @@
         graphicsContext.fillPath(path);
 
-    if (!m_isShowingFindIndicator || !shouldHideFindIndicatorOnScroll())
+    if (!m_isShowingFindIndicator)
         return;
 
     if (Frame* selectedFrame = frameWithSelection(m_webPage->corePage())) {
-        IntRect findIndicatorRect = selectedFrame->view()->contentsToRootView(enclosingIntRect(selectedFrame->selection().selectionBounds()));
-
-        if (findIndicatorRect != m_findIndicatorRect)
-            hideFindIndicator();
-    }
+        IntRect findIndicatorRect = selectedFrame->view()->contentsToRootView(enclosingIntRect(selectedFrame->selection().selectionBounds(FrameSelection::ClipToVisibleContent::No)));
+
+        if (findIndicatorRect != m_findIndicatorRect) {
+            // We are underneath painting, so it's not safe to mutate the layer tree synchronously.
+            callOnMainThread([weakWebPage = makeWeakPtr(m_webPage)] {
+                if (!weakWebPage)
+                    return;
+                weakWebPage->findController().didScrollAffectingFindIndicatorPosition();
+            });
+        }
+    }
+}
+
+void FindController::didScrollAffectingFindIndicatorPosition()
+{
+    if (shouldHideFindIndicatorOnScroll())
+        hideFindIndicator();
+    else if (Frame *selectedFrame = frameWithSelection(m_webPage->corePage()))
+        updateFindIndicator(*selectedFrame, true, false);
 }
 

trunk/Source/WebKit/WebProcess/WebPage/FindController.h

@@ -93 +93 @@
     unsigned findIndicatorRadius() const;
     bool shouldHideFindIndicatorOnScroll() const;
+    void didScrollAffectingFindIndicatorPosition();
 
     WebPage* m_webPage;

trunk/Source/WebKit/WebProcess/WebPage/ios/FindControllerIOS.mm

@@ -100 +100 @@
 
     m_findIndicatorOverlayClient = std::make_unique<FindIndicatorOverlayClientIOS>(selectedFrame, textIndicator.get());
+    m_findIndicatorRect = enclosingIntRect(textIndicator->selectionRectInRootViewCoordinates());
     m_findIndicatorOverlay = PageOverlay::create(*m_findIndicatorOverlayClient, PageOverlay::OverlayType::Document);
     m_webPage->corePage()->pageOverlayController().installPageOverlay(*m_findIndicatorOverlay, PageOverlay::FadeMode::DoNotFade);