Closed Bug 902393 Opened 6 years ago Closed 6 years ago

[Utility tray] Once the utility tray is pulled down it is not possible to pull it back up

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:koi+, b2g-v1.2 fixed)

RESOLVED FIXED
blocking-b2g koi+
Tracking Status
b2g-v1.2 --- fixed

People

(Reporter: isabelrios, Assigned: vingtetun)

References

Details

(Keywords: regression, Whiteboard: burirun1)

Attachments

(2 files)

Unagi device master 08/07 build:
Gecko-a469a2f
Gaia-90c0082
ref ril

STR
1. Form homescreen, pull down the utility tray
2. Try to close it by pulling up

EXPECTED
The utility tray is hidden

ACTUAL
They only way to close the utility tray is by tapping on home key

This is a regression
blocking-b2g: --- → koi?
Keywords: regression
Keywords: smoketest
David, can you all take a look at this regression on 1.2?
Flags: needinfo?(dscravaglieri)
No longer blocks: b2g-central-dogfood
Keywords: smoketest
Duplicate of this bug: 905268
Can we assign someone here? It's really a clownshoes regression.
Duplicate of this bug: 913218
Has anything changed with touch events recently? This is odd because it's working properly in Firefox Nightly, but is broken on a device. (We use a touch event shim, and not real touch events in the browser)

I am seeing the wrong event target being passed into the touch event listener which is causing an early return here: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/utility_tray.js#L50
As kevin says, the last change to utilit-tray is two month ago. Could we have regression window here? I don't think this is a gaia issue..
(In reply to Alive Kuo [:alive] from comment #6)
> As kevin says, the last change to utilit-tray is two month ago. Could we
> have regression window here? I don't think this is a gaia issue..

Sure.
QA Contact: nkot
Duplicate of this bug: 915264
Regression range:
Build ID: 20130806104538 - Last working
Gecko: http://hg.mozilla.org/mozilla-central/rev/1e381c91885d
Gaia: 42c4efb7550820b7b6d6086d419a32a9e0cad174
Platform Version: 26.0a1

Build ID: 20130807070231 - First broken
Gecko: http://hg.mozilla.org/mozilla-central/rev/1fb5d14e8348
Gaia: 1dba2b511bb09eff8d5e18636d08db26799e2483
Platform Version: 26.0a1
Whiteboard: burirun1
(In reply to Kevin Grandon :kgrandon from comment #5)
> Has anything changed with touch events recently? This is odd because it's
> working properly in Firefox Nightly, but is broken on a device. (We use a
> touch event shim, and not real touch events in the browser)
> 
> I am seeing the wrong event target being passed into the touch event
> listener which is causing an early return here:
> https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/utility_tray.
> js#L50

kevin or alive, can one of you take this?   this is gotta be koi+'d.  regression range in comment 9, dating back to aug 7th.
Flags: needinfo?(kgrandon)
Flags: needinfo?(alive)
Annoying, taken :/
Assignee: nobody → alive
Flags: needinfo?(alive)
Attached file 902393.patch
Fix by:
1. Change touch event target
2. Add autoMoving to UtilityTray
3. Disable click event when utility tray is auto moving.
Attachment #809756 - Flags: review?(timdream)
Attachment #809756 - Flags: review?(timdream) → review+
Clearing the flag, thanks for taking.
Flags: needinfo?(kgrandon)
Alive, do we really need all those changes? This bug is just a event fluffling regression and could be fixed simply with the above patch.
(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #15)
> Created attachment 810230 [details] [diff] [review]
> event.target,fluffing.regression.patch
> 
> Alive, do we really need all those changes? This bug is just a event
> fluffling regression and could be fixed simply with the above patch.

I like the removal of the touch events listener on all the window to replace it with overlay/statusbar though.
(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #16)
> (In reply to Vivien Nicolas (:vingtetun) (:21) from comment #15)
> > Created attachment 810230 [details] [diff] [review]
> > event.target,fluffing.regression.patch
> > 
> > Alive, do we really need all those changes? This bug is just a event
> > fluffling regression and could be fixed simply with the above patch.
> 
> I like the removal of the touch events listener on all the window to replace
> it with overlay/statusbar though.

Hm, from your patch and commit history I don't see what's regressed..
To be honest I don't know who regresses this bug either.
(In reply to Alive Kuo [:alive] from comment #17)
> (In reply to Vivien Nicolas (:vingtetun) (:21) from comment #16)
> > (In reply to Vivien Nicolas (:vingtetun) (:21) from comment #15)
> > > Created attachment 810230 [details] [diff] [review]
> > > event.target,fluffing.regression.patch
> > > 
> > > Alive, do we really need all those changes? This bug is just a event
> > > fluffling regression and could be fixed simply with the above patch.
> > 
> > I like the removal of the touch events listener on all the window to replace
> > it with overlay/statusbar though.
> 
> Hm, from your patch and commit history I don't see what's regressed..
> To be honest I don't know who regresses this bug either.

This is a regression from bug 789358. It changes the way touch event targets are determined.
(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #19)
> (In reply to Alive Kuo [:alive] from comment #17)
> > (In reply to Vivien Nicolas (:vingtetun) (:21) from comment #16)
> > > (In reply to Vivien Nicolas (:vingtetun) (:21) from comment #15)
> > > > Created attachment 810230 [details] [diff] [review]
> > > > event.target,fluffing.regression.patch
> > > > 
> > > > Alive, do we really need all those changes? This bug is just a event
> > > > fluffling regression and could be fixed simply with the above patch.
> > > 
> > > I like the removal of the touch events listener on all the window to replace
> > > it with overlay/statusbar though.
> > 
> > Hm, from your patch and commit history I don't see what's regressed..
> > To be honest I don't know who regresses this bug either.
> 
> This is a regression from bug 789358. It changes the way touch event targets
> are determined.

OK, this explains why I see the event target is #quick-settings when I touch #utility-tray-grippy.
Assignee: alive → nobody
Please steal this bug since I failed on finding out the root cause.
(In reply to Alive Kuo [:alive] from comment #21)
> Please steal this bug since I failed on finding out the root cause.

I knew it because I push to enable event fluffling. I will not have guess it otherwise ;)
Comment on attachment 810230 [details] [diff] [review]
event.target,fluffing.regression.patch

Let me know if you like this patch.
Attachment #810230 - Flags: review?(alive)
Comment on attachment 810230 [details] [diff] [review]
event.target,fluffing.regression.patch

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

I hope to remove event listener on window object.

::: apps/system/js/utility_tray.js
@@ +18,5 @@
>  
>    init: function ut_init() {
>      var touchEvents = ['touchstart', 'touchmove', 'touchend'];
>      touchEvents.forEach(function bindEvents(name) {
>        window.addEventListener(name, this);

Could you change the event target here to utility-tray + statusbar only?
Attachment #810230 - Flags: review?(alive) → review+
(In reply to Alive Kuo [:alive] from comment #24)
> Comment on attachment 810230 [details] [diff] [review]
> event.target,fluffing.regression.patch
> 
> Review of attachment 810230 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I hope to remove event listener on window object.
> 
> ::: apps/system/js/utility_tray.js
> @@ +18,5 @@
> >  
> >    init: function ut_init() {
> >      var touchEvents = ['touchstart', 'touchmove', 'touchend'];
> >      touchEvents.forEach(function bindEvents(name) {
> >        window.addEventListener(name, this);
> 
> Could you change the event target here to utility-tray + statusbar only?

Sure. I will change it to utility-tray + statusbar + grippy.
Vivien is working on it
Assignee: nobody → 21
blocking-b2g: koi? → koi+
Flags: needinfo?(dscravaglieri)
Duplicate of this bug: 922888
(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #25)
> (In reply to Alive Kuo [:alive] from comment #24)
> > Comment on attachment 810230 [details] [diff] [review]
> > event.target,fluffing.regression.patch
> > 
> > Review of attachment 810230 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > I hope to remove event listener on window object.
> > 
> > ::: apps/system/js/utility_tray.js
> > @@ +18,5 @@
> > >  
> > >    init: function ut_init() {
> > >      var touchEvents = ['touchstart', 'touchmove', 'touchend'];
> > >      touchEvents.forEach(function bindEvents(name) {
> > >        window.addEventListener(name, this);
> > 
> > Could you change the event target here to utility-tray + statusbar only?
> 
> Sure. I will change it to utility-tray + statusbar + grippy.

No need to do this since the grippy is pointer-events-none.
BTW I added a pointer-events:none on #quick-settings in bug 924479 (not the link, just the background).
> No need to do this since the grippy is pointer-events-none.
> BTW I added a pointer-events:none on #quick-settings in bug 924479 (not the
> link, just the background).

Scratch this.
(Just saw the patch content)
Duplicate of this bug: 925010
Duplicate of this bug: 925695
What is missing here to land this?
Flags: needinfo?(21)
https://github.com/mozilla-b2g/gaia/commit/33583bf8b98f38eb265a2a95443e9695278ad3f7
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: needinfo?(21)
Resolution: --- → FIXED
I was not able to uplift this bug to v1.2.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1.2, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1.2
  git cherry-pick -x -m1 33583bf8b98f38eb265a2a95443e9695278ad3f7
  <RESOLVE MERGE CONFLICTS>
  git commit
Flags: needinfo?(21)
I tried the fix on an Unagi (inbound & gaia-master), the drag is correctly fixed but the depth of the status bar is not always the same.  Sometimes the status bar appears on top, or below the shade of the utility tray.
Uplifted 33583bf8b98f38eb265a2a95443e9695278ad3f7 to:
v1.2: 69e0b55e0f17af6a6f755f058d03a7189ccf5728
Sounds like John did the job here.
You need to log in before you can comment on or make changes to this bug.