After setting playback rate to negative value: when seeking we should calculate start & end time. When playback rate is negative start time should be time passed to 'seek' method, and end time should be '0'.
Created attachment 216012 [details] Patch and Layout test
Comment on attachment 216012 [details] Patch and Layout test inproper url to bugizlla entry
Created attachment 216013 [details] Patch & layout test
Comment on attachment 216013 [details] Patch & layout test View in context: https://bugs.webkit.org/attachment.cgi?id=216013&action=review Thanks for the patch, it's a regression I think... this used to work at some point :) r- only because of the test issue and missing results > LayoutTests/media/video-seek-with-negative-playback.html:1 > +<video src = 'content/test.ogv'></video> Please use findMediaFile()
Created attachment 216019 [details] Patch & layout test
Comment on attachment 216019 [details] Patch & layout test View in context: https://bugs.webkit.org/attachment.cgi?id=216019&action=review Looks good but I have some nits about the test. Can you please fix them? > LayoutTests/media/video-seek-with-negative-playback.html:1 > +<video></video> <video/> It'd be good to make this a normal html file with the usual head and body too. > LayoutTests/media/video-seek-with-negative-playback.html:8 > + function () The coding style is not very consistent about curly braces. Can the test be refactored to avoid the nested waitForEvent()s? > LayoutTests/media/video-seek-with-negative-playback.html:16 > + if ( lastTime == -1) { empty space
Created attachment 216035 [details] Patch & layout test
(In reply to comment #6) > (From update of attachment 216019 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=216019&action=review > > Looks good but I have some nits about the test. Can you please fix them? > > > LayoutTests/media/video-seek-with-negative-playback.html:1 > > +<video></video> > > <video/> > It'd be good to make this a normal html file with the usual head and body too. The problem is that when i use html, head, and body tags test would look much more complicated becouse of needed getElementById calls, etc. > > > LayoutTests/media/video-seek-with-negative-playback.html:8 > > + function () > > The coding style is not very consistent about curly braces. Can the test be refactored to avoid the nested waitForEvent()s? The problem is that we have to wait for seeked event to listen for timeupdate event. Also we can't seek if player is not ready to play. > > > LayoutTests/media/video-seek-with-negative-playback.html:16 > > + if ( lastTime == -1) { > > empty space
(In reply to comment #8) > (In reply to comment #6) > > (From update of attachment 216019 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=216019&action=review > > > > Looks good but I have some nits about the test. Can you please fix them? > > > > > LayoutTests/media/video-seek-with-negative-playback.html:1 > > > +<video></video> > > > > <video/> > > It'd be good to make this a normal html file with the usual head and body too. > > The problem is that when i use html, head, and body tags test would look much more complicated becouse of needed getElementById calls, etc. > Come on, it's not that complicated. There's also a findMediaElement() function... > > > > > LayoutTests/media/video-seek-with-negative-playback.html:8 > > > + function () > > > > The coding style is not very consistent about curly braces. Can the test be refactored to avoid the nested waitForEvent()s? > > The problem is that we have to wait for seeked event to listen for timeupdate event. Also we can't seek if player is not ready to play. > Fine.
Created attachment 216145 [details] Patch & layout test
Comment on attachment 216145 [details] Patch & layout test Clearing flags on attachment: 216145 Committed r158734: <http://trac.webkit.org/changeset/158734>
All reviewed patches have been landed. Closing bug.
(In reply to comment #12) > All reviewed patches have been landed. Closing bug. This new test immediately failed on all Apple bots. Additionally, the test was committed with CRLF line-endings. Cleaned up the test and results in https://trac.webkit.org/r158752.
(In reply to comment #13) > (In reply to comment #12) > > All reviewed patches have been landed. Closing bug. > > This new test immediately failed on all Apple bots. Additionally, the test was committed with CRLF line-endings. Cleaned up the test and results in https://trac.webkit.org/r158752. Oh, I thought the mac EWS was running layout tests nowadays? Sorry about the trouble.