Bug 196863

Summary: [GTK][WPE] Add enable-media websetting
Product: WebKit Reporter: Philippe Normand <pnormand>
Component: WebKitGTKAssignee: Philippe Normand <pnormand>
Status: RESOLVED FIXED    
Severity: Normal CC: berto, bugs-noreply, cgarcia, ews-watchlist, gustavo, mcatanzaro
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch mcatanzaro: review+

Description Philippe Normand 2019-04-12 09:13:41 PDT
It can be useful for headless browsers, for instance.
Comment 1 Philippe Normand 2019-04-12 09:17:02 PDT
Created attachment 367324 [details]
Patch
Comment 2 EWS Watchlist 2019-04-12 09:19:07 PDT
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 3 Michael Catanzaro 2019-04-12 10:26:03 PDT
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.
Comment 4 Philippe Normand 2019-04-12 10:38:42 PDT
(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>
Comment 5 Michael Catanzaro 2019-04-12 10:55:36 PDT
(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?
Comment 6 Philippe Normand 2019-04-12 10:59:25 PDT
I tested it with MB, it works.
Comment 7 Philippe Normand 2019-04-12 11:06:16 PDT
`mediaEnabled` is handled in make_names.pl which generates HTMLElementFactory.cpp
Comment 8 Philippe Normand 2019-04-15 03:23:12 PDT
Created attachment 367413 [details]
Patch
Comment 9 Michael Catanzaro 2019-04-15 07:20:49 PDT
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.
Comment 10 Philippe Normand 2019-04-15 07:41:37 PDT
Committed r244260: <https://trac.webkit.org/changeset/244260>