run-launcher --gtk LayoutTests/media/content/test.mp4 Should show the volume slider when clicking on the controls volume button. Right now it toggles mute state.
I wonder if this regression was actually introduced by Bug 82150. Also, as a quick test to do (that I haven't tried), what happens if you make MediaPlayerPrivateGStreamer::hasAudio() always return true? Asking because MediaControlRootElement::showVolumeSlider() checks for it.
(In reply to comment #1) > I wonder if this regression was actually introduced by Bug 82150. > > Also, as a quick test to do (that I haven't tried), what happens if you make MediaPlayerPrivateGStreamer::hasAudio() always return true? Asking because MediaControlRootElement::showVolumeSlider() checks for it. After reverting that and trying, it looks like it is not.
Created attachment 175000 [details] Partially reverting r115829, new baseline and updated test expectations There as a problem caused by r115829 that prevented the volume slider from showing. I reverted the commit partially and updated the tests.
Comment on attachment 175000 [details] Partially reverting r115829, new baseline and updated test expectations View in context: https://bugs.webkit.org/attachment.cgi?id=175000&action=review Looks good, just a nit to fix :) We need to sync with Gustavo when landing this patch though, so no cq for this time. > LayoutTests/platform/gtk-wk2/TestExpectations:589 > +Bug(GTK) media/track/track-cue-container-rendering-position.html [ Failure ] It'd be nice to open a bug ASAP and mention it in the test expectation. So it's not forgotten after. > LayoutTests/platform/gtk/TestExpectations:1394 > +Bug(GTK) media/media-document-audio-repaint.html [ Failure ] > +Bug(GTK) media/track/track-cue-container-rendering-position.html [ Failure ] > +Bug(GTK) media/track/track-cue-rendering-horizontal.html [ Failure ] > +Bug(GTK) media/track/track-cue-rendering-vertical.html [ Failure ] Ditto!
+Gustavo, we'll need a clean build for this patch. Be prepared! Tomorrow maybe?
Created attachment 175185 [details] Partially reverting r115829, new baseline and updated test expectations (In reply to comment #4) > (From update of attachment 175000 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=175000&action=review > > Looks good, just a nit to fix :) We need to sync with Gustavo when landing this patch though, so no cq for this time. Roger that. > > LayoutTests/platform/gtk-wk2/TestExpectations:589 > > +Bug(GTK) media/track/track-cue-container-rendering-position.html [ Failure ] > > It'd be nice to open a bug ASAP and mention it in the test expectation. So it's not forgotten after. > > > LayoutTests/platform/gtk/TestExpectations:1394 > > +Bug(GTK) media/media-document-audio-repaint.html [ Failure ] > > +Bug(GTK) media/track/track-cue-container-rendering-position.html [ Failure ] > > +Bug(GTK) media/track/track-cue-rendering-horizontal.html [ Failure ] > > +Bug(GTK) media/track/track-cue-rendering-vertical.html [ Failure ] > > Ditto! Done. Something that I had forgotten to say: please, have special look at the expectations for media/video-volume-slider as it has some subtantial changes, but I think they are logical because now it should be showing the slider.
Committed r135273: <http://trac.webkit.org/changeset/135273>
Comment on attachment 175185 [details] Partially reverting r115829, new baseline and updated test expectations Heh seems like I forgot to set r+ before landing, but I did the review for real :)