The Page's muted setting should not affect the HTMLMediaElement's 'muted' IDL attribute

• Component: Media
• Status: RESOLVED FIXED

[Ada Chan]

For example, if the HTMLMediaElement's muted attribute has not been set, but the Page's muted setting is set to true, querying the element's muted property in script should still return false.  The Page's muted setting is more of an override, similar to how the Page's media volume works.

[Ada Chan]

Created attachment 240697 [details]
Patch

[Eric Carlson]

Comment on attachment 240697 [details]
Patch

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

> Source/WebCore/html/HTMLMediaElement.cpp:6042
> +    return (document().page() && document().page()->isMuted()) || muted();

Minor nit: "muted()" is cheaper than "document().page() && document().page()->isMuted()" so it should be first.

> Source/WebCore/testing/Internals.h:349
>      void installMockPageOverlay(const String& overlayType, ExceptionCode&);
>      String pageOverlayLayerTreeAsText(ExceptionCode&) const;
>  
> +    void setMuted(bool);

Nit: I think "page" should be in the method name. "setPageMuted" maybe?

[Ada Chan]

(In reply to comment #2)
> Comment on attachment 240697 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=240697&action=review
> 
> > Source/WebCore/html/HTMLMediaElement.cpp:6042
> > +    return (document().page() && document().page()->isMuted()) || muted();
> 
> Minor nit: "muted()" is cheaper than "document().page() &&
> document().page()->isMuted()" so it should be first.

Fixed.

> 
> > Source/WebCore/testing/Internals.h:349
> >      void installMockPageOverlay(const String& overlayType, ExceptionCode&);
> >      String pageOverlayLayerTreeAsText(ExceptionCode&) const;
> >  
> > +    void setMuted(bool);
> 
> Nit: I think "page" should be in the method name. "setPageMuted" maybe?

Renamed as you suggested.

Thanks Eric!

[Joseph Pecoraro]

Looks like this may have broken the windows build. Probably just a symbol export issue:
<https://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/83112/steps/compile-webkit/logs/stdio>

>      1>WebCoreTestSupport.lib(Internals.obj) : error LNK2019: unresolved external symbol "public: void __thiscall WebCore::Page::setMuted(bool)" (?setMuted@Page@WebCore@@QAEX_N@Z) referenced in function "public: void __thiscall WebCore::Internals::setPageMuted(bool)" (?setPageMuted@Internals@WebCore@@QAEX_N@Z)
>      1>C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\bin32\DumpRenderTree.dll : fatal error LNK1120: 1 unresolved externals
>      1>Done Building Project "C:\cygwin\home\buildbot\slave\win-debug\build\Tools\DumpRenderTree\DumpRenderTree.vcxproj\DumpRenderTree\DumpRenderTree.vcxproj" (Build target(s)) -- FAILED.

[Ada Chan]

Committed:
http://trac.webkit.org/changeset/175384

And as Joe noted, that broke Windows.  First attempt to fix it is:
http://trac.webkit.org/changeset/175393

[Ada Chan]

The Windows 32-bit build error should be fixed with http://trac.webkit.org/changeset/175393.  Roger is going to help me figure out the symbol to export for 64-bit.