1124605 - After perform Seek twice, the playing status doesn't consistent with video control bar.

Status: RESOLVED FIXED

(Toolkit :: Video/Audio Controls, defect)

Description

[Benjamin Chen [:bechen]]

STR: on b2g platform.
1. open a HTTP streaming by browser.
2. click to open video control panel
3. click to seek twice quickly

After the second seek operation, the audio/video is paused, but the video control shows it is playing(shows pause button).

I have trace a little code, guess something wrong here:

https://dxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/videocontrols.xml?from=videocontrols.xml#171

https://dxr.mozilla.org/mozilla-central/source/layout/xul/nsSliderFrame.cpp?from=nssliderframe.cpp#211

The second seek seems has only one DragStateChanged event, it should be two?

Comment 1

[Benjamin Chen [:bechen]]

When I try to click the second seek, the video control panel seems disappearing.

Comment 2

[Benjamin Chen [:bechen]]

These days I had tried to dig more about the bug.

Now I think there are 2 problems inside.
1. The second's seek operation's touch end doesn't dispatch to the nsSliderFrame. So we should ensure the nsSliderFrame receive "NS_TOUCH_END" eventually.
- https://dxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp?from=nspresshell.cpp#495
-- I guess the nsIFrame is null because the control bar is hiding.
- Maybe I can add StopDrag() in nsSliderFrame::HandleRelease()?

2. The state transition in MediaDecoder.cpp.
During the seeking operation, the MediaDecoder will receive seek->pause->play->seekstopped
The pause and play are called by videocontrols.xml and solve problem 1 ensure the play will be called.
So the problem is when the MediaDecoder::Play is called, the mNextState is still remain in PLAY_STATE_PAUSED, then seekstopped transit the state to PLAY_STATE_PAUSED.

Comment 3

[Benjamin Chen [:bechen]]

Attachment: bug-1124605.v01.patch

In this patch, I add a flag "isDraggingComplete" and call dragStateChanged(false) before the element is hidden, so that we can ensure the video element's playing status after seek.

Hi Chris and Ben:
Could you give me some feedback or review about this patch?

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

bechen: We fixed a several bugs that sound similar in the Gecko layer recently. Would you be able to test with a recent mozilla-central?

I think you should ask Jared Wein to provide feedback on the patch here; he works on and reviews the video controls code.

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8556943 [details] [diff] [review]
bug-1124605.v01.patch

Review of attachment 8556943 [details] [diff] [review]:
-----------------------------------------------------------------

Jared would be a better person to provide feedback here.

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Ben, did you test with a recent mozilla-central? I'm having trouble reproducing this bug on desktop Firefox Nightly (Windows 8.1, 38.01a 2015-02-09).

Comment 7

[Benjamin Chen [:bechen]]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #6)
> Ben, did you test with a recent mozilla-central? I'm having trouble
> reproducing this bug on desktop Firefox Nightly (Windows 8.1, 38.01a
> 2015-02-09).

Hi Jared:
I can't reproduce this bug on desktop build, clearly something different between the mouse event/touch event.
But I can reproduce this bug easily at b2g platform on flame device.

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8556943 [details] [diff] [review]
bug-1124605.v01.patch

Review of attachment 8556943 [details] [diff] [review]:
-----------------------------------------------------------------

I don't think this bug will affect desktop controls, since they behave slightly differently than the mobile controls (and also there is some delay with tapping on a mobile device while it waits to see if the intent is a pinch or other gesture). With that being said, this looks like a harmless change for desktop.

Comment 9

[Benjamin Chen [:bechen]]

try server:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=aa792bd6f702

Comment 10

[Benjamin Chen [:bechen]]

Attachment: bug-1124605.v01.patch

r=jaws

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/1fd60e2c17ae

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1fd60e2c17ae