[webkit-reviews] review granted: [Bug 165801] [Mac] -[WKWebView findMatchesForString:relativeToMatch:findOptions:maxResults:resultCollector:] invokes the resultCollector with didWrap = NO even when it wraps : [Attachment 306089] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Apr 3 13:16:00 PDT 2017
Wenson Hsieh <wenson_hsieh at apple.com> has granted Tim Horton
<thorton at apple.com>'s request for review:
Bug 165801: [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
Attachment 306089: Patch
https://bugs.webkit.org/attachment.cgi?id=306089&action=review
--- Comment #9 from Wenson Hsieh <wenson_hsieh at apple.com> ---
Comment on attachment 306089
--> https://bugs.webkit.org/attachment.cgi?id=306089
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=306089&action=review
> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/FindInPage.mm:224
> + [webView findMatchesForString:@"word" relativeToMatch:nil
findOptions:defaultFindOptions maxResults:1 resultCollector:^(NSArray *matches,
BOOL didWrap) {
Minor nit - I would factor out the call to findMatchesForString into a helper
to make these calls more readable (and maybe wait for each one to complete
before moving on so we don't need to nest each step in the test in a new
block).
> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/FindInPage.mm:249
> + findMatchesDone = false;
Side note: I think it's a bit weird that findMatchesDone is a static variable
here :/ I think we should just make it local to each test, but that could be
for another patch later. Also, do we need to set it to false here? I thought
since each test is run in a different process, this state should not bleed into
any subsequent tests.
More information about the webkit-reviews
mailing list