[webkit-reviews] review granted: [Bug 129263] WebKitFindOptions shouldn't expose WEBKIT_FIND_OPTIONS_SHOW_{OVERLAY, FIND_INDICATOR, HIGHLIGHT} : [Attachment 225163] Patch

[bugzilla-daemon at webkit.org]

Sergio Villar Senin <svillar at igalia.com> has granted Enrique Ocaña
<eocanha at igalia.com>'s request for review:
Bug 129263: WebKitFindOptions shouldn't expose
WEBKIT_FIND_OPTIONS_SHOW_{OVERLAY,FIND_INDICATOR,HIGHLIGHT}
https://bugs.webkit.org/show_bug.cgi?id=129263

Attachment 225163: Patch
https://bugs.webkit.org/attachment.cgi?id=225163&action=review

------- Additional Comments from Sergio Villar Senin <svillar at igalia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=225163&action=review


r=me provided you fix the issues I mention.

> Source/WebKit2/UIProcess/API/gtk/WebKitFindController.cpp:86
> +namespace FindOptions {

Let's use a different function name instead of adding another namespace, see
bellow.

> Source/WebKit2/UIProcess/API/gtk/WebKitFindController.cpp:88
> +static inline WebKit::FindOptions toFindOptions(uint32_t type)

I'd suggest toWebFindOptions for example.

BTW, I spot this in the previous review but forgot to add a comment. Let's
change the name of the argument, "type" does not mean anything, we need a more
meaningful name like findOptions for example.

> Source/WebKit2/UIProcess/API/gtk/WebKitFindController.cpp:97
> +static inline WebKitFindOptions toWebKitFindOptions(uint32_t type)

Same thing about the name of the argument.

> Source/WebKit2/UIProcess/API/gtk/WebKitFindController.cpp:106
> +} // namespace WebKitFindController

We can remove this then.