[Webkit-unassigned] [Bug 219995] [GTK] Expose setCORSDisablingPatterns

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=219995

Michael Catanzaro <mcatanzaro at gnome.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #416469|review?                     |review-
              Flags|                            |

--- Comment #3 from Michael Catanzaro <mcatanzaro at gnome.org> ---
Comment on attachment 416469
  --> https://bugs.webkit.org/attachment.cgi?id=416469
Patch

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

I like it!

> Source/WebKit/ChangeLog:8
> +        No new tests (OOPS!).

Constructing a test for this will be a lot harder than just implementing the API, but it's required to land new WebKit API.

Something very simple in WebViewTest.cpp should suffice.

> Source/WebKit/UIProcess/API/gtk/WebKitWebView.h:594
> +                                                      const gchar * const       *patternList);

gchar* (the * hangs on the type)

> Source/WebKit/UIProcess/API/gtk/WebKitWebViewGtk.cpp:469
> +void webkit_web_view_set_cors_disabling_patterns(WebKitWebView* webView, const gchar * const *patternList)

Needs a doc comment with introspection annotations and a Since: 2.32 tag.

Also, remember that in WebKit (and most C++ projects) the * hangs on the type, not the variable name. So it's: const gchar* const * patternList

> Source/WebKit/UIProcess/API/gtk/WebKitWebViewGtk.cpp:477
> +    for (size_t i = 0; patternList[i]; ++i)

We normally use: i++

(Unless there is really some good reason to use prefix increment, which in this case, there is not.)

> Source/WebKit/UIProcess/API/gtk/WebKitWebViewGtk.cpp:480
> +    auto& page = *webkitWebViewBaseGetPage(reinterpret_cast<WebKitWebViewBase*>(webView));

I was going to say: you can use a GObject cast: WEBKIT_WEB_VIEW_BASE(webView)

But: actually you can't, because WebKitWebViewBase doesn't exist for WPE port. So you'd need an #if PLATFORM(GTK) condition. Then I got confused: why is the WPE EWS green? Finally, I noticed you added this to WebKitWebViewGtk.cpp.

OK, well, there is nothing GTK-specific here. So move this to Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp instead. Then, use getPage(webView) to get the page without having to type WebKitWebViewBase directly, and now this will work for WPE. And lastly, modify Source/WebKit/UIProcess/API/wpe/WebKitWebView.h.

> Source/WebKit/UIProcess/API/gtk/docs/webkit2gtk-4.0-sections.txt:293
> +webkit_web_view_set_cors_disabling_patterns

Needs to be added to WPE docs as well.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20201217/0d7d4161/attachment.htm>