Bug 196863 – [GTK][WPE] Add enable-media websetting

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Philippe Normand]

It can be useful for headless browsers, for instance.

[Philippe Normand]

Created attachment 367324 [details]
Patch

[EWS Watchlist]

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

[Michael Catanzaro]

Comment on attachment 367324 [details]
Patch

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

Will you have a follow-up patch to make code check the setting, I presume?

> Source/WebKit/Shared/WebPreferences.yaml:531
> +  condition: ENABLE(VIDEO)

Er what about audio?

> Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:1510
> +     * WebKitSettings:enable-media:
> +     *
> +     *
> +     * Enable or disable support for Media on pages. This setting is enabled by
> +     * default. Disabling it means <audio> and <video> elements will have
> +     * playback support disabled.

Media -> media

Since: @2.26

> Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:1516
> +            _("Whether Media content should be handled"),

Media -> media

> Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:3708
> + * Returns: %TRUE If media support is enabled or %FALSE otherwise.

If -> if

Since: @2.26

> Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:3722
> + * Set the #WebKitSettings:enable-media property.

Since: @2.26

> Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitSettings.cpp:346
> +

Remove this extra blank line.

[Philippe Normand]

(In reply to Michael Catanzaro from comment #3)
> Comment on attachment 367324 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=367324&action=review
> 
> Will you have a follow-up patch to make code check the setting, I presume?
> 

What do you mean? The patch includes a unit-test.

> > Source/WebKit/Shared/WebPreferences.yaml:531
> > +  condition: ENABLE(VIDEO)
> 
> Er what about audio?
> 

ENABLE(VIDEO) implies <audio>

[Michael Catanzaro]

(In reply to Philippe Normand from comment #4)
> What do you mean? The patch includes a unit-test.

I mean: currently the setting does nothing. You have a patch coming to make it work, right?

[Philippe Normand]

I tested it with MB, it works.

[Philippe Normand]

`mediaEnabled` is handled in make_names.pl which generates HTMLElementFactory.cpp

[Philippe Normand]

Created attachment 367413 [details]
Patch

[Michael Catanzaro]

Comment on attachment 367413 [details]
Patch

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

> Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:1507
> +     *

You have an extra blank line here.

[Philippe Normand]

Committed r244260: <https://trac.webkit.org/changeset/244260>