Bug 196863 - [GTK][WPE] Add enable-media websetting
Summary: [GTK][WPE] Add enable-media websetting
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Philippe Normand
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-04-12 09:13 PDT by Philippe Normand
Modified: 2019-04-15 07:41 PDT (History)
6 users (show)

See Also:


Attachments
Patch (10.01 KB, patch)
2019-04-12 09:17 PDT, Philippe Normand
no flags Details | Formatted Diff | Diff
Patch (10.14 KB, patch)
2019-04-15 03:23 PDT, Philippe Normand
mcatanzaro: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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 Build Bot 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>