[webkit-reviews] review granted: [Bug 207384] [GTK][WPE] Expose allowTopNavigationToDataURL : [Attachment 390093] Patch

[bugzilla-daemon at webkit.org]

Adrian Perez <aperez at igalia.com> has granted Lauro Moura <lmoura at igalia.com>'s
request for review:
Bug 207384: [GTK][WPE] Expose allowTopNavigationToDataURL
https://bugs.webkit.org/show_bug.cgi?id=207384

Attachment 390093: Patch

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




--- Comment #5 from Adrian Perez <aperez at igalia.com> ---
Comment on attachment 390093
  --> https://bugs.webkit.org/attachment.cgi?id=390093
Patch

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

Patch looks great, thanks for submitting it! I think we should land this,
and it only needs a couple of small bits in the API documentation. Feel
free to land it after taking care of them.

> Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:1449
> +	* Whether or not the top frame is allowed to navigate to data URLS.

Typo: s/URLS/URLs

It would be nicer to have a couple of lines additional lines here in the API
documentation explaining the reason for the default (disabled), and telling
why some application which embeds a WebKitWebView might want to enable it.

For some cues as to why disabling top-level data:// loads be default is good,
there's this Mozilla blog post:

 
https://blog.mozilla.org/security/2017/11/27/blocking-top-level-navigations-dat
a-urls-firefox-59/

As for when to enable it, I think that the main use-case is when an application
which is *NOT* a generic web browser embeds a WebKitWebView widget and wants
to load data:// URLs using webkit_web_view_load_uri() to provide content to
be displayed that is controlled by the application.

> Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:1457
> +	       _("Whether or not top frame navigation is allowed to data
URLS"),

Typo: s/URLS/URLs