Bug 122049 - REGRESSION(r156546): media/video-no-audio.html broken
Summary: REGRESSION(r156546): media/video-no-audio.html broken
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords:
Depends on: 122021 122043 122111
Blocks:
  Show dependency treegraph
 
Reported: 2013-09-27 22:15 PDT by Jer Noble
Modified: 2013-09-30 09:59 PDT (History)
5 users (show)

See Also:


Attachments
Patch (7.83 KB, patch)
2013-09-27 22:18 PDT, Jer Noble
eric.carlson: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (588.31 KB, application/zip)
2013-09-27 23:07 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion (602.45 KB, application/zip)
2013-09-27 23:25 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion (604.47 KB, application/zip)
2013-09-28 00:28 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2013-09-27 22:15:40 PDT
REGRESSION(r156546): media/video-no-audio.html broken
Comment 1 Jer Noble 2013-09-27 22:18:33 PDT
Created attachment 212877 [details]
Patch
Comment 2 Jer Noble 2013-09-27 22:19:46 PDT
Tests will continue to fail until the fix for bug #122021 is landed.
Comment 3 Jer Noble 2013-09-27 22:52:06 PDT
Rather, till bug #122043 is landed.
Comment 4 Build Bot 2013-09-27 23:07:44 PDT
Comment on attachment 212877 [details]
Patch

Attachment 212877 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/2657372

New failing tests:
media/controls-strict.html
media/controls-styling-strict.html
media/audio-controls-rendering.html
media/click-volume-bar-not-pausing.html
fast/hidpi/video-controls-in-hidpi.html
media/controls-without-preload.html
media/media-controls-clone.html
fast/layers/video-layer.html
media/controls-after-reload.html
Comment 5 Build Bot 2013-09-27 23:07:46 PDT
Created attachment 212878 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 6 Build Bot 2013-09-27 23:25:20 PDT
Comment on attachment 212877 [details]
Patch

Attachment 212877 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/2651351

New failing tests:
media/controls-strict.html
media/controls-styling-strict.html
media/audio-controls-rendering.html
media/click-volume-bar-not-pausing.html
fast/hidpi/video-controls-in-hidpi.html
media/controls-without-preload.html
media/media-controls-clone.html
fast/layers/video-layer.html
media/controls-after-reload.html
Comment 7 Build Bot 2013-09-27 23:25:21 PDT
Created attachment 212879 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 8 Build Bot 2013-09-28 00:28:52 PDT
Comment on attachment 212877 [details]
Patch

Attachment 212877 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/2657393

New failing tests:
media/controls-strict.html
media/controls-styling-strict.html
media/audio-controls-rendering.html
media/click-volume-bar-not-pausing.html
fast/hidpi/video-controls-in-hidpi.html
media/controls-without-preload.html
media/media-controls-clone.html
fast/layers/video-layer.html
media/controls-after-reload.html
Comment 9 Build Bot 2013-09-28 00:28:54 PDT
Created attachment 212881 [details]
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-03  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 10 Eric Carlson 2013-09-30 08:45:43 PDT
Comment on attachment 212877 [details]
Patch

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

> Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:474
> +    handleAudioTrackChange: function(event)
> +    {
> +        this.updateHasAudio();
> +    },
> +
> +    handleAudioTrackAdd: function(event)
> +    {
> +        this.updateHasAudio();
> +    },
> +
> +    handleAudioTrackRemove: function(event)
> +    {
> +        this.updateHasAudio();
> +    },

Why not have a singe event handler for all three events, so each of these does the same thing?
Comment 11 Jer Noble 2013-09-30 08:49:25 PDT
(In reply to comment #10)
> (From update of attachment 212877 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=212877&action=review
> 
> > Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:474
> > +    handleAudioTrackChange: function(event)
> > +    {
> > +        this.updateHasAudio();
> > +    },
> > +
> > +    handleAudioTrackAdd: function(event)
> > +    {
> > +        this.updateHasAudio();
> > +    },
> > +
> > +    handleAudioTrackRemove: function(event)
> > +    {
> > +        this.updateHasAudio();
> > +    },
> 
> Why not have a singe event handler for all three events, so each of these does the same thing?

Sure; that just means registering with updateHasAudio, rather than each individual function.
Comment 12 Jer Noble 2013-09-30 09:08:39 PDT
Committed r156656: <http://trac.webkit.org/changeset/156656>
Comment 13 Alexey Proskuryakov 2013-09-30 09:51:49 PDT
This broke several other tests, and Jer is not on IRC, so I'm rolling out.
Comment 14 WebKit Commit Bot 2013-09-30 09:54:09 PDT
Re-opened since this is blocked by bug 122111
Comment 15 Alexey Proskuryakov 2013-09-30 09:59:48 PDT
Rolled out in <http://trac.webkit.org/changeset/156661>.

Broken tests: <http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK1%20(Tests)/r156656%20(13365)/results.html>. Not sure about the one that timed out - it never failed before, but it's not clear how it could possibly be affected.