- Revision
- 246948
- Author
- timothy_hor...@apple.com
- Date
- 2019-06-28 20:48:18 -0700 (Fri, 28 Jun 2019)
Log 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.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (246947 => 246948)
--- trunk/Source/WebCore/ChangeLog 2019-06-28 23:35:44 UTC (rev 246947)
+++ trunk/Source/WebCore/ChangeLog 2019-06-29 03:48:18 UTC (rev 246948)
@@ -1,3 +1,22 @@
+2019-06-28 Tim Horton <timothy_hor...@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 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 <za...@apple.com>
[Text autosizing][iPadOS] bing.com is hard to read even with boosted text because of the line height
Modified: trunk/Source/WebCore/editing/FrameSelection.cpp (246947 => 246948)
--- trunk/Source/WebCore/editing/FrameSelection.cpp 2019-06-28 23:35:44 UTC (rev 246947)
+++ trunk/Source/WebCore/editing/FrameSelection.cpp 2019-06-29 03:48:18 UTC (rev 246948)
@@ -2288,7 +2288,7 @@
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 @@
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 @@
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;
}
Modified: trunk/Source/WebCore/editing/FrameSelection.h (246947 => 246948)
--- trunk/Source/WebCore/editing/FrameSelection.h 2019-06-28 23:35:44 UTC (rev 246947)
+++ trunk/Source/WebCore/editing/FrameSelection.h 2019-06-29 03:48:18 UTC (rev 246948)
@@ -269,7 +269,8 @@
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;
Modified: trunk/Source/WebCore/page/TextIndicator.cpp (246947 => 246948)
--- trunk/Source/WebCore/page/TextIndicator.cpp 2019-06-28 23:35:44 UTC (rev 246947)
+++ trunk/Source/WebCore/page/TextIndicator.cpp 2019-06-29 03:48:18 UTC (rev 246948)
@@ -380,7 +380,7 @@
// 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;
Modified: trunk/Source/WebKit/ChangeLog (246947 => 246948)
--- trunk/Source/WebKit/ChangeLog 2019-06-28 23:35:44 UTC (rev 246947)
+++ trunk/Source/WebKit/ChangeLog 2019-06-29 03:48:18 UTC (rev 246948)
@@ -1,3 +1,25 @@
+2019-06-28 Tim Horton <timothy_hor...@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 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 <ryanhad...@apple.com>
Unreviewed build fix attempt after r246928.
Modified: trunk/Source/WebKit/WebProcess/WebPage/FindController.cpp (246947 => 246948)
--- trunk/Source/WebKit/WebProcess/WebPage/FindController.cpp 2019-06-28 23:35:44 UTC (rev 246947)
+++ trunk/Source/WebKit/WebProcess/WebPage/FindController.cpp 2019-06-29 03:48:18 UTC (rev 246948)
@@ -499,17 +499,31 @@
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()));
+ IntRect findIndicatorRect = selectedFrame->view()->contentsToRootView(enclosingIntRect(selectedFrame->selection().selectionBounds(FrameSelection::ClipToVisibleContent::No)));
- if (findIndicatorRect != m_findIndicatorRect)
- hideFindIndicator();
+ 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)
Modified: trunk/Source/WebKit/WebProcess/WebPage/FindController.h (246947 => 246948)
--- trunk/Source/WebKit/WebProcess/WebPage/FindController.h 2019-06-28 23:35:44 UTC (rev 246947)
+++ trunk/Source/WebKit/WebProcess/WebPage/FindController.h 2019-06-29 03:48:18 UTC (rev 246948)
@@ -92,6 +92,7 @@
unsigned findIndicatorRadius() const;
bool shouldHideFindIndicatorOnScroll() const;
+ void didScrollAffectingFindIndicatorPosition();
WebPage* m_webPage;
WebCore::PageOverlay* m_findPageOverlay { nullptr };
Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/FindControllerIOS.mm (246947 => 246948)
--- trunk/Source/WebKit/WebProcess/WebPage/ios/FindControllerIOS.mm 2019-06-28 23:35:44 UTC (rev 246947)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/FindControllerIOS.mm 2019-06-29 03:48:18 UTC (rev 246948)
@@ -99,6 +99,7 @@
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);
