NSTextFinder holes don't scroll with the page
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Jul 2019 19:46:32 +0000 (19:46 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Jul 2019 19:46:32 +0000 (19:46 +0000)
https://bugs.webkit.org/show_bug.cgi?id=199815
<rdar://problem/52280514>

Reviewed by Simon Fraser.

Source/WebCore:

* rendering/ScrollAlignment.h:

Source/WebKit:

* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _usePlatformFindUI]):
(-[WKWebView _setUsePlatformFindUI:]):
(-[WKWebView _ensureTextFinderClient]):
Add a bit so that clients can choose whether they want our find UI,
which respects scrolling, or the system find UI, which works when
doing multi-document searches. Right now you can't have both.

(-[WKWebView scrollFindMatchToVisible:]):
Implement and plumb scrollFindMatchToVisible.

* UIProcess/API/Cocoa/WKWebViewPrivate.h:
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::indicateFindMatch):
* UIProcess/WebPageProxy.h:
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::indicateFindMatch):
* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/WebPage.messages.in:
Plumb indicateFindMatch to FindController.

* UIProcess/mac/WKTextFinderClient.h:
* UIProcess/mac/WKTextFinderClient.mm:
(-[WKTextFinderClient initWithPage:view:usePlatformFindUI:]):
Store whether or not we are using the platform find UI.

(-[WKTextFinderClient findMatchesForString:relativeToMatch:findOptions:maxResults:resultCollector:]):
If using WebKit find UI, turn on our UI in the options passed to findString[Matches].

(-[WKTextFinderClient scrollFindMatchToVisible:]):
If using WebKit find UI, when the platform tells us to reveal a find match, indicate it.

(-[WKTextFinderClient didFindStringMatchesWithRects:didWrapAround:]):
If using WebKit find UI, lie to the platform and return no rects so that it doesn't paint a yellow rectangle.

(-[WKTextFinderClient getImageForMatchResult:completionHandler:]):
If using WebKit find UI, lie to the platform and return no image so that it doesn't paint a yellow rectangle.

(-[WKTextFinderClient initWithPage:view:]): Deleted.

* WebProcess/WebPage/FindController.cpp:
(WebKit::FindController::updateFindUIAfterPageScroll):
Make it possible to use our find UI with 'findStringMatches'; since the platform
find infrastrucutre depends on knowing about all matches up front, we need
to use 'findStringMatches' instead of 'findString', but we had never combined
that with our UI. Don't throw away the other matches when we indicate one, and
don't send DidFindString in the case we are coming from 'findStringMatches'.
This all needs a bit of cleanup in the future.

(WebKit::FindController::findString):
Tell updateFindUIAfterPageScroll that we are 'findString' so that it uses its normal behavior.

(WebKit::FindController::findStringMatches):
Tell updateFindUIAfterPageScroll that we are 'findStringMatches' so that it uses the adjusted behavior.

(WebKit::FindController::indicateFindMatch):
Add 'indicateFindMatch', which both selects the match and moves the indicator there.

* WebProcess/WebPage/FindController.h:

Tools:

* MiniBrowser/mac/WK2BrowserWindowController.m:
(-[MiniBrowserNSTextFinder performAction:]):
(-[WK2BrowserWindowController awakeFromNib]):
Adopt the WebKit UI in MiniBrowser. Also, override NSTextFinder's
-performAction: so that we can intercept the close action and
hide our own find UI, since there is no delegate for that action yet.

* TestWebKitAPI/Tests/WebKitCocoa/FindInPage.mm:
(TEST):

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

18 files changed:
Source/WebCore/ChangeLog
Source/WebCore/PAL/pal/spi/mac/NSTextFinderSPI.h
Source/WebCore/rendering/ScrollAlignment.h
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebPageProxy.h
Source/WebKit/UIProcess/mac/WKTextFinderClient.h
Source/WebKit/UIProcess/mac/WKTextFinderClient.mm
Source/WebKit/WebProcess/WebPage/FindController.cpp
Source/WebKit/WebProcess/WebPage/FindController.h
Source/WebKit/WebProcess/WebPage/WebPage.cpp
Source/WebKit/WebProcess/WebPage/WebPage.h
Source/WebKit/WebProcess/WebPage/WebPage.messages.in
Tools/ChangeLog
Tools/MiniBrowser/mac/WK2BrowserWindowController.m
Tools/TestWebKitAPI/Tests/WebKitCocoa/FindInPage.mm

index 8e92e15..58c9895 100644 (file)
@@ -1,3 +1,13 @@
+2019-07-16  Tim Horton  <timothy_horton@apple.com>
+
+        NSTextFinder holes don't scroll with the page
+        https://bugs.webkit.org/show_bug.cgi?id=199815
+        <rdar://problem/52280514>
+
+        Reviewed by Simon Fraser.
+
+        * rendering/ScrollAlignment.h:
+
 2019-07-16  Youenn Fablet  <youenn@apple.com>
 
         [iOS] Make sure the first camera device in the list is the front camera
index 4fd8090..93f4ff9 100644 (file)
@@ -52,6 +52,7 @@ typedef enum : NSUInteger {
 - (void)getSelectedText:(void (^)(NSString *selectedTextString))completionHandler;
 - (void)selectFindMatch:(id <NSTextFinderAsynchronousDocumentFindMatch>)findMatch completionHandler:(void (^)(void))completionHandler;
 - (void)replaceMatches:(NSArray *)matches withString:(NSString *)replacementString inSelectionOnly:(BOOL)selectionOnly resultCollector:(void (^)(NSUInteger replacementCount))resultCollector;
+- (void)scrollFindMatchToVisible:(id <NSTextFinderAsynchronousDocumentFindMatch>)findMatch;
 
 @end
 
index 02c80cd..694008e 100644 (file)
@@ -67,7 +67,7 @@ struct ScrollAlignment {
 
     static const ScrollAlignment alignCenterIfNotVisible;
     static const ScrollAlignment alignToEdgeIfNotVisible;
-    static const ScrollAlignment alignCenterIfNeeded;
+    WEBCORE_EXPORT static const ScrollAlignment alignCenterIfNeeded;
     WEBCORE_EXPORT static const ScrollAlignment alignToEdgeIfNeeded;
     WEBCORE_EXPORT static const ScrollAlignment alignCenterAlways;
     static const ScrollAlignment alignTopAlways;
index c36bc25..830b6dc 100644 (file)
@@ -1,3 +1,71 @@
+2019-07-16  Tim Horton  <timothy_horton@apple.com>
+
+        NSTextFinder holes don't scroll with the page
+        https://bugs.webkit.org/show_bug.cgi?id=199815
+        <rdar://problem/52280514>
+
+        Reviewed by Simon Fraser.
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _usePlatformFindUI]):
+        (-[WKWebView _setUsePlatformFindUI:]):
+        (-[WKWebView _ensureTextFinderClient]):
+        Add a bit so that clients can choose whether they want our find UI,
+        which respects scrolling, or the system find UI, which works when
+        doing multi-document searches. Right now you can't have both.
+
+        (-[WKWebView scrollFindMatchToVisible:]):
+        Implement and plumb scrollFindMatchToVisible.
+
+        * UIProcess/API/Cocoa/WKWebViewPrivate.h:
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::indicateFindMatch):
+        * UIProcess/WebPageProxy.h:
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::indicateFindMatch):
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/WebPage.messages.in:
+        Plumb indicateFindMatch to FindController.
+
+        * UIProcess/mac/WKTextFinderClient.h:
+        * UIProcess/mac/WKTextFinderClient.mm:
+        (-[WKTextFinderClient initWithPage:view:usePlatformFindUI:]):
+        Store whether or not we are using the platform find UI.
+
+        (-[WKTextFinderClient findMatchesForString:relativeToMatch:findOptions:maxResults:resultCollector:]):
+        If using WebKit find UI, turn on our UI in the options passed to findString[Matches].
+
+        (-[WKTextFinderClient scrollFindMatchToVisible:]):
+        If using WebKit find UI, when the platform tells us to reveal a find match, indicate it.
+
+        (-[WKTextFinderClient didFindStringMatchesWithRects:didWrapAround:]):
+        If using WebKit find UI, lie to the platform and return no rects so that it doesn't paint a yellow rectangle.
+
+        (-[WKTextFinderClient getImageForMatchResult:completionHandler:]):
+        If using WebKit find UI, lie to the platform and return no image so that it doesn't paint a yellow rectangle.
+
+        (-[WKTextFinderClient initWithPage:view:]): Deleted.
+
+        * WebProcess/WebPage/FindController.cpp:
+        (WebKit::FindController::updateFindUIAfterPageScroll):
+        Make it possible to use our find UI with 'findStringMatches'; since the platform
+        find infrastrucutre depends on knowing about all matches up front, we need
+        to use 'findStringMatches' instead of 'findString', but we had never combined
+        that with our UI. Don't throw away the other matches when we indicate one, and
+        don't send DidFindString in the case we are coming from 'findStringMatches'.
+        This all needs a bit of cleanup in the future.
+
+        (WebKit::FindController::findString):
+        Tell updateFindUIAfterPageScroll that we are 'findString' so that it uses its normal behavior.
+
+        (WebKit::FindController::findStringMatches):
+        Tell updateFindUIAfterPageScroll that we are 'findStringMatches' so that it uses the adjusted behavior.
+
+        (WebKit::FindController::indicateFindMatch):
+        Add 'indicateFindMatch', which both selects the match and moves the indicator there.
+
+        * WebProcess/WebPage/FindController.h:
+
 2019-07-16  Chris Dumez  <cdumez@apple.com>
 
         Speed up StorageManager::getValues()
index 5b4f51f..46647ce 100644 (file)
@@ -273,6 +273,8 @@ static Optional<WebCore::ScrollbarOverlayStyle> toCoreScrollbarStyle(_WKOverlayS
     Optional<BOOL> _resolutionForShareSheetImmediateCompletionForTesting;
     RetainPtr<WKSafeBrowsingWarning> _safeBrowsingWarning;
 
+    BOOL _usePlatformFindUI;
+
 #if PLATFORM(IOS_FAMILY)
     RetainPtr<_WKRemoteObjectRegistry> _remoteObjectRegistry;
 
@@ -770,6 +772,8 @@ static void validate(WKWebViewConfiguration *configuration)
 
     _iconLoadingDelegate = std::make_unique<WebKit::IconLoadingDelegate>(self);
 
+    _usePlatformFindUI = YES;
+
     [self _setUpSQLiteDatabaseTrackerClient];
 
     for (auto& pair : pageConfiguration->urlSchemeHandlers())
@@ -4319,10 +4323,24 @@ IGNORE_WARNINGS_END
 }
 #endif
 
+- (BOOL)_usePlatformFindUI
+{
+    return _usePlatformFindUI;
+}
+
+- (void)_setUsePlatformFindUI:(BOOL)usePlatformFindUI
+{
+    _usePlatformFindUI = usePlatformFindUI;
+
+    if (_textFinderClient)
+        [self _hideFindUI];
+    _textFinderClient = nil;
+}
+
 - (WKTextFinderClient *)_ensureTextFinderClient
 {
     if (!_textFinderClient)
-        _textFinderClient = adoptNS([[WKTextFinderClient alloc] initWithPage:*_page view:self]);
+        _textFinderClient = adoptNS([[WKTextFinderClient alloc] initWithPage:*_page view:self usePlatformFindUI:_usePlatformFindUI]);
     return _textFinderClient.get();
 }
 
@@ -4336,6 +4354,11 @@ IGNORE_WARNINGS_END
     [[self _ensureTextFinderClient] replaceMatches:matches withString:replacementString inSelectionOnly:selectionOnly resultCollector:resultCollector];
 }
 
+- (void)scrollFindMatchToVisible:(id<NSTextFinderAsynchronousDocumentFindMatch>)match
+{
+    [[self _ensureTextFinderClient] scrollFindMatchToVisible:match];
+}
+
 - (NSView *)documentContainerView
 {
     return self;
index dc58ac7..739bcad 100644 (file)
@@ -359,6 +359,9 @@ typedef NS_OPTIONS(NSUInteger, _WKRectEdge) {
 @property (nonatomic, setter=_setThumbnailView:) _WKThumbnailView *_thumbnailView WK_API_AVAILABLE(macos(10.13.4));
 @property (nonatomic, setter=_setIgnoresAllEvents:) BOOL _ignoresAllEvents WK_API_AVAILABLE(macos(10.13.4));
 
+// Defaults to YES; if set to NO, WebKit will draw the grey wash and highlights itself.
+@property (nonatomic, setter=_setUsePlatformFindUI:) BOOL _usePlatformFindUI WK_API_AVAILABLE(macos(WK_MAC_TBA));
+
 #endif
 
 - (WKNavigation *)_reloadWithoutContentBlockers WK_API_AVAILABLE(macos(10.12), ios(10.0));
index 2af0e81..18ca17f 100644 (file)
@@ -3574,6 +3574,11 @@ void WebPageProxy::selectFindMatch(int32_t matchIndex)
     m_process->send(Messages::WebPage::SelectFindMatch(matchIndex), m_pageID);
 }
 
+void WebPageProxy::indicateFindMatch(int32_t matchIndex)
+{
+    m_process->send(Messages::WebPage::IndicateFindMatch(matchIndex), m_pageID);
+}
+
 void WebPageProxy::hideFindUI()
 {
     m_process->send(Messages::WebPage::HideFindUI(), m_pageID);
index ad24dc4..01831b9 100644 (file)
@@ -983,6 +983,7 @@ public:
     void findStringMatches(const String&, FindOptions, unsigned maxMatchCount);
     void getImageForFindMatch(int32_t matchIndex);
     void selectFindMatch(int32_t matchIndex);
+    void indicateFindMatch(int32_t matchIndex);
     void didGetImageForFindMatch(const ShareableBitmap::Handle& contentImageHandle, uint32_t matchIndex);
     void hideFindUI();
     void countStringMatches(const String&, FindOptions, unsigned maxMatchCount);
index c18ab4f..79ffecf 100644 (file)
@@ -38,7 +38,7 @@ class WebPageProxy;
 
 @interface WKTextFinderClient : NSObject
 
-- (instancetype)initWithPage:(WebKit::WebPageProxy&)page view:(NSView *)view;
+- (instancetype)initWithPage:(WebKit::WebPageProxy&)page view:(NSView *)view usePlatformFindUI:(BOOL)usePlatformFindUI;
 - (void)willDestroyView:(NSView *)view;
 
 @end
index 3c72858..b7f6c2f 100644 (file)
@@ -72,9 +72,19 @@ private:
         [m_textFinderClient didGetImageForMatchResult:image];
     }
 
-    void didFindString(WebPageProxy*, const String&, const Vector<WebCore::IntRect>& matchRects, uint32_t, int32_t, bool didWrapAround) override
+    void didFindString(WebPageProxy*, const String&, const Vector<WebCore::IntRect>& matchRects, uint32_t matchCount, int32_t matchIndex, bool didWrapAround) override
     {
-        [m_textFinderClient didFindStringMatchesWithRects:{ matchRects } didWrapAround:didWrapAround];
+        Vector<Vector<WebCore::IntRect>> allMatches;
+        if (matchCount != static_cast<unsigned>(kWKMoreThanMaximumMatchCount)) {
+            // Synthesize a vector of match rects for all `matchCount` matches,
+            // filling in the actual rects for the one that we received.
+            // The rest will remain empty, but it's important to NSTextFinder
+            // that they at least exist.
+            allMatches.resize(matchCount);
+            allMatches[matchIndex].appendVector(matchRects);
+        }
+
+        [m_textFinderClient didFindStringMatchesWithRects:allMatches didWrapAround:didWrapAround];
     }
 
     void didFailToFindString(WebPageProxy*, const String& string) override
@@ -138,13 +148,14 @@ private:
 @end
 
 @implementation WKTextFinderClient {
-    WebKit::WebPageProxy *_page;
+    WebKit::WebPageProxy_page;
     NSView *_view;
     Deque<WTF::Function<void(NSArray *, bool didWrap)>> _findReplyCallbacks;
     Deque<WTF::Function<void(NSImage *)>> _imageReplyCallbacks;
+    BOOL _usePlatformFindUI;
 }
 
-- (instancetype)initWithPage:(WebKit::WebPageProxy&)page view:(NSView *)view
+- (instancetype)initWithPage:(WebKit::WebPageProxy&)page view:(NSView *)view usePlatformFindUI:(BOOL)usePlatformFindUI
 {
     self = [super init];
 
@@ -153,6 +164,7 @@ private:
 
     _page = &page;
     _view = view;
+    _usePlatformFindUI = usePlatformFindUI;
     
     _page->setFindMatchesClient(std::make_unique<WebKit::TextFinderFindClient>(self));
     _page->setFindClient(std::make_unique<WebKit::TextFinderFindClient>(self));
@@ -202,6 +214,12 @@ private:
     if (findOptions & NSTextFinderAsynchronousDocumentFindOptionsStartsWith)
         kitFindOptions |= WebKit::FindOptionsAtWordStarts;
 
+    if (!_usePlatformFindUI) {
+        kitFindOptions |= WebKit::FindOptionsShowOverlay;
+        kitFindOptions |= WebKit::FindOptionsShowFindIndicator;
+        kitFindOptions |= WebKit::FindOptionsDetermineMatchIndex;
+    }
+
     RetainPtr<NSProgress> progress = [NSProgress progressWithTotalUnitCount:1];
     auto copiedResultCollector = Block_copy(resultCollector);
     _findReplyCallbacks.append([progress, copiedResultCollector] (NSArray *matches, bool didWrap) {
@@ -235,6 +253,17 @@ private:
     _page->selectFindMatch(textFinderMatch.index);
 }
 
+- (void)scrollFindMatchToVisible:(id <NSTextFinderAsynchronousDocumentFindMatch>)findMatch
+{
+    if (_usePlatformFindUI)
+        return;
+
+    ASSERT([findMatch isKindOfClass:[WKTextFinderMatch class]]);
+
+    WKTextFinderMatch *textFinderMatch = static_cast<WKTextFinderMatch *>(findMatch);
+    _page->indicateFindMatch(textFinderMatch.index);
+}
+
 #pragma mark - FindMatchesClient
 
 static RetainPtr<NSArray> arrayFromRects(const Vector<WebCore::IntRect>& matchRects)
@@ -254,7 +283,12 @@ static RetainPtr<NSArray> arrayFromRects(const Vector<WebCore::IntRect>& matchRe
     unsigned matchCount = rectsForMatches.size();
     RetainPtr<NSMutableArray> matchObjects = adoptNS([[NSMutableArray alloc] initWithCapacity:matchCount]);
     for (unsigned i = 0; i < matchCount; i++) {
-        RetainPtr<NSArray> nsMatchRects = arrayFromRects(rectsForMatches[i]);
+        RetainPtr<NSArray> nsMatchRects;
+
+        if (_usePlatformFindUI)
+            nsMatchRects = arrayFromRects(rectsForMatches[i]);
+        else
+            nsMatchRects = @[];
         RetainPtr<WKTextFinderMatch> match = adoptNS([[WKTextFinderMatch alloc] initWithClient:self view:_view index:i rects:nsMatchRects.get()]);
         [matchObjects addObject:match.get()];
     }
@@ -278,6 +312,11 @@ static RetainPtr<NSArray> arrayFromRects(const Vector<WebCore::IntRect>& matchRe
 
 - (void)getImageForMatchResult:(id <NSTextFinderAsynchronousDocumentFindMatch>)findMatch completionHandler:(void (^)(NSImage *generatedImage))completionHandler
 {
+    if (!_usePlatformFindUI) {
+        completionHandler(nil);
+        return;
+    }
+
     ASSERT([findMatch isKindOfClass:[WKTextFinderMatch class]]);
 
     WKTextFinderMatch *textFinderMatch = static_cast<WKTextFinderMatch *>(findMatch);
index 4ec65b7..1281cbd 100644 (file)
@@ -130,7 +130,7 @@ static Frame* frameWithSelection(Page* page)
     return 0;
 }
 
-void FindController::updateFindUIAfterPageScroll(bool found, const String& string, FindOptions options, unsigned maxMatchCount, DidWrap didWrap)
+void FindController::updateFindUIAfterPageScroll(bool found, const String& string, FindOptions options, unsigned maxMatchCount, DidWrap didWrap, FindUIOriginator originator)
 {
     Frame* selectedFrame = frameWithSelection(m_webPage->corePage());
     
@@ -190,14 +190,21 @@ void FindController::updateFindUIAfterPageScroll(bool found, const String& strin
                 m_foundStringMatchIndex -= matchCount;
         }
 
-        m_findMatches.clear();
-        Vector<IntRect> matchRects;
-        if (auto range = m_webPage->corePage()->selection().firstRange()) {
-            range->absoluteTextRects(matchRects);
-            m_findMatches.append(range);
-        }
+        // If updating UI after finding an individual match, update the current
+        // match rects and inform the UI process that we succeeded.
+        // If we're doing a multi-result search and just updating the indicator,
+        // this would blow away the results for the other matches.
+        // FIXME: This whole class needs a much clearer division between these two paths.
+        if (originator == FindUIOriginator::FindString) {
+            m_findMatches.clear();
+            Vector<IntRect> matchRects;
+            if (auto range = m_webPage->corePage()->selection().firstRange()) {
+                range->absoluteTextRects(matchRects);
+                m_findMatches.append(range);
+            }
 
-        m_webPage->send(Messages::WebPageProxy::DidFindString(string, matchRects, matchCount, m_foundStringMatchIndex, didWrap == DidWrap::Yes));
+            m_webPage->send(Messages::WebPageProxy::DidFindString(string, matchRects, matchCount, m_foundStringMatchIndex, didWrap == DidWrap::Yes));
+        }
     }
 
     if (!shouldShowOverlay) {
@@ -267,7 +274,7 @@ void FindController::findString(const String& string, FindOptions options, unsig
 
     RefPtr<WebPage> protectedWebPage = m_webPage;
     m_webPage->drawingArea()->dispatchAfterEnsuringUpdatedScrollPosition([protectedWebPage, found, string, options, maxMatchCount, didWrap] () {
-        protectedWebPage->findController().updateFindUIAfterPageScroll(found, string, options, maxMatchCount, didWrap);
+        protectedWebPage->findController().updateFindUIAfterPageScroll(found, string, options, maxMatchCount, didWrap, FindUIOriginator::FindString);
     });
 }
 
@@ -286,6 +293,14 @@ void FindController::findStringMatches(const String& string, FindOptions options
     }
 
     m_webPage->send(Messages::WebPageProxy::DidFindStringMatches(string, matchRects, indexForSelection));
+
+    if (!(options & FindOptionsShowOverlay || options & FindOptionsShowFindIndicator))
+        return;
+
+    bool found = !m_findMatches.isEmpty();
+    m_webPage->drawingArea()->dispatchAfterEnsuringUpdatedScrollPosition([protectedWebPage = makeRefPtr(m_webPage), found, string, options, maxMatchCount] () {
+        protectedWebPage->findController().updateFindUIAfterPageScroll(found, string, options, maxMatchCount, DidWrap::No, FindUIOriginator::FindStringMatches);
+    });
 }
 
 void FindController::getImageForFindMatch(uint32_t matchIndex)
@@ -325,6 +340,19 @@ void FindController::selectFindMatch(uint32_t matchIndex)
     frame->selection().setSelection(VisibleSelection(*m_findMatches[matchIndex]));
 }
 
+void FindController::indicateFindMatch(uint32_t matchIndex)
+{
+    selectFindMatch(matchIndex);
+
+    Frame* selectedFrame = frameWithSelection(m_webPage->corePage());
+    if (!selectedFrame)
+        return;
+
+    selectedFrame->selection().revealSelection();
+
+    updateFindIndicator(*selectedFrame, !!m_findPageOverlay);
+}
+
 void FindController::hideFindUI()
 {
     m_findMatches.clear();
index 983cb33..0f26403 100644 (file)
@@ -59,6 +59,7 @@ public:
     void findStringMatches(const String&, FindOptions, unsigned maxMatchCount);
     void getImageForFindMatch(uint32_t matchIndex);
     void selectFindMatch(uint32_t matchIndex);
+    void indicateFindMatch(uint32_t matchIndex);
     void hideFindUI();
     void countStringMatches(const String&, FindOptions, unsigned maxMatchCount);
     uint32_t replaceMatches(const Vector<uint32_t>& matchIndices, const String& replacementText, bool selectionOnly);
@@ -83,7 +84,8 @@ private:
     Vector<WebCore::FloatRect> rectsForTextMatchesInRect(WebCore::IntRect clipRect);
     bool updateFindIndicator(WebCore::Frame& selectedFrame, bool isShowingOverlay, bool shouldAnimate = true);
 
-    void updateFindUIAfterPageScroll(bool found, const String&, FindOptions, unsigned maxMatchCount, WebCore::DidWrap);
+    enum class FindUIOriginator : uint8_t { FindString, FindStringMatches };
+    void updateFindUIAfterPageScroll(bool found, const String&, FindOptions, unsigned maxMatchCount, WebCore::DidWrap, FindUIOriginator);
 
     void willFindString();
     void didFindString();
index 06c7316..1005db5 100644 (file)
@@ -4087,6 +4087,11 @@ void WebPage::selectFindMatch(uint32_t matchIndex)
     findController().selectFindMatch(matchIndex);
 }
 
+void WebPage::indicateFindMatch(uint32_t matchIndex)
+{
+    findController().indicateFindMatch(matchIndex);
+}
+
 void WebPage::hideFindUI()
 {
     findController().hideFindUI();
index 0cbb8c6..eb9728c 100644 (file)
@@ -1438,6 +1438,7 @@ private:
     void findStringMatches(const String&, uint32_t findOptions, uint32_t maxMatchCount);
     void getImageForFindMatch(uint32_t matchIndex);
     void selectFindMatch(uint32_t matchIndex);
+    void indicateFindMatch(uint32_t matchIndex);
     void hideFindUI();
     void countStringMatches(const String&, uint32_t findOptions, uint32_t maxMatchCount);
     void replaceMatches(const Vector<uint32_t>& matchIndices, const String& replacementText, bool selectionOnly, CallbackID);
index 8800209..c11b49d 100644 (file)
@@ -284,6 +284,7 @@ GenerateSyntheticEditingCommand(enum:uint8_t WebKit::SyntheticEditingCommandType
     FindStringMatches(String string, uint32_t findOptions, unsigned maxMatchCount)
     GetImageForFindMatch(uint32_t matchIndex)
     SelectFindMatch(uint32_t matchIndex)
+    IndicateFindMatch(uint32_t matchIndex)
     HideFindUI()
     CountStringMatches(String string, uint32_t findOptions, unsigned maxMatchCount)
     ReplaceMatches(Vector<uint32_t> matchIndices, String replacementText, bool selectionOnly, WebKit::CallbackID callbackID)
index 4260f7b..0e3b992 100644 (file)
@@ -1,3 +1,21 @@
+2019-07-16  Tim Horton  <timothy_horton@apple.com>
+
+        NSTextFinder holes don't scroll with the page
+        https://bugs.webkit.org/show_bug.cgi?id=199815
+        <rdar://problem/52280514>
+
+        Reviewed by Simon Fraser.
+
+        * MiniBrowser/mac/WK2BrowserWindowController.m:
+        (-[MiniBrowserNSTextFinder performAction:]):
+        (-[WK2BrowserWindowController awakeFromNib]):
+        Adopt the WebKit UI in MiniBrowser. Also, override NSTextFinder's
+        -performAction: so that we can intercept the close action and
+        hide our own find UI, since there is no delegate for that action yet.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/FindInPage.mm:
+        (TEST):
+
 2019-07-15  Keith Miller  <keith_miller@apple.com>
 
         JSGlobalObject type macros should support feature flags and WeakRef should have one
index 4943344..69f10c0 100644 (file)
@@ -47,6 +47,24 @@ static void* keyValueObservingContext = &keyValueObservingContext;
 static const int testHeaderBannerHeight = 42;
 static const int testFooterBannerHeight = 58;
 
+@interface MiniBrowserNSTextFinder : NSTextFinder
+
+@property (nonatomic, copy) dispatch_block_t hideInterfaceCallback;
+
+@end
+
+@implementation MiniBrowserNSTextFinder
+
+- (void)performAction:(NSTextFinderAction)op
+{
+    [super performAction:op];
+
+    if (op == NSTextFinderActionHideFindInterface && _hideInterfaceCallback)
+        _hideInterfaceCallback();
+}
+
+@end
+
 @interface WK2BrowserWindowController () <NSTextFinderBarContainer, WKNavigationDelegate, WKUIDelegate, _WKIconLoadingDelegate>
 @end
 
@@ -58,7 +76,7 @@ static const int testFooterBannerHeight = 58;
 
     BOOL _useShrinkToFit;
 
-    NSTextFinder *_textFinder;
+    MiniBrowserNSTextFinder *_textFinder;
     NSView *_textFindBarView;
     BOOL _findBarVisible;
 }
@@ -97,11 +115,16 @@ static const int testFooterBannerHeight = 58;
 
     _zoomTextOnly = NO;
 
-    _textFinder = [[NSTextFinder alloc] init];
+    _webView._usePlatformFindUI = NO;
+
+    _textFinder = [[MiniBrowserNSTextFinder alloc] init];
     _textFinder.incrementalSearchingEnabled = YES;
-    _textFinder.incrementalSearchingShouldDimContentView = YES;
+    _textFinder.incrementalSearchingShouldDimContentView = NO;
     _textFinder.client = _webView;
     _textFinder.findBarContainer = self;
+    _textFinder.hideInterfaceCallback = ^{
+        [_webView _hideFindUI];
+    };
 }
 
 - (instancetype)initWithConfiguration:(WKWebViewConfiguration *)configuration
index 42d076c..41eb163 100644 (file)
@@ -142,6 +142,37 @@ TEST(WebKit, FindInPage)
     EXPECT_EQ((NSUInteger)1000, [result.matches count]);
 }
 
+TEST(WebKit, FindInPageWithPlatformPresentation)
+{
+    // This should be the same as above, but does not generate rects or images, so that AppKit won't paint its find UI.
+
+    RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 200, 200)]);
+    [webView _setOverrideDeviceScaleFactor:2];
+    [webView _setUsePlatformFindUI:NO];
+
+    NSURLRequest *request = [NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"lots-of-text" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]];
+    [webView loadRequest:request];
+    [webView _test_waitForDidFinishNavigation];
+
+    // Find all matches, but recieve no rects.
+    auto result = findMatches(webView.get(), @"Birthday");
+    EXPECT_EQ((NSUInteger)360, [result.matches count]);
+    RetainPtr<FindMatch> match = [result.matches objectAtIndex:0];
+    EXPECT_EQ((NSUInteger)0, [match textRects].count);
+
+    // Ensure that no image is generated.
+    __block bool generateTextImageDone = false;
+    [match generateTextImage:^(NSImage *image) {
+        EXPECT_EQ(image, nullptr);
+        generateTextImageDone = true;
+    }];
+    TestWebKitAPI::Util::run(&generateTextImageDone);
+
+    // Ensure that we cap the number of matches. There are actually 1600, but we only get the first 1000.
+    result = findMatches(webView.get(), @" ");
+    EXPECT_EQ((NSUInteger)1000, [result.matches count]);
+}
+
 TEST(WebKit, FindInPageWrapping)
 {
     RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 100, 100)]);