[webkit-reviews] review granted: [Bug 206287] [WPE] Expose the WebKitOptionMenu APIs : [Attachment 388218] Patch

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has granted Zan Dobersek
<zan at falconsigh.net>'s request for review:
Bug 206287: [WPE] Expose the WebKitOptionMenu APIs
https://bugs.webkit.org/show_bug.cgi?id=206287

Attachment 388218: Patch

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




--- Comment #13 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 388218
  --> https://bugs.webkit.org/attachment.cgi?id=388218
Patch

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

> Source/WebKit/ChangeLog:61
> +2020-01-19  Zan Dobersek  <zdobersek at igalia.com>

double changelog

> Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:2163
> +	   G_TYPE_BOOLEAN, 1,
> +	   WEBKIT_TYPE_OPTION_MENU);

Since we plan to add the rectangle later I would add the parameter here as a
G_TYPE_POINTER. Then when adding the rectangle we just need to change the type.

> Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:2758
> +    g_signal_emit(webView, signals[SHOW_OPTION_MENU], 0, menu, &handled);

Here we would pass nullptr as the rectangle, and I would add a fixme comment
too.

> Source/WebKit/UIProcess/API/wpe/WPEView.h:44
> +typedef struct _WebKitWebView WebKitWebView;

Do we need this here?

> Source/WebKit/UIProcess/API/wpe/WebKitOptionMenu.h:2
> + * Copyright (C) 2017 Igalia S.L.

2020

> Source/WebKit/UIProcess/API/wpe/WebKitOptionMenuItem.h:2
> + * Copyright (C) 2017 Igalia S.L.

2020

> Source/WebKit/UIProcess/API/wpe/WebKitPopupMenu.cpp:27
> +#include "WebKitWebViewPrivate.h"

Do we need this header here?

> Source/WebKit/UIProcess/API/wpe/WebKitPopupMenu.h:26
> +typedef struct _WebKitWebView WebKitWebView;

We don't need this.

> Source/WebKit/UIProcess/API/wpe/WebKitPopupMenu.h:50
> +    WebKitWebView* m_webView;

And this is now unused.

> Source/WebKit/UIProcess/API/wpe/WebKitWebView.h:250
> +    gboolean       (* show_option_menu)	      (WebKitWebView	       
   *web_view,
> +						       WebKitOptionMenu        
   *menu);

We would need to add a gpointer rectangle parameter here too.