Diff
Modified: trunk/Source/WebCore/ChangeLog (214892 => 214893)
--- trunk/Source/WebCore/ChangeLog 2017-04-04 19:43:29 UTC (rev 214892)
+++ trunk/Source/WebCore/ChangeLog 2017-04-04 19:59:52 UTC (rev 214893)
@@ -1,3 +1,38 @@
+2017-04-04 Tim Horton <[email protected]>
+
+ [Mac] -[WKWebView findMatchesForString:relativeToMatch:findOptions:maxResults:resultCollector:] invokes the resultCollector with didWrap = NO even when it wraps
+ https://bugs.webkit.org/show_bug.cgi?id=165801
+ <rdar://problem/29649535>
+
+ Reviewed by Wenson Hsieh.
+
+ New API tests: WebKit2.FindInPageWrapping*
+
+ * page/FrameTree.cpp:
+ (WebCore::FrameTree::traverseNextWithWrap):
+ (WebCore::FrameTree::traversePreviousWithWrap):
+ (WebCore::FrameTree::traverseNextInPostOrderWithWrap):
+ * page/FrameTree.h:
+ Add CanWrap and DidWrap boolean enums, and add an optional out argument
+ to traverse*WithWrap indicating whether a wrap actually occurred.
+
+ * history/CachedPage.cpp:
+ (WebCore::firePageShowAndPopStateEvents):
+ * history/PageCache.cpp:
+ (WebCore::destroyRenderTree):
+ Adjust to the new CanWrap enum.
+
+ * page/Page.cpp:
+ (WebCore::incrementFrame):
+ (WebCore::Page::findString):
+ (WebCore::Page::findStringMatchingRanges):
+ (WebCore::Page::rangeOfString):
+ (WebCore::Page::findMatchesForText):
+ (WebCore::Page::unmarkAllTextMatches):
+ * page/Page.h:
+ Adjust to the new CanWrap enum, and optionally plumb DidWrap through
+ to callers of findString().
+
2017-04-04 Carlos Garcia Campos <[email protected]>
[GTK] PLATFORM(GTK) && !USE(COORDINATED_GRAPHICS_THREADED) is no longer possible
Modified: trunk/Source/WebCore/history/CachedPage.cpp (214892 => 214893)
--- trunk/Source/WebCore/history/CachedPage.cpp 2017-04-04 19:43:29 UTC (rev 214892)
+++ trunk/Source/WebCore/history/CachedPage.cpp 2017-04-04 19:59:52 UTC (rev 214893)
@@ -78,7 +78,7 @@
// Dispatching _javascript_ events can cause frame destruction.
auto& mainFrame = page.mainFrame();
Vector<Ref<Frame>> childFrames;
- for (auto* child = mainFrame.tree().traverseNextInPostOrderWithWrap(true); child; child = child->tree().traverseNextInPostOrderWithWrap(false))
+ for (auto* child = mainFrame.tree().traverseNextInPostOrder(CanWrap::Yes); child; child = child->tree().traverseNextInPostOrder(CanWrap::No))
childFrames.append(*child);
for (auto& child : childFrames) {
Modified: trunk/Source/WebCore/history/PageCache.cpp (214892 => 214893)
--- trunk/Source/WebCore/history/PageCache.cpp 2017-04-04 19:43:29 UTC (rev 214892)
+++ trunk/Source/WebCore/history/PageCache.cpp 2017-04-04 19:59:52 UTC (rev 214893)
@@ -374,7 +374,7 @@
// Note that destruction happens bottom-up so that the main frame's tree dies last.
static void destroyRenderTree(MainFrame& mainFrame)
{
- for (Frame* frame = mainFrame.tree().traversePreviousWithWrap(true); frame; frame = frame->tree().traversePreviousWithWrap(false)) {
+ for (Frame* frame = mainFrame.tree().traversePrevious(CanWrap::Yes); frame; frame = frame->tree().traversePrevious(CanWrap::No)) {
if (!frame->document())
continue;
auto& document = *frame->document();
Modified: trunk/Source/WebCore/page/FrameTree.cpp (214892 => 214893)
--- trunk/Source/WebCore/page/FrameTree.cpp 2017-04-04 19:43:29 UTC (rev 214892)
+++ trunk/Source/WebCore/page/FrameTree.cpp 2017-04-04 19:59:52 UTC (rev 214893)
@@ -403,18 +403,21 @@
return nullptr;
}
-Frame* FrameTree::traverseNextWithWrap(bool wrap) const
+Frame* FrameTree::traverseNext(CanWrap canWrap, DidWrap* didWrap) const
{
if (Frame* result = traverseNext())
return result;
- if (wrap)
+ if (canWrap == CanWrap::Yes) {
+ if (didWrap)
+ *didWrap = DidWrap::Yes;
return &m_thisFrame.mainFrame();
+ }
return nullptr;
}
-Frame* FrameTree::traversePreviousWithWrap(bool wrap) const
+Frame* FrameTree::traversePrevious(CanWrap canWrap, DidWrap* didWrap) const
{
// FIXME: besides the wrap feature, this is just the traversePreviousNode algorithm
@@ -424,20 +427,23 @@
return parentFrame;
// no siblings, no parent, self==top
- if (wrap)
+ if (canWrap == CanWrap::Yes) {
+ if (didWrap)
+ *didWrap = DidWrap::Yes;
return deepLastChild();
+ }
// top view is always the last one in this ordering, so prev is nil without wrap
return nullptr;
}
-Frame* FrameTree::traverseNextInPostOrderWithWrap(bool wrap) const
+Frame* FrameTree::traverseNextInPostOrder(CanWrap canWrap) const
{
if (m_nextSibling)
return m_nextSibling->tree().deepFirstChild();
if (m_parent)
return m_parent;
- if (wrap)
+ if (canWrap == CanWrap::Yes)
return deepFirstChild();
return nullptr;
}
Modified: trunk/Source/WebCore/page/FrameTree.h (214892 => 214893)
--- trunk/Source/WebCore/page/FrameTree.h 2017-04-04 19:43:29 UTC (rev 214892)
+++ trunk/Source/WebCore/page/FrameTree.h 2017-04-04 19:59:52 UTC (rev 214893)
@@ -23,6 +23,9 @@
namespace WebCore {
+enum class CanWrap : bool { No, Yes };
+enum class DidWrap : bool { No, Yes };
+
class Frame;
class TreeScope;
@@ -62,10 +65,10 @@
WEBCORE_EXPORT Frame* traverseNext(const Frame* stayWithin = nullptr) const;
// Rendered means being the main frame or having an ownerRenderer. It may not have been parented in the Widget tree yet (see WidgetHierarchyUpdatesSuspensionScope).
WEBCORE_EXPORT Frame* traverseNextRendered(const Frame* stayWithin = nullptr) const;
- WEBCORE_EXPORT Frame* traverseNextWithWrap(bool) const;
- WEBCORE_EXPORT Frame* traversePreviousWithWrap(bool) const;
+ WEBCORE_EXPORT Frame* traverseNext(CanWrap, DidWrap* = nullptr) const;
+ WEBCORE_EXPORT Frame* traversePrevious(CanWrap, DidWrap* = nullptr) const;
- Frame* traverseNextInPostOrderWithWrap(bool) const;
+ Frame* traverseNextInPostOrder(CanWrap) const;
WEBCORE_EXPORT void appendChild(Frame&);
void detachFromParent() { m_parent = nullptr; }
Modified: trunk/Source/WebCore/page/Page.cpp (214892 => 214893)
--- trunk/Source/WebCore/page/Page.cpp 2017-04-04 19:43:29 UTC (rev 214892)
+++ trunk/Source/WebCore/page/Page.cpp 2017-04-04 19:59:52 UTC (rev 214893)
@@ -647,19 +647,19 @@
}
}
-static Frame* incrementFrame(Frame* curr, bool forward, bool wrapFlag)
+static Frame* incrementFrame(Frame* curr, bool forward, CanWrap canWrap, DidWrap* didWrap = nullptr)
{
return forward
- ? curr->tree().traverseNextWithWrap(wrapFlag)
- : curr->tree().traversePreviousWithWrap(wrapFlag);
+ ? curr->tree().traverseNext(canWrap, didWrap)
+ : curr->tree().traversePrevious(canWrap, didWrap);
}
-bool Page::findString(const String& target, FindOptions options)
+bool Page::findString(const String& target, FindOptions options, DidWrap* didWrap)
{
if (target.isEmpty())
return false;
- bool shouldWrap = options & WrapAround;
+ CanWrap canWrap = options & WrapAround ? CanWrap::Yes : CanWrap::No;
Frame* frame = &focusController().focusedOrMainFrame();
Frame* startFrame = frame;
do {
@@ -669,12 +669,14 @@
focusController().setFocusedFrame(frame);
return true;
}
- frame = incrementFrame(frame, !(options & Backwards), shouldWrap);
+ frame = incrementFrame(frame, !(options & Backwards), canWrap, didWrap);
} while (frame && frame != startFrame);
// Search contents of startFrame, on the other side of the selection that we did earlier.
// We cheat a bit and just research with wrap on
- if (shouldWrap && !startFrame->selection().isNone()) {
+ if (canWrap == CanWrap::Yes && !startFrame->selection().isNone()) {
+ if (didWrap)
+ *didWrap = DidWrap::Yes;
bool found = startFrame->editor().findString(target, options | WrapAround | StartInSelection);
focusController().setFocusedFrame(frame);
return found;
@@ -693,7 +695,7 @@
frame->editor().countMatchesForText(target, 0, options, limit ? (limit - matchRanges.size()) : 0, true, &matchRanges);
if (frame->selection().isRange())
frameWithSelection = frame;
- frame = incrementFrame(frame, true, false);
+ frame = incrementFrame(frame, true, CanWrap::No);
} while (frame);
if (matchRanges.isEmpty())
@@ -735,7 +737,7 @@
if (referenceRange && referenceRange->ownerDocument().page() != this)
return nullptr;
- bool shouldWrap = options & WrapAround;
+ CanWrap canWrap = options & WrapAround ? CanWrap::Yes : CanWrap::No;
Frame* frame = referenceRange ? referenceRange->ownerDocument().frame() : &mainFrame();
Frame* startFrame = frame;
do {
@@ -742,12 +744,12 @@
if (RefPtr<Range> resultRange = frame->editor().rangeOfString(target, frame == startFrame ? referenceRange : 0, options & ~WrapAround))
return resultRange;
- frame = incrementFrame(frame, !(options & Backwards), shouldWrap);
+ frame = incrementFrame(frame, !(options & Backwards), canWrap);
} while (frame && frame != startFrame);
// Search contents of startFrame, on the other side of the reference range that we did earlier.
// We cheat a bit and just search again with wrap on.
- if (shouldWrap && referenceRange) {
+ if (canWrap == CanWrap::Yes && referenceRange) {
if (RefPtr<Range> resultRange = startFrame->editor().rangeOfString(target, referenceRange, options | WrapAround | StartInSelection))
return resultRange;
}
@@ -767,7 +769,7 @@
if (shouldMarkMatches == MarkMatches)
frame->editor().setMarkedTextMatchesAreHighlighted(shouldHighlightMatches == HighlightMatches);
matchCount += frame->editor().countMatchesForText(target, 0, options, maxMatchCount ? (maxMatchCount - matchCount) : 0, shouldMarkMatches == MarkMatches, 0);
- frame = incrementFrame(frame, true, false);
+ frame = incrementFrame(frame, true, CanWrap::No);
} while (frame);
return matchCount;
@@ -788,7 +790,7 @@
Frame* frame = &mainFrame();
do {
frame->document()->markers().removeMarkers(DocumentMarker::TextMatch);
- frame = incrementFrame(frame, true, false);
+ frame = incrementFrame(frame, true, CanWrap::No);
} while (frame);
}
Modified: trunk/Source/WebCore/page/Page.h (214892 => 214893)
--- trunk/Source/WebCore/page/Page.h 2017-04-04 19:43:29 UTC (rev 214892)
+++ trunk/Source/WebCore/page/Page.h 2017-04-04 19:59:52 UTC (rev 214893)
@@ -145,6 +145,9 @@
Unresponsive
};
+enum class CanWrap : bool;
+enum class DidWrap : bool;
+
class Page : public Supplementable<Page> {
WTF_MAKE_NONCOPYABLE(Page);
WTF_MAKE_FAST_ALLOCATED;
@@ -264,7 +267,7 @@
void setTabKeyCyclesThroughElements(bool b) { m_tabKeyCyclesThroughElements = b; }
bool tabKeyCyclesThroughElements() const { return m_tabKeyCyclesThroughElements; }
- WEBCORE_EXPORT bool findString(const String&, FindOptions);
+ WEBCORE_EXPORT bool findString(const String&, FindOptions, DidWrap* = nullptr);
WEBCORE_EXPORT RefPtr<Range> rangeOfString(const String&, Range*, FindOptions);
Modified: trunk/Source/WebKit/mac/ChangeLog (214892 => 214893)
--- trunk/Source/WebKit/mac/ChangeLog 2017-04-04 19:43:29 UTC (rev 214892)
+++ trunk/Source/WebKit/mac/ChangeLog 2017-04-04 19:59:52 UTC (rev 214893)
@@ -1,3 +1,15 @@
+2017-04-04 Tim Horton <[email protected]>
+
+ [Mac] -[WKWebView findMatchesForString:relativeToMatch:findOptions:maxResults:resultCollector:] invokes the resultCollector with didWrap = NO even when it wraps
+ https://bugs.webkit.org/show_bug.cgi?id=165801
+ <rdar://problem/29649535>
+
+ Reviewed by Wenson Hsieh.
+
+ * WebView/WebView.mm:
+ (incrementFrame):
+ Adjust to the new CanWrap enum.
+
2017-03-27 Joseph Pecoraro <[email protected]>
Web Inspector: Saving files should not suggest the top level directory
Modified: trunk/Source/WebKit/mac/WebView/WebView.mm (214892 => 214893)
--- trunk/Source/WebKit/mac/WebView/WebView.mm 2017-04-04 19:43:29 UTC (rev 214892)
+++ trunk/Source/WebKit/mac/WebView/WebView.mm 2017-04-04 19:59:52 UTC (rev 214893)
@@ -6796,9 +6796,10 @@
static WebFrame *incrementFrame(WebFrame *frame, WebFindOptions options = 0)
{
Frame* coreFrame = core(frame);
+ CanWrap canWrap = options & WebFindOptionsWrapAround ? CanWrap::Yes : CanWrap::No;
return kit((options & WebFindOptionsBackwards)
- ? coreFrame->tree().traversePreviousWithWrap(options & WebFindOptionsWrapAround)
- : coreFrame->tree().traverseNextWithWrap(options & WebFindOptionsWrapAround));
+ ? coreFrame->tree().traversePrevious(canWrap)
+ : coreFrame->tree().traverseNext(canWrap));
}
- (BOOL)searchFor:(NSString *)string direction:(BOOL)forward caseSensitive:(BOOL)caseFlag wrap:(BOOL)wrapFlag
Modified: trunk/Source/WebKit/win/WebView.cpp (214892 => 214893)
--- trunk/Source/WebKit/win/WebView.cpp 2017-04-04 19:43:29 UTC (rev 214892)
+++ trunk/Source/WebKit/win/WebView.cpp 2017-04-04 19:59:52 UTC (rev 214893)
@@ -3856,9 +3856,10 @@
static Frame *incrementFrame(Frame *curr, bool forward, bool wrapFlag)
{
+ CanWrap canWrap = wrapFlag ? CanWrap::Yes : CanWrap::No;
return forward
- ? curr->tree().traverseNextWithWrap(wrapFlag)
- : curr->tree().traversePreviousWithWrap(wrapFlag);
+ ? curr->tree().traverseNext(canWrap)
+ : curr->tree().traversePrevious(canWrap);
}
HRESULT WebView::searchFor(_In_ BSTR str, BOOL forward, BOOL caseFlag, BOOL wrapFlag, _Out_ BOOL* found)
Modified: trunk/Source/WebKit2/ChangeLog (214892 => 214893)
--- trunk/Source/WebKit2/ChangeLog 2017-04-04 19:43:29 UTC (rev 214892)
+++ trunk/Source/WebKit2/ChangeLog 2017-04-04 19:59:52 UTC (rev 214893)
@@ -1,3 +1,40 @@
+2017-04-04 Tim Horton <[email protected]>
+
+ [Mac] -[WKWebView findMatchesForString:relativeToMatch:findOptions:maxResults:resultCollector:] invokes the resultCollector with didWrap = NO even when it wraps
+ https://bugs.webkit.org/show_bug.cgi?id=165801
+ <rdar://problem/29649535>
+
+ Reviewed by Wenson Hsieh.
+
+ Previously, when doing an incremental find that wrapped, we would
+ say that it did not, leading NSTextFinder to not provide its usual
+ wrapping UI, and other clients of the NSTextFinderClient protocol to
+ get confused by the lack of wrapping.
+
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::didFindString):
+ * UIProcess/WebPageProxy.h:
+ * UIProcess/WebPageProxy.messages.in:
+ * UIProcess/API/APIFindClient.h:
+ (API::FindClient::didFindString):
+ * UIProcess/API/C/WKPage.cpp:
+ (WKPageSetPageFindClient):
+ * UIProcess/Cocoa/FindClient.h:
+ * UIProcess/Cocoa/FindClient.mm:
+ (WebKit::FindClient::didFindString):
+ * WebProcess/WebPage/FindController.cpp:
+ (WebKit::FindController::updateFindUIAfterPageScroll):
+ (WebKit::FindController::findString):
+ * WebProcess/WebPage/FindController.h:
+ Plumb DidWrap from FindController's call to findString back through
+ the DidFindString message.
+
+ * UIProcess/mac/WKTextFinderClient.mm:
+ (-[WKTextFinderClient didFindStringMatchesWithRects:didWrapAround:]):
+ (-[WKTextFinderClient didFindStringMatchesWithRects:]): Deleted.
+ Make use of the new DidWrap information to stop lying to NSTextFinder
+ about whether a wrap actually occurred.
+
2017-04-03 Wenson Hsieh <[email protected]>
Data interaction should register type identifiers in order of priority
Modified: trunk/Source/WebKit2/UIProcess/API/APIFindClient.h (214892 => 214893)
--- trunk/Source/WebKit2/UIProcess/API/APIFindClient.h 2017-04-04 19:43:29 UTC (rev 214892)
+++ trunk/Source/WebKit2/UIProcess/API/APIFindClient.h 2017-04-04 19:59:52 UTC (rev 214893)
@@ -43,7 +43,7 @@
virtual ~FindClient() { }
virtual void didCountStringMatches(WebKit::WebPageProxy*, const WTF::String&, uint32_t) { }
- virtual void didFindString(WebKit::WebPageProxy*, const WTF::String&, const Vector<WebCore::IntRect>& matchRects, uint32_t, int32_t) { }
+ virtual void didFindString(WebKit::WebPageProxy*, const WTF::String&, const Vector<WebCore::IntRect>& matchRects, uint32_t, int32_t, bool didWrapAround) { }
virtual void didFailToFindString(WebKit::WebPageProxy*, const WTF::String&) { }
};
Modified: trunk/Source/WebKit2/UIProcess/API/C/WKPage.cpp (214892 => 214893)
--- trunk/Source/WebKit2/UIProcess/API/C/WKPage.cpp 2017-04-04 19:43:29 UTC (rev 214892)
+++ trunk/Source/WebKit2/UIProcess/API/C/WKPage.cpp 2017-04-04 19:59:52 UTC (rev 214893)
@@ -931,7 +931,7 @@
}
private:
- void didFindString(WebPageProxy* page, const String& string, const Vector<WebCore::IntRect>&, uint32_t matchCount, int32_t) override
+ void didFindString(WebPageProxy* page, const String& string, const Vector<WebCore::IntRect>&, uint32_t matchCount, int32_t, bool didWrapAround) override
{
if (!m_client.didFindString)
return;
Modified: trunk/Source/WebKit2/UIProcess/Cocoa/FindClient.h (214892 => 214893)
--- trunk/Source/WebKit2/UIProcess/Cocoa/FindClient.h 2017-04-04 19:43:29 UTC (rev 214892)
+++ trunk/Source/WebKit2/UIProcess/Cocoa/FindClient.h 2017-04-04 19:59:52 UTC (rev 214893)
@@ -48,7 +48,7 @@
private:
// From API::FindClient
virtual void didCountStringMatches(WebPageProxy*, const String&, uint32_t matchCount);
- virtual void didFindString(WebPageProxy*, const String&, const Vector<WebCore::IntRect>&, uint32_t matchCount, int32_t matchIndex);
+ virtual void didFindString(WebPageProxy*, const String&, const Vector<WebCore::IntRect>&, uint32_t matchCount, int32_t matchIndex, bool didWrapAround);
virtual void didFailToFindString(WebPageProxy*, const String&);
WKWebView *m_webView;
Modified: trunk/Source/WebKit2/UIProcess/Cocoa/FindClient.mm (214892 => 214893)
--- trunk/Source/WebKit2/UIProcess/Cocoa/FindClient.mm 2017-04-04 19:43:29 UTC (rev 214892)
+++ trunk/Source/WebKit2/UIProcess/Cocoa/FindClient.mm 2017-04-04 19:59:52 UTC (rev 214893)
@@ -57,7 +57,7 @@
[m_delegate.get() _webView:m_webView didCountMatches:matchCount forString:string];
}
-void FindClient::didFindString(WebPageProxy*, const String& string, const Vector<WebCore::IntRect>&, uint32_t matchCount, int32_t matchIndex)
+void FindClient::didFindString(WebPageProxy*, const String& string, const Vector<WebCore::IntRect>&, uint32_t matchCount, int32_t matchIndex, bool)
{
if (m_delegateMethods.webviewDidFindString)
[m_delegate.get() _webView:m_webView didFindMatches:matchCount forString:string withMatchIndex:matchIndex];
Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp (214892 => 214893)
--- trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp 2017-04-04 19:43:29 UTC (rev 214892)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp 2017-04-04 19:59:52 UTC (rev 214893)
@@ -4466,9 +4466,9 @@
#endif
}
-void WebPageProxy::didFindString(const String& string, const Vector<WebCore::IntRect>& matchRects, uint32_t matchCount, int32_t matchIndex)
+void WebPageProxy::didFindString(const String& string, const Vector<WebCore::IntRect>& matchRects, uint32_t matchCount, int32_t matchIndex, bool didWrapAround)
{
- m_findClient->didFindString(this, string, matchRects, matchCount, matchIndex);
+ m_findClient->didFindString(this, string, matchRects, matchCount, matchIndex, didWrapAround);
}
void WebPageProxy::didFindStringMatches(const String& string, const Vector<Vector<WebCore::IntRect>>& matchRects, int32_t firstIndexAfterSelection)
Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.h (214892 => 214893)
--- trunk/Source/WebKit2/UIProcess/WebPageProxy.h 2017-04-04 19:43:29 UTC (rev 214892)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.h 2017-04-04 19:59:52 UTC (rev 214893)
@@ -788,7 +788,7 @@
void setTextIndicator(const WebCore::TextIndicatorData&, uint64_t /* WebCore::TextIndicatorWindowLifetime */ lifetime = 0 /* Permanent */);
void setTextIndicatorAnimationProgress(float);
void clearTextIndicator();
- void didFindString(const String&, const Vector<WebCore::IntRect>&, uint32_t matchCount, int32_t matchIndex);
+ void didFindString(const String&, const Vector<WebCore::IntRect>&, uint32_t matchCount, int32_t matchIndex, bool didWrapAround);
void didFailToFindString(const String&);
void didFindStringMatches(const String&, const Vector<Vector<WebCore::IntRect>>& matchRects, int32_t firstIndexAfterSelection);
Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.messages.in (214892 => 214893)
--- trunk/Source/WebKit2/UIProcess/WebPageProxy.messages.in 2017-04-04 19:43:29 UTC (rev 214892)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.messages.in 2017-04-04 19:59:52 UTC (rev 214893)
@@ -241,7 +241,7 @@
DidCountStringMatches(String string, uint32_t matchCount)
SetTextIndicator(struct WebCore::TextIndicatorData indicator, uint64_t lifetime)
ClearTextIndicator()
- DidFindString(String string, Vector<WebCore::IntRect> matchRect, uint32_t matchCount, int32_t matchIndex)
+ DidFindString(String string, Vector<WebCore::IntRect> matchRect, uint32_t matchCount, int32_t matchIndex, bool didWrapAround)
DidFailToFindString(String string)
DidFindStringMatches(String string, Vector<Vector<WebCore::IntRect>> matches, int32_t firstIndexAfterSelection)
DidGetImageForFindMatch(WebKit::ShareableBitmap::Handle contentImageHandle, uint32_t matchIndex)
Modified: trunk/Source/WebKit2/UIProcess/ios/WKPDFView.mm (214892 => 214893)
--- trunk/Source/WebKit2/UIProcess/ios/WKPDFView.mm 2017-04-04 19:43:29 UTC (rev 214892)
+++ trunk/Source/WebKit2/UIProcess/ios/WKPDFView.mm 2017-04-04 19:59:52 UTC (rev 214893)
@@ -614,7 +614,7 @@
_currentFindMatchIndex = 0;
for (const auto& knownMatch : _cachedFindMatches) {
if (match.stringRange.location == [knownMatch stringRange].location && match.stringRange.length == [knownMatch stringRange].length) {
- page->findClient().didFindString(page.get(), string, { }, _cachedFindMatches.size(), _currentFindMatchIndex);
+ page->findClient().didFindString(page.get(), string, { }, _cachedFindMatches.size(), _currentFindMatchIndex, false);
break;
}
_currentFindMatchIndex++;
Modified: trunk/Source/WebKit2/UIProcess/mac/WKTextFinderClient.mm (214892 => 214893)
--- trunk/Source/WebKit2/UIProcess/mac/WKTextFinderClient.mm 2017-04-04 19:43:29 UTC (rev 214892)
+++ trunk/Source/WebKit2/UIProcess/mac/WKTextFinderClient.mm 2017-04-04 19:59:52 UTC (rev 214893)
@@ -47,7 +47,7 @@
@interface WKTextFinderClient ()
-- (void)didFindStringMatchesWithRects:(const Vector<Vector<IntRect>>&)rects;
+- (void)didFindStringMatchesWithRects:(const Vector<Vector<IntRect>>&)rects didWrapAround:(BOOL)didWrapAround;
- (void)getImageForMatchResult:(id <NSTextFinderAsynchronousDocumentFindMatch>)findMatch completionHandler:(void (^)(NSImage *generatedImage))completionHandler;
- (void)didGetImageForMatchResult:(WebImage *)string;
@@ -66,7 +66,7 @@
private:
void didFindStringMatches(WebPageProxy* page, const String&, const Vector<Vector<IntRect>>& matchRects, int32_t) override
{
- [m_textFinderClient didFindStringMatchesWithRects:matchRects];
+ [m_textFinderClient didFindStringMatchesWithRects:matchRects didWrapAround:NO];
}
void didGetImageForMatchResult(WebPageProxy* page, WebImage* image, int32_t index) override
@@ -74,14 +74,14 @@
[m_textFinderClient didGetImageForMatchResult:image];
}
- void didFindString(WebPageProxy*, const String&, const Vector<IntRect>& matchRects, uint32_t, int32_t) override
+ void didFindString(WebPageProxy*, const String&, const Vector<IntRect>& matchRects, uint32_t, int32_t, bool didWrapAround) override
{
- [m_textFinderClient didFindStringMatchesWithRects:{ matchRects }];
+ [m_textFinderClient didFindStringMatchesWithRects:{ matchRects } didWrapAround:didWrapAround];
}
void didFailToFindString(WebPageProxy*, const String& string) override
{
- [m_textFinderClient didFindStringMatchesWithRects:{ }];
+ [m_textFinderClient didFindStringMatchesWithRects:{ } didWrapAround:NO];
}
RetainPtr<WKTextFinderClient> m_textFinderClient;
@@ -234,7 +234,7 @@
return nsMatchRects;
}
-- (void)didFindStringMatchesWithRects:(const Vector<Vector<IntRect>>&)rectsForMatches
+- (void)didFindStringMatchesWithRects:(const Vector<Vector<IntRect>>&)rectsForMatches didWrapAround:(BOOL)didWrapAround
{
if (_findReplyCallbacks.isEmpty())
return;
@@ -248,7 +248,7 @@
[matchObjects addObject:match.get()];
}
- replyCallback(matchObjects.get(), NO);
+ replyCallback(matchObjects.get(), didWrapAround);
}
- (void)didGetImageForMatchResult:(WebImage *)image
@@ -277,6 +277,9 @@
Block_release(copiedImageCallback);
});
+ // FIXME: There is no guarantee that this will ever result in didGetImageForMatchResult
+ // being called (and thus us calling our completion handler); we should harden this
+ // against all of the early returns in FindController::getImageForFindMatch.
_page->getImageForFindMatch(textFinderMatch.index);
}
Modified: trunk/Source/WebKit2/WebProcess/WebPage/FindController.cpp (214892 => 214893)
--- trunk/Source/WebKit2/WebProcess/WebPage/FindController.cpp 2017-04-04 19:43:29 UTC (rev 214892)
+++ trunk/Source/WebKit2/WebProcess/WebPage/FindController.cpp 2017-04-04 19:59:52 UTC (rev 214893)
@@ -114,7 +114,7 @@
return 0;
}
-void FindController::updateFindUIAfterPageScroll(bool found, const String& string, FindOptions options, unsigned maxMatchCount)
+void FindController::updateFindUIAfterPageScroll(bool found, const String& string, FindOptions options, unsigned maxMatchCount, DidWrap didWrap)
{
Frame* selectedFrame = frameWithSelection(m_webPage->corePage());
@@ -181,7 +181,7 @@
m_findMatches.append(range);
}
- m_webPage->send(Messages::WebPageProxy::DidFindString(string, matchRects, matchCount, m_foundStringMatchIndex));
+ m_webPage->send(Messages::WebPageProxy::DidFindString(string, matchRects, matchCount, m_foundStringMatchIndex, didWrap == DidWrap::Yes));
if (!(options & FindOptionsShowFindIndicator) || !selectedFrame || !updateFindIndicator(*selectedFrame, shouldShowOverlay))
hideFindIndicator();
@@ -232,10 +232,11 @@
m_findMatches.clear();
bool found;
+ DidWrap didWrap = DidWrap::No;
if (pluginView)
found = pluginView->findString(string, coreOptions, maxMatchCount);
else
- found = m_webPage->corePage()->findString(string, coreOptions);
+ found = m_webPage->corePage()->findString(string, coreOptions, &didWrap);
if (found) {
didFindString();
@@ -249,8 +250,8 @@
}
RefPtr<WebPage> protectedWebPage = m_webPage;
- m_webPage->drawingArea()->dispatchAfterEnsuringUpdatedScrollPosition([protectedWebPage, found, string, options, maxMatchCount] () {
- protectedWebPage->findController().updateFindUIAfterPageScroll(found, string, options, maxMatchCount);
+ m_webPage->drawingArea()->dispatchAfterEnsuringUpdatedScrollPosition([protectedWebPage, found, string, options, maxMatchCount, didWrap] () {
+ protectedWebPage->findController().updateFindUIAfterPageScroll(found, string, options, maxMatchCount, didWrap);
});
}
Modified: trunk/Source/WebKit2/WebProcess/WebPage/FindController.h (214892 => 214893)
--- trunk/Source/WebKit2/WebProcess/WebPage/FindController.h 2017-04-04 19:43:29 UTC (rev 214892)
+++ trunk/Source/WebKit2/WebProcess/WebPage/FindController.h 2017-04-04 19:59:52 UTC (rev 214893)
@@ -41,6 +41,8 @@
namespace WebCore {
class Frame;
class Range;
+
+enum class DidWrap : bool;
}
namespace WebKit {
@@ -81,7 +83,7 @@
Vector<WebCore::IntRect> rectsForTextMatchesInRect(WebCore::IntRect clipRect);
bool updateFindIndicator(WebCore::Frame& selectedFrame, bool isShowingOverlay, bool shouldAnimate = true);
- void updateFindUIAfterPageScroll(bool found, const String&, FindOptions, unsigned maxMatchCount);
+ void updateFindUIAfterPageScroll(bool found, const String&, FindOptions, unsigned maxMatchCount, WebCore::DidWrap);
void willFindString();
void didFindString();
Modified: trunk/Tools/ChangeLog (214892 => 214893)
--- trunk/Tools/ChangeLog 2017-04-04 19:43:29 UTC (rev 214892)
+++ trunk/Tools/ChangeLog 2017-04-04 19:59:52 UTC (rev 214893)
@@ -1,3 +1,15 @@
+2017-04-04 Tim Horton <[email protected]>
+
+ [Mac] -[WKWebView findMatchesForString:relativeToMatch:findOptions:maxResults:resultCollector:] invokes the resultCollector with didWrap = NO even when it wraps
+ https://bugs.webkit.org/show_bug.cgi?id=165801
+ <rdar://problem/29649535>
+
+ Reviewed by Wenson Hsieh.
+
+ * TestWebKitAPI/Tests/WebKit2Cocoa/FindInPage.mm:
+ (TEST):
+ Add some tests for wrapping finds.
+
2017-04-03 Joseph Pecoraro <[email protected]>
Add some new patterns to filter-build-webkit
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/FindInPage.mm (214892 => 214893)
--- trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/FindInPage.mm 2017-04-04 19:43:29 UTC (rev 214892)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/FindInPage.mm 2017-04-04 19:59:52 UTC (rev 214893)
@@ -34,24 +34,51 @@
typedef enum : NSUInteger {
NSTextFinderAsynchronousDocumentFindOptionsBackwards = 1 << 0,
+ NSTextFinderAsynchronousDocumentFindOptionsWrap = 1 << 1,
} NSTextFinderAsynchronousDocumentFindOptions;
+NSTextFinderAsynchronousDocumentFindOptions noFindOptions = (NSTextFinderAsynchronousDocumentFindOptions)0;
+NSTextFinderAsynchronousDocumentFindOptions backwardsFindOptions =NSTextFinderAsynchronousDocumentFindOptionsBackwards;
+NSTextFinderAsynchronousDocumentFindOptions wrapFindOptions =NSTextFinderAsynchronousDocumentFindOptionsWrap;
+NSTextFinderAsynchronousDocumentFindOptions wrapBackwardsFindOptions = (NSTextFinderAsynchronousDocumentFindOptions)(NSTextFinderAsynchronousDocumentFindOptionsWrap | NSTextFinderAsynchronousDocumentFindOptionsBackwards);
+
@protocol NSTextFinderAsynchronousDocumentFindMatch <NSObject>
@property (retain, nonatomic, readonly) NSArray *textRects;
- (void)generateTextImage:(void (^)(NSImage *generatedImage))completionHandler;
@end
+typedef id <NSTextFinderAsynchronousDocumentFindMatch> FindMatch;
+
@interface WKWebView (NSTextFinderSupport)
-- (void)findMatchesForString:(NSString *)targetString relativeToMatch:(id <NSTextFinderAsynchronousDocumentFindMatch>)relativeMatch findOptions:(NSTextFinderAsynchronousDocumentFindOptions)findOptions maxResults:(NSUInteger)maxResults resultCollector:(void (^)(NSArray *matches, BOOL didWrap))resultCollector;
+- (void)findMatchesForString:(NSString *)targetString relativeToMatch:(FindMatch)relativeMatch findOptions:(NSTextFinderAsynchronousDocumentFindOptions)findOptions maxResults:(NSUInteger)maxResults resultCollector:(void (^)(NSArray *matches, BOOL didWrap))resultCollector;
@end
-static bool findMatchesDone;
+typedef struct {
+ RetainPtr<NSArray> matches;
+ BOOL didWrap;
+} FindResult;
+static FindResult findMatches(WKWebView *webView, NSString *findString, NSTextFinderAsynchronousDocumentFindOptions findOptions = noFindOptions, NSUInteger maxResults = NSUIntegerMax)
+{
+ __block FindResult result;
+ __block bool done = false;
+
+ [webView findMatchesForString:findString relativeToMatch:nil findOptions:findOptions maxResults:maxResults resultCollector:^(NSArray *matches, BOOL didWrap) {
+ result.matches = matches;
+ result.didWrap = didWrap;
+ done = true;
+ }];
+
+ TestWebKitAPI::Util::run(&done);
+
+ return result;
+}
+
TEST(WebKit2, FindInPage)
{
- RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 100, 100)]);
+ RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 200, 200)]);
[webView _setOverrideDeviceScaleFactor:2];
NSURLRequest *request = [NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"lots-of-text" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]];
@@ -58,80 +85,124 @@
[webView loadRequest:request];
[webView _test_waitForDidFinishNavigation];
- NSTextFinderAsynchronousDocumentFindOptions noFindOptions = (NSTextFinderAsynchronousDocumentFindOptions)0;
-
// Find all matches.
- [webView findMatchesForString:@"Birthday" relativeToMatch:nil findOptions:noFindOptions maxResults:NSUIntegerMax resultCollector:^(NSArray *matches, BOOL didWrap) {
- EXPECT_EQ((NSUInteger)360, matches.count);
+ auto result = findMatches(webView.get(), @"Birthday");
+ EXPECT_EQ((NSUInteger)360, [result.matches count]);
+ RetainPtr<FindMatch> match = [result.matches objectAtIndex:0];
+ EXPECT_EQ((NSUInteger)1, [match textRects].count);
- id <NSTextFinderAsynchronousDocumentFindMatch> firstMatch = [matches objectAtIndex:0];
- EXPECT_EQ((NSUInteger)1, firstMatch.textRects.count);
-
- findMatchesDone = true;
+ // Ensure that the generated image has the correct DPI.
+ __block bool generateTextImageDone = false;
+ [match generateTextImage:^(NSImage *image) {
+ CGImageRef CGImage = [image CGImageForProposedRect:nil context:nil hints:nil];
+ EXPECT_EQ(image.size.width, CGImageGetWidth(CGImage) / 2);
+ EXPECT_EQ(image.size.height, CGImageGetHeight(CGImage) / 2);
+ generateTextImageDone = true;
}];
+ TestWebKitAPI::Util::run(&generateTextImageDone);
- TestWebKitAPI::Util::run(&findMatchesDone);
- findMatchesDone = false;
+ // Find one match, doing an incremental search.
+ result = findMatches(webView.get(), @"Birthday", noFindOptions, 1);
+ EXPECT_EQ((NSUInteger)1, [result.matches count]);
+ RetainPtr<FindMatch> firstMatch = [result.matches firstObject];
+ EXPECT_EQ((NSUInteger)1, [firstMatch textRects].count);
+ // Find the next match in incremental mode.
+ result = findMatches(webView.get(), @"Birthday", noFindOptions, 1);
+ EXPECT_EQ((NSUInteger)1, [result.matches count]);
+ RetainPtr<FindMatch> secondMatch = [result.matches firstObject];
+ EXPECT_EQ((NSUInteger)1, [secondMatch textRects].count);
+ EXPECT_FALSE(NSEqualRects([[firstMatch textRects].lastObject rectValue], [[secondMatch textRects].lastObject rectValue]));
+
+ // Find the previous match in incremental mode.
+ result = findMatches(webView.get(), @"Birthday", backwardsFindOptions, 1);
+ EXPECT_EQ((NSUInteger)1, [result.matches count]);
+ RetainPtr<FindMatch> firstMatchAgain = [result.matches firstObject];
+ EXPECT_EQ((NSUInteger)1, [firstMatchAgain textRects].count);
+ EXPECT_TRUE(NSEqualRects([[firstMatch textRects].lastObject rectValue], [[firstMatchAgain textRects].lastObject rectValue]));
+
+ // 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(WebKit2, FindInPageWrapping)
+{
+ RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 100, 100)]);
+ [webView _setOverrideDeviceScaleFactor:2];
+
+ [webView loadHTMLString:@"word word" baseURL:nil];
+ [webView _test_waitForDidFinishNavigation];
+
// Find one match, doing an incremental search.
- [webView findMatchesForString:@"Birthday" relativeToMatch:nil findOptions:noFindOptions maxResults:1 resultCollector:^(NSArray *matches, BOOL didWrap) {
- EXPECT_EQ((NSUInteger)1, matches.count);
+ auto result = findMatches(webView.get(), @"word", wrapFindOptions, 1);
+ EXPECT_EQ((NSUInteger)1, [result.matches count]);
+ EXPECT_FALSE(result.didWrap);
- id <NSTextFinderAsynchronousDocumentFindMatch> firstMatch = [matches objectAtIndex:0];
- EXPECT_EQ((NSUInteger)1, firstMatch.textRects.count);
+ result = findMatches(webView.get(), @"word", wrapFindOptions, 1);
+ EXPECT_EQ((NSUInteger)1, [result.matches count]);
+ EXPECT_FALSE(result.didWrap);
- // Find the next match in incremental mode.
- [webView findMatchesForString:@"Birthday" relativeToMatch:nil findOptions:noFindOptions maxResults:1 resultCollector:^(NSArray *matches, BOOL didWrap) {
- EXPECT_EQ((NSUInteger)1, matches.count);
+ // The next match should wrap.
+ result = findMatches(webView.get(), @"word", wrapFindOptions, 1);
+ EXPECT_EQ((NSUInteger)1, [result.matches count]);
+ EXPECT_TRUE(result.didWrap);
- id <NSTextFinderAsynchronousDocumentFindMatch> secondMatch = [matches objectAtIndex:0];
- EXPECT_EQ((NSUInteger)1, secondMatch.textRects.count);
+ // Going backward after wrapping should wrap again.
+ result = findMatches(webView.get(), @"word", wrapBackwardsFindOptions, 1);
+ EXPECT_EQ((NSUInteger)1, [result.matches count]);
+ EXPECT_TRUE(result.didWrap);
+}
- EXPECT_FALSE(NSEqualRects([firstMatch.textRects.lastObject rectValue], [secondMatch.textRects.lastObject rectValue]));
+TEST(WebKit2, FindInPageWrappingDisabled)
+{
+ RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 100, 100)]);
+ [webView _setOverrideDeviceScaleFactor:2];
- // Find the previous match in incremental mode.
- [webView findMatchesForString:@"Birthday" relativeToMatch:nil findOptions:NSTextFinderAsynchronousDocumentFindOptionsBackwards maxResults:1 resultCollector:^(NSArray *matches, BOOL didWrap) {
- EXPECT_EQ((NSUInteger)1, matches.count);
+ [webView loadHTMLString:@"word word" baseURL:nil];
+ [webView _test_waitForDidFinishNavigation];
- id <NSTextFinderAsynchronousDocumentFindMatch> firstMatchAgain = [matches objectAtIndex:0];
- EXPECT_EQ((NSUInteger)1, firstMatchAgain.textRects.count);
+ // Find one match, doing an incremental search.
+ auto result = findMatches(webView.get(), @"word", noFindOptions, 1);
+ EXPECT_EQ((NSUInteger)1, [result.matches count]);
+ EXPECT_FALSE(result.didWrap);
- EXPECT_TRUE(NSEqualRects([firstMatch.textRects.lastObject rectValue], [firstMatchAgain.textRects.lastObject rectValue]));
-
- findMatchesDone = true;
- }];
- }];
+ result = findMatches(webView.get(), @"word", noFindOptions, 1);
+ EXPECT_EQ((NSUInteger)1, [result.matches count]);
+ EXPECT_FALSE(result.didWrap);
- }];
+ // The next match should fail, because wrapping is disabled.
+ result = findMatches(webView.get(), @"word", noFindOptions, 1);
+ EXPECT_EQ((NSUInteger)0, [result.matches count]);
+ EXPECT_FALSE(result.didWrap);
+}
- TestWebKitAPI::Util::run(&findMatchesDone);
- findMatchesDone = false;
+TEST(WebKit2, FindInPageWrappingSubframe)
+{
+ RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 100, 100)]);
+ [webView _setOverrideDeviceScaleFactor:2];
- // Ensure that the generated image has the correct DPI.
- [webView findMatchesForString:@"Birthday" relativeToMatch:nil findOptions:noFindOptions maxResults:NSUIntegerMax resultCollector:^(NSArray *matches, BOOL didWrap) {
- EXPECT_EQ((NSUInteger)360, matches.count);
+ [webView loadHTMLString:@"word <iframe srcdoc='word'>" baseURL:nil];
+ [webView _test_waitForDidFinishNavigation];
- id <NSTextFinderAsynchronousDocumentFindMatch> firstMatch = [matches objectAtIndex:0];
- [firstMatch generateTextImage:^(NSImage *image) {
- CGImageRef CGImage = [image CGImageForProposedRect:nil context:nil hints:nil];
- EXPECT_EQ(image.size.width, CGImageGetWidth(CGImage) / 2);
- EXPECT_EQ(image.size.height, CGImageGetHeight(CGImage) / 2);
+ // Find one match, doing an incremental search.
+ auto result = findMatches(webView.get(), @"word", wrapFindOptions, 1);
+ EXPECT_EQ((NSUInteger)1, [result.matches count]);
+ EXPECT_FALSE(result.didWrap);
- findMatchesDone = true;
- }];
- }];
+ result = findMatches(webView.get(), @"word", wrapFindOptions, 1);
+ EXPECT_EQ((NSUInteger)1, [result.matches count]);
+ EXPECT_FALSE(result.didWrap);
- TestWebKitAPI::Util::run(&findMatchesDone);
- findMatchesDone = false;
+ // The next match should wrap.
+ result = findMatches(webView.get(), @"word", wrapFindOptions, 1);
+ EXPECT_EQ((NSUInteger)1, [result.matches count]);
+ EXPECT_TRUE(result.didWrap);
- // Ensure that we cap the number of matches. There are actually 1600, but we only get the first 1000.
- [webView findMatchesForString:@" " relativeToMatch:nil findOptions:noFindOptions maxResults:NSUIntegerMax resultCollector:^(NSArray *matches, BOOL didWrap) {
- EXPECT_EQ((NSUInteger)1000, matches.count);
-
- findMatchesDone = true;
- }];
- TestWebKitAPI::Util::run(&findMatchesDone);
- findMatchesDone = false;
+ // Going backward after wrapping should wrap again.
+ result = findMatches(webView.get(), @"word", wrapBackwardsFindOptions, 1);
+ EXPECT_EQ((NSUInteger)1, [result.matches count]);
+ EXPECT_TRUE(result.didWrap);
}
#endif
