Bug 93867 – Fullscreen/normal volume sliders don't stay in sync

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

Fullscreen/normal volume sliders don't stay in sync

[Jer Noble]

Created attachment 158043 [details]
Patch

[WebKit Review Bot]

Comment on attachment 158043 [details]
Patch

Attachment 158043 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13480854

New failing tests:
media/video-controls-fullscreen-volume.html

[WebKit Review Bot]

Created attachment 158066 [details]
Archive of layout-test-results from gce-cr-linux-05

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-05  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid

[Jer Noble]

Created attachment 158075 [details]
Patch

[Eric Carlson]

Comment on attachment 158075 [details]
Patch

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

> LayoutTests/media/video-controls-fullscreen-volume.html:12
> +        var startTest = function() {

Why are the functions declared as local variables?

[Jer Noble]

(In reply to comment #5)
> (From update of attachment 158075 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=158075&action=review
> 
> > LayoutTests/media/video-controls-fullscreen-volume.html:12
> > +        var startTest = function() {
> 
> Why are the functions declared as local variables?

In JavaScript, functions are objects.  So even if you declared them C-style, they'd still be local variables.  (Except, since they're in global scope, they're actually global variables.)  The C-style function definition is just syntactic sugar for this style of declaration.

[Eric Carlson]

(In reply to comment #6)
> (In reply to comment #5)
> > (From update of attachment 158075 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=158075&action=review
> > 
> > > LayoutTests/media/video-controls-fullscreen-volume.html:12
> > > +        var startTest = function() {
> > 
> > Why are the functions declared as local variables?
> 
> In JavaScript, functions are objects.  So even if you declared them C-style, they'd still be local variables.  (Except, since they're in global scope, they're actually global variables.)  The C-style function definition is just syntactic sugar for this style of declaration.

Right, but why declare them this way when very few layout tests do? 

Are pedantry points redeemable now :-) ?

[Jer Noble]

(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > (From update of attachment 158075 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=158075&action=review
> > > 
> > > > LayoutTests/media/video-controls-fullscreen-volume.html:12
> > > > +        var startTest = function() {
> > > 
> > > Why are the functions declared as local variables?
> > 
> > In JavaScript, functions are objects.  So even if you declared them C-style, they'd still be local variables.  (Except, since they're in global scope, they're actually global variables.)  The C-style function definition is just syntactic sugar for this style of declaration.
> 
> Right, but why declare them this way when very few layout tests do? 

Huh, I always write them this way.

> Are pedantry points redeemable now :-) ?

They're worth double with your WebKit Value Card. :)

[WebKit Review Bot]

Comment on attachment 158075 [details]
Patch

Clearing flags on attachment: 158075

Committed r125590: <http://trac.webkit.org/changeset/125590>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.