Attachment 373166 Details for Bug 175032 – Patch

[patch] Patch

bug-175032-20190628171401.patch (text/plain), 10.15 KB, created by Tim Horton on 2019-06-28 17:14:01 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Tim Horton

Created: 2019-06-28 17:14:01 PDT

Size: 10.15 KB

patch

obsolete

>Subversion Revision: 246908
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 596824f4ada88cfc9c95aff5b4ee73979fb1481e..e2bb37b60fc052a10be6cb8917f02317f95cd0b6 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,22 @@
>+2019-06-28  Tim Horton  <timothy_horton@apple.com>
>+
>+        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 NOBODY (OOPS!).
>+
>+        * 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-27  Simon Fraser  <simon.fraser@apple.com>
> 
>         REGRESSION (r246869): ASSERTION FAILED: !renderer().hasRepaintLayoutRects() || renderer().repaintLayoutRects().m_repaintRect == renderer().clippedOverflowRectForRepaint(renderer().containerForRepaint())
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index f9d9b1731875be17e94a592b03c548eceaea5699..674e6fc74ae4653152d1242b05f35eef519c9499 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,25 @@
>+2019-06-28  Tim Horton  <timothy_horton@apple.com>
>+
>+        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 NOBODY (OOPS!).
>+
>+        * 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-27  Daniel Bates  <dabates@apple.com>
> 
>         [iOS] Select all with existing range selection replaces range instead of selecting all text
>diff --git a/Source/WebCore/editing/FrameSelection.cpp b/Source/WebCore/editing/FrameSelection.cpp
>index c34fa7ced479bc90434d4a80bab5da913ff196dc..34de896e3099dae6c14fc5bf083a8e6e409a05ca 100644
>--- a/Source/WebCore/editing/FrameSelection.cpp
>+++ b/Source/WebCore/editing/FrameSelection.cpp
>@@ -2288,7 +2288,7 @@ bool FrameSelection::shouldDeleteSelection(const VisibleSelection& selection) co
>     return m_frame->editor().client()->shouldDeleteRange(selection.toNormalizedRange().get());
> }
> 
>-FloatRect FrameSelection::selectionBounds(bool clipToVisibleContent) const
>+FloatRect FrameSelection::selectionBounds(ClipToVisibleContent clipToVisibleContent) const
> {
>     if (!m_frame->document())
>         return LayoutRect();
>@@ -2299,8 +2299,13 @@ FloatRect FrameSelection::selectionBounds(bool clipToVisibleContent) const
>         return LayoutRect();
> 
>     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();
> }
> 
> void FrameSelection::getClippedVisibleTextRectangles(Vector<FloatRect>& rectangles, TextRectangleHeight textRectHeight) const
>@@ -2391,7 +2396,7 @@ void FrameSelection::revealSelection(SelectionRevealMode revealMode, const Scrol
>         rect = absoluteCaretBounds(&insideFixed);
>         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;
>     }
> 
>diff --git a/Source/WebCore/editing/FrameSelection.h b/Source/WebCore/editing/FrameSelection.h
>index 1166a1e2a921d8ac6da3b3e1936a4113a3462742..ac3fadacd479e70303dc1ed6719780832415e96f 100644
>--- a/Source/WebCore/editing/FrameSelection.h
>+++ b/Source/WebCore/editing/FrameSelection.h
>@@ -269,7 +269,8 @@ public:
>     void setTypingStyle(RefPtr<EditingStyle>&& style) { m_typingStyle = WTFMove(style); }
>     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 };
>     WEBCORE_EXPORT void getClippedVisibleTextRectangles(Vector<FloatRect>&, TextRectangleHeight = TextRectangleHeight::SelectionHeight) const;
>diff --git a/Source/WebCore/page/TextIndicator.cpp b/Source/WebCore/page/TextIndicator.cpp
>index a05d2eb8d75e30707ecb524be80cab618e45baa3..9e62b14dd0c19c71be9e071384a00d6fb591125e 100644
>--- a/Source/WebCore/page/TextIndicator.cpp
>+++ b/Source/WebCore/page/TextIndicator.cpp
>@@ -380,7 +380,7 @@ static bool initializeIndicator(TextIndicatorData& data, Frame& frame, const Ran
> 
>     // 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;
> 
>diff --git a/Source/WebKit/WebProcess/WebPage/FindController.cpp b/Source/WebKit/WebProcess/WebPage/FindController.cpp
>index 63cc28392e480dfbd5432d69000fa38f22de662f..4ec65b7806e7382e421519e5d740ef80242c3e0f 100644
>--- a/Source/WebKit/WebProcess/WebPage/FindController.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/FindController.cpp
>@@ -499,17 +499,31 @@ void FindController::drawRect(PageOverlay&, GraphicsContext& graphicsContext, co
>     for (auto& path : whiteFramePaths)
>         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);
>+}
>+
> bool FindController::mouseEvent(PageOverlay&, const PlatformMouseEvent& mouseEvent)
> {
>     if (mouseEvent.type() == PlatformEvent::MousePressed)
>diff --git a/Source/WebKit/WebProcess/WebPage/FindController.h b/Source/WebKit/WebProcess/WebPage/FindController.h
>index e2f4a110d02ac3a45318289675d6a0098fbc01af..983cb3361ff8b93029dbd73ec036265dd32f886f 100644
>--- a/Source/WebKit/WebProcess/WebPage/FindController.h
>+++ b/Source/WebKit/WebProcess/WebPage/FindController.h
>@@ -92,6 +92,7 @@ private:
>     
>     unsigned findIndicatorRadius() const;
>     bool shouldHideFindIndicatorOnScroll() const;
>+    void didScrollAffectingFindIndicatorPosition();
> 
>     WebPage* m_webPage;
>     WebCore::PageOverlay* m_findPageOverlay { nullptr };
>diff --git a/Source/WebKit/WebProcess/WebPage/ios/FindControllerIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/FindControllerIOS.mm
>index dcb64e933c42091e56f402202a218c881071c6d0..9abb2a79299d596b91e6ef61ca43ef86190a9049 100644
>--- a/Source/WebKit/WebProcess/WebPage/ios/FindControllerIOS.mm
>+++ b/Source/WebKit/WebProcess/WebPage/ios/FindControllerIOS.mm
>@@ -99,6 +99,7 @@ bool FindController::updateFindIndicator(Frame& selectedFrame, bool isShowingOve
>         return false;
> 
>     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);
>

Actions: View | Formatted Diff | Diff

Attachments on bug 175032: 316865 | 335689 | 373166