Bug 170338 - Modern media controls should never be enabled in non cocoa ports
Summary: Modern media controls should never be enabled in non cocoa ports
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk, LayoutTestFailure
Depends on:
Blocks:
 
Reported: 2017-03-31 05:57 PDT by Carlos Garcia Campos
Modified: 2017-03-31 06:37 PDT (History)
5 users (show)

See Also:

Attachments
Patch (2.78 KB, patch)
2017-03-31 05:59 PDT, Carlos Garcia Campos 		mcatanzaro: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2017-03-31 05:57:27 PDT 
It's currently enabled, because it uses the default value for all other runtime features, but modern media controls are not a cross-platform feature. I think this is why media/video-click-dblckick-standalone.html started to fail after r214426. I can't reproduce the failure locally, though.
Comment 1 Carlos Garcia Campos 2017-03-31 05:59:46 PDT 
Created attachment 305967 [details]
Patch
Comment 2 Michael Catanzaro 2017-03-31 06:31:57 PDT 
Comment on attachment 305967 [details]
Patch

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

This needs a build flag too. Having only a runtime flag really only makes sense if the feature could plausibly work on all platforms. This one is Mac-only.

And please name it with _ENABLED.

> Source/WebKit2/Shared/WebPreferencesDefinitions.h:320
> +#define DEFAULT_MODERN_MEDIA_CONTROLS true

DEFAULT_MODERN_MEDIA_CONTROLS_ENABLED

> Source/WebKit2/Shared/WebPreferencesDefinitions.h:322
> +#define DEFAULT_MODERN_MEDIA_CONTROLS false

DEFAULT_MODERN_MEDIA_CONTROLS_ENABLED
Comment 3 Carlos Garcia Campos 2017-03-31 06:35:46 PDT 
(In reply to Michael Catanzaro from comment #2)
> Comment on attachment 305967 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=305967&action=review
> 
> This needs a build flag too. Having only a runtime flag really only makes
> sense if the feature could plausibly work on all platforms. This one is
> Mac-only.
> 
> And please name it with _ENABLED.

Indeed. That was my intention, but somehow I didn't add that in the end.

> > Source/WebKit2/Shared/WebPreferencesDefinitions.h:320
> > +#define DEFAULT_MODERN_MEDIA_CONTROLS true
> 
> DEFAULT_MODERN_MEDIA_CONTROLS_ENABLED
> 
> > Source/WebKit2/Shared/WebPreferencesDefinitions.h:322
> > +#define DEFAULT_MODERN_MEDIA_CONTROLS false
> 
> DEFAULT_MODERN_MEDIA_CONTROLS_ENABLED
Comment 4 Carlos Garcia Campos 2017-03-31 06:37:43 PDT 
Committed r214666: <http://trac.webkit.org/changeset/214666>