[webkit-reviews] review granted: [Bug 201775] FindController::findString always updates foundStringMatchIndex even if match is the same as before : [Attachment 380349] Patch

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has granted mmokary at apple.com's request for
review:
Bug 201775: FindController::findString always updates foundStringMatchIndex
even if match is the same as before
https://bugs.webkit.org/show_bug.cgi?id=201775

Attachment 380349: Patch

https://bugs.webkit.org/attachment.cgi?id=380349&action=review




--- Comment #10 from Tim Horton <thorton at apple.com> ---
Comment on attachment 380349
  --> https://bugs.webkit.org/attachment.cgi?id=380349
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=380349&action=review

> Source/WebKit/Shared/API/c/WKFindOptions.h:44
> +    kWKFindOptionsShowHighlight = 1 << 7,
> +    kWKFindOptionsNoIndexChange = 1 << 8

You shouldn't have to add this

> Source/WebKit/Shared/API/c/WKSharedAPICast.h:772
> +    if (wkFindOptions & kWKFindOptionsNoIndexChange)
> +	   findOptions |= FindOptionsNoIndexChange;

Or this.

> Source/WebKit/Shared/WebFindOptions.h:40
> +    FindOptionsNoIndexChange = 1 << 8,
> +    FindOptionsDetermineMatchIndex = 1 << 9,

Technically fine in this case because it's internal only but still weird to add
things in the middle of the enum.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewFindString.mm:192
> +    [firstWebView _findString:@"hello" options:findOptions
maxCount:maxCount];

This is ObjC API so it should always take ObjC API enums (_WKFindOptions and
_WKFindOptionsNoIndexChange). What went wrong that you have to use the C API
constant?! Looking at the implementation of _findString:options:maxCount: in
WKWebView, it uses the toFindOptions() function that's right above it, which
goes from _WKFindOptions->WebKit::FindOptions, so I think this is just wrong.