Bug 187847

Created attachment 345435 [details]
Patch

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

Comment on attachment 345435 [details]
Patch

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

(Informal) r+ with a nit

> Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:1336
> +     * but also output capabilities to find the best match based on the deviceâs

Please change the “â” to an apostrophe here... Or is this Bugzilla
not showing UTF-8 or some other encoding correctly?

Comment on attachment 345435 [details]
Patch

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

>> Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:1336
>> +     * but also output capabilities to find the best match based on the deviceâs
> 
> Please change the “â” to an apostrophe here... Or is this Bugzilla
> not showing UTF-8 or some other encoding correctly?

It's a bugzilla bug. I simply verbatim-copied the spec abstract :)

Committed r234038: <https://trac.webkit.org/changeset/234038>

I believe we agreed not to expose new WebKitSettings in the API until the associated feature is largely implemented.

Why is a setting needed? This is something that we will want to be off by default (unavailable in Epiphany)? I doubt any applications will know or want to turn it off if it is on by default? I have to trust your judgment here, because I don't know anything about the feature... I just want to make sure it makes sense to add the API. And avoid the situation where we have enable-mediasource and enable-media-stream that have done nothing for the first five years.

If you read the patch you'll see it's off by default.