Bug 170338

Summary: Modern media controls should never be enabled in non cocoa ports
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, calvaris, dino, graouts, mcatanzaro
Priority: P2 Keywords: Gtk, LayoutTestFailure
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch mcatanzaro: review+

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>