975923 - music play/pause button in utility tray sometimes need twice click to enable it

Status: RESOLVED DUPLICATE of bug 972081

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[yang.zhao]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/535.19 (KHTML, like Gecko) Ubuntu/12.04 Chromium/18.0.1025.151 Chrome/18.0.1025.151 Safari/535.19

Steps to reproduce:

1,open music app
2,play a song then press home button
3,drag down the status bar,click play/pause button  
4,repeat step 2 and step 3


Actual results:

sometimes the play/pause button need to be clicked TWICE to enable it 


Expected results:

click the play/pause button in statusbar ONCE could enable the button

Comment 1

[yang.zhao]

I found that the "click" events in media_playback.js haven't been triggered sometimes,but the "mouse down"/"mouse up"/"touch start"/"touch end" events can be triggered at the same time.

Comment 2

[James Zhang (Spreadtrum)]

We can reproduce this issue on fugu/tarako/hamachi with v1.3/v1.3t branch. 
And we can 100% reproduce this issue on fugu.

Comment 3

[Dominic Kuo [:dkuo]]

I guess the issue is in the system part of the media play back, taking it.

Comment 4

[thomas tsai]

I sometimes can repro this "double-click" issue in FTU, too. The next button may need twice clicks to proceed. But I can only repro this "double-click" issue in tarako.

Comment 5

[Joe Cheng [:jcheng] (please needinfo)]

dominic, seem like you are looking at this already, do you mind providing some brief update? Thanks

Comment 6

[Dominic Kuo [:dkuo]]

I was fixing the other blockers and sorry about not updating this bug, I took a quick test and found, we have music controls in the utility tray(status bar) and lock screen, but this issue is only reproducible on utility tray, I will change the component to system first and continue investigating.

Comment 7

[Steven Yang [:styang]]

It's 100% reproducible.

Comment 8

[Dominic Kuo [:dkuo]]

I tested again on tarako and inari, and found this is probably because the play/pause button is too small for the fingers. When the button is touched, the play/pause button will be highlighted, but if the user's finger move away then leave the play/pause area, play or pause will not be triggered because the last touchend is not on the play/pause button.

I am not 100% sure about this and will continue to investigate.

Comment 9

[Steven Yang [:styang]]

Hi James, should be a HW limitation. Mind to take it? Thanks!

Comment 10

[James Zhang (Spreadtrum)]

You mean it's touch panel hardware issue?
Yang, please add your comment here.

Comment 11

[yang.zhao]

In fact,the click event is binded on the media-playback-container,which is large enough for the finger click.And I also print the x,y when the event 'touch-start'/'touch-end' are triggered in twice clicks,the results show that twice clicks is nearly on the same x,y ,so I think it has nothing to  do with the button is small or not.

'notifications-container' is the parent div of 'media-playback-container'Since we can 100% reproduce this issue on fugu,I found that if I first click on the area 'notifications-container',then click the play/pause button once,the button can be used normally.I guess it maybe has something to do with the container focus or panel active and so on.

Comment 12

[Dominic Kuo [:dkuo]]

(In reply to yang.zhao from comment #11)
> In fact,the click event is binded on the media-playback-container,which is
> large enough for the finger click.And I also print the x,y when the event
> 'touch-start'/'touch-end' are triggered in twice clicks,the results show
> that twice clicks is nearly on the same x,y ,so I think it has nothing to 
> do with the button is small or not.

If touchstart in media-playback-container but touchend is outside of media-playback-container, then click event won't be triggered, so actually the size does matter, we cannot make sure the user's finger will touchstart and touchend at/near the same position.

> 'notifications-container' is the parent div of
> 'media-playback-container'Since we can 100% reproduce this issue on fugu,I
> found that if I first click on the area 'notifications-container',then click
> the play/pause button once,the button can be used normally.I guess it maybe
> has something to do with the container focus or panel active and so on.

This sounds more reasonable to me because it's probably because the play/pause button was not normally pressed if the focused area is not media-playback-container. But what I saw is the play/pause button was highlighted with lightblue so should be already switched the focused on that area.

Comment 13

[Dominic Kuo [:dkuo]]

I tried to keep same gaia and flash different gecko(1.3 ans master) to test then found, this issue is only reproducible on 1.3, so might be a regression caused by gecko, I will try to find the range and also request some help from QA, thanks.

Comment 14

[yang.zhao]

(In reply to Dominic Kuo [:dkuo] from comment #12)
> (In reply to yang.zhao from comment #11)
> > In fact,the click event is binded on the media-playback-container,which is
> > large enough for the finger click.And I also print the x,y when the event
> > 'touch-start'/'touch-end' are triggered in twice clicks,the results show
> > that twice clicks is nearly on the same x,y ,so I think it has nothing to 
> > do with the button is small or not.
> 
> If touchstart in media-playback-container but touchend is outside of
> media-playback-container, then click event won't be triggered, so actually
> the size does matter, we cannot make sure the user's finger will touchstart
> and touchend at/near the same position.
> 
Yes,it does matter.But I have print the x,y ,it shows that the twice clicks are almost at the same position,maybe 1px or 2px deviation.But the first click is not ok,the second is ok.
> > 'notifications-container' is the parent div of
> > 'media-playback-container'Since we can 100% reproduce this issue on fugu,I
> > found that if I first click on the area 'notifications-container',then click
> > the play/pause button once,the button can be used normally.I guess it maybe
> > has something to do with the container focus or panel active and so on.
> 
> This sounds more reasonable to me because it's probably because the
> play/pause button was not normally pressed if the focused area is not
> media-playback-container. But what I saw is the play/pause button was
> highlighted with lightblue so should be already switched the focused on that
> area.

The highlighted color is blue,right?It's defined in CSS:
.media-playback-controls > button:active {
  background-color: #00aacc;
}
But I don't know why the click event hasn't been triggered.

Comment 15

[Joe Cheng [:jcheng] (please needinfo)]

back to dkuo, 1.3T+ for regression

Comment 16

[Dominic Kuo [:dkuo]]

(In reply to yang.zhao from comment #14)
> The highlighted color is blue,right?It's defined in CSS:
> .media-playback-controls > button:active {
>   background-color: #00aacc;
> }
> But I don't know why the click event hasn't been triggered.

Yes, I mean blue though I said lightblue.

I have confirmed different gecko caused this issue in comment 13, so I think the root cause is not touchstart and touchend, and if the click event did fired, then I guess it might be an IAC issue that we should see if the music app did received the IAC commands.

Comment 17

[Jason Smith [:jsmith]]

Switching to QA Wanted to see what happens on 1.3. That will figure out if there's a regression here or not.

Comment 18

[Lionel Mauritson]

After testing on the latest 1.3 & 1.4 and the 1.3 & 1.4 from 2014-02-23, I am unable to confirm this on the Buri.
It is difficult to tell if the touch doesn't register sometimes or if I just missed. 
If the "misses" were touches that were not registered then the repro rate for me is very low; while tapping as I normally would I didn't have any problem on any of my listed builds.
I have, however, noticed that if I press my finger down very slowly it will not register as a touch. Perhaps this is related?
Please let me know if there are additional methods I could use to test this.

Comment 19

[Jason Smith [:jsmith]]

Leaving qawanted here to check if this reproduces on the latest tarako build.

Comment 20

[Steven Yang [:styang]]

Hi Thomas, could you also have someone in your team to check this issue with Dominic in case he needs help? thanks!

Comment 21

[James Zhang (Spreadtrum)]

(In reply to Steven Yang [:styang] from comment #9)
> Hi James, should be a HW limitation. Mind to take it? Thanks!

I don't think it's HW issue because hamachi also has this issue.

Comment 22

[Dominic Kuo [:dkuo]]

Attachment: Media_playback_controls_hightlighted.png

The screenshot when the issue occurred.

Comment 23

[Dominic Kuo [:dkuo]]

Just make sure we are on the same page so I have captured a screenshot of this issue, yang.zhao, do you see the same screenshot(attachment 8392099 [details]) when you reproduced this issue? that's, you think the playpause button is pressed/highlighted but nothing happens? thanks.

Comment 24

[yang.zhao]

(In reply to Dominic Kuo [:dkuo] from comment #23)
> Just make sure we are on the same page so I have captured a screenshot of
> this issue, yang.zhao, do you see the same screenshot(attachment 8392099 [details]
> [details]) when you reproduced this issue? that's, you think the playpause
> button is pressed/highlighted but nothing happens? thanks.

No.When the issue occurred,sometimes the button was highlighted but most of the time not.It just like that no click on the button.

Comment 25

[yang.zhao]

(In reply to yang.zhao from comment #24)
> (In reply to Dominic Kuo [:dkuo] from comment #23)
> > Just make sure we are on the same page so I have captured a screenshot of
> > this issue, yang.zhao, do you see the same screenshot(attachment 8392099 [details]
> > [details]) when you reproduced this issue? that's, you think the playpause
> > button is pressed/highlighted but nothing happens? thanks.
> 
> No.When the issue occurred,sometimes the button was highlighted but most of
> the time not.It just like that no click on the button.

Please ignore the last comment,tested it on the latest version on Tarako,the play/pause button is highlighted when press down but nothing happens.Just like what you said.

Comment 26

[Dominic Kuo [:dkuo]]

(In reply to yang.zhao from comment #25)
> Please ignore the last comment,tested it on the latest version on Tarako,the
> play/pause button is highlighted when press down but nothing happens.Just
> like what you said.

Great, thanks Yang zhao. And I tried to find some similar issue, like bug 935561, the button in sms was also highlighted but the finger already left the button.

Vivien, do you have any idea on this issue? looks like it's related to dom/browser-element/BrowserElementPanning.js, and note that on master we cannot reproduce it so some gecko patch should already fix this, thanks!

Comment 27

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Dominic Kuo [:dkuo] from comment #26)
> (In reply to yang.zhao from comment #25)
> > Please ignore the last comment,tested it on the latest version on Tarako,the
> > play/pause button is highlighted when press down but nothing happens.Just
> > like what you said.
> 
> Great, thanks Yang zhao. And I tried to find some similar issue, like bug
> 935561, the button in sms was also highlighted but the finger already left
> the button.
> 
> Vivien, do you have any idea on this issue? looks like it's related to
> dom/browser-element/BrowserElementPanning.js, and note that on master we
> cannot reproduce it so some gecko patch should already fix this, thanks!

I read the description but on top of my head nothing should do this for regular content.
That said the system app has always be a special app when it comes to events and how they are dispatched.

Back in the days the system app used to receive both mouse events and touch events, without any respects to the touch specification.
First someone needs to check if that still the case, in the sense that mouse events are delivered along touch events, not following the spec order.

Because of this behavior BrowserElementPanning.js has a special case to handle such app, which is controlled by a flag at http://mxr.mozilla.org/mozilla-central/source/dom/browser-element/BrowserElementPanning.js#27

This flag has the consequence of preventing the click event to be fired because of a mouse events interaction that is not triggered by touch events. Because in the past the events in the system app used to be:

mousedown, touchstart, mousemove, touchmove, mouseup, touchend, click, mousemove mousedown, mouseup, click

So it worth checking this special behavior and all the cases that set the |this.preventNextClick| flag in BrowserElementPanning.js to see if the click is cancelled because of a bug in this code.

That said, since it works on master I would suspect that the issue is elsewhere. Since the 1.3 freeze I land those 2 changes to BEP.js:
 bug 972081 and bug 978544

Maybe one of them helps with the issue (bug 972081 notably remove some spinning of the event loop).
I also fixed bug 978546, as events were sometimes dispatched strangely between the notification tray and the current app while panning. Worth checking too.

Comment 28

[Dominic Kuo [:dkuo]]

Thanks Vivien, these information really helps! I will pick up these patches and hope we can find the one that fix this issue, will update here.

Comment 29

[Dominic Kuo [:dkuo]]

Okay, after applied the patch in bug 972081 this issue seems fixed to me, I couldn't reproduce it with the patch in bug 972081, we should uplift it to fix this issue.

Yang zhao, do you mind to apply bug 972081 and test if the patch does solve this issue? thanks.

Comment 30

[yang.zhao]

(In reply to Dominic Kuo [:dkuo] from comment #29)
> Okay, after applied the patch in bug 972081 this issue seems fixed to me, I
> couldn't reproduce it with the patch in bug 972081, we should uplift it to
> fix this issue.
> 
> Yang zhao, do you mind to apply bug 972081 and test if the patch does solve
> this issue? thanks.

I've applied bug 972081,and it seems works on Tarako.I also applied the patch to Fugu,on which the issue always occurs,and find that Fugu is OK too!I think the patch does solve this issue!

Comment 31

[Joe Cheng [:jcheng] (please needinfo)]

Dominic, do you feel this is a dup of Bug 972081? Thanks

Comment 32

[Dominic Kuo [:dkuo]]

(In reply to Joe Cheng [:jcheng] from comment #31)
> Dominic, do you feel this is a dup of Bug 972081? Thanks

I am not sure because bug 972081 is a gecko issue which I am not familiar with, but since Yang and I both confirmed bug 972081 dose solve this issue, I think we should just uplift bug 972081 to 1.3t(maybe 1.3 and 1.4 as well), so I am fine with this bug depends on it or duplicate it.

Comment 34

[John Hammink]

Gaia      8132b88334f61288060a2680857c54c7af82cdc5
Gecko     57625761da9e5c35d1ddd516221ce274a3575d15
BuildID   20140325060053
Version   28.0
ro.build.version.incremental=121
ro.build.date=Tue Mar 25 06:09:04 CST 2014

Verified fixed against the original STR.