[search] edge gestures keep working when clicking on the rockerbar

VERIFIED FIXED in 2.2 S10 (17apr)

Status

defect
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: apastor, Assigned: etienne)

Tracking

unspecified
2.2 S10 (17apr)
x86
macOS

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.2 verified, b2g-master verified)

Details

(Whiteboard: [systemsfe], )

Attachments

(2 attachments)

Reporter

Description

4 years ago
STR:

1.- Open 2 apps
2.- Go to one of them
3.- Click on the rocketbar and quickly start switching between apps with an edge gesture

Not sure what's the expected behavior here. Probably that the edge gesture doesn't work at all after clicking on the search.
Reporter

Comment 1

4 years ago
 Rob, could you please confirm what's the expected behavior here?
Flags: needinfo?(rmacdonald)
Whiteboard: [systemsfe]
Hi Alberto...

Sorry for the delay. I went back through the specs and this isn't referenced so there's a bit of a gap here. Generally, though, as I was working with Etienne on sheets we tried to minimize disabling the edge gesture to make it universally available and predictable. So in apps where the keyboard is displayed for an inline form edge gestures work and the keyboard is dismissed as the user activates the edge gesture.

I would apply the same logic to rocketbar and dismiss the keyboard and search in a similar fashion to the inline keyboard scenarios. However, from what I recall there may have been some constraints on this so I'll flag Etienne as he likely has a better recollection on this and any proposed solution.

- Rob
Flags: needinfo?(rmacdonald) → needinfo?(etienne)
Assignee

Comment 3

4 years ago
For the inline keyboard case what's happening is really clear, you can see the 2 app windows moving below your finger while the keyboard is hiding, and you clearly "grabbed" one of the window.

I'm afraid it might be a bit weird in the case of the rocketbar, because of the rocketbar overlay hiding the app windows below.

So I don't have a strong opinion. But if disabling the rocketbar when a sheet-gesture-begins result in jankiness/weirdness I'm fine with just disabling the edge gestures while the rocketbar is focused.
Flags: needinfo?(etienne)
Rob, can you make a decision if we should fix this or close the bug?
Flags: needinfo?(rmacdonald)
Thanks for flagging me, Gregor. I propose we disable the edge gesture for now to maintain the polish. We can then revisit the transition in an upcoming release when we have more time to test it. Let me know if you have any concerns. - Rob
Flags: needinfo?(rmacdonald)
alberto, can you look into doing what rob suggested in commment 5?
blocking-b2g: --- → 2.2+
Flags: needinfo?(apastor)
Reporter

Updated

4 years ago
Assignee: nobody → apastor
Flags: needinfo?(apastor)
Reporter

Updated

4 years ago
Attachment #8591568 - Flags: review?(kgrandon)
Comment on attachment 8591568 [details] [review]
[gaia] albertopq:1138956-edge-rocketbar > mozilla-b2g:master

Etienne - could you review this?

I think it also might be better to change the approach to listen to rocketbar events so we don't have to introduce coupling to the rocketbar object.
Attachment #8591568 - Flags: review?(kgrandon) → review?(etienne)
Assignee

Comment 9

4 years ago
Comment on attachment 8591568 [details] [review]
[gaia] albertopq:1138956-edge-rocketbar > mozilla-b2g:master

(In reply to Kevin Grandon :kgrandon from comment #8)
> Comment on attachment 8591568 [details] [review]
> [gaia] albertopq:1138956-edge-rocketbar > mozilla-b2g:master
> 
> Etienne - could you review this?
> 
> I think it also might be better to change the approach to listen to
> rocketbar events so we don't have to introduce coupling to the rocketbar
> object.

Definitely!

We should disable the edge zones when entering the rocketbar, like [1] and re-enable it when it closes (if we're not on the homescreen), like [2].

We don't want to forward at all here since we would be forwarding to an app window the user can't see.


[1] https://github.com/mozilla-b2g/gaia/blob/43f67fb074c326286bdb15b31749b0905d427558/apps/system/js/edge_swipe_detector.js#L127
[2] https://github.com/mozilla-b2g/gaia/blob/43f67fb074c326286bdb15b31749b0905d427558/apps/system/js/edge_swipe_detector.js#L154-156
Attachment #8591568 - Flags: review?(etienne) → review-
Reporter

Comment 10

4 years ago
The edge gestures are already disabled via css on [1]. The patch is trying to solve the race condition between clicking on the rocketbar and start swiping. @Etienne, I'm sure you are right about forwarding, but I cannot see how 'this.lifecycleEnabled = false;' would solve the problem shown in the video. Touch events will still move the cards, won't they?

@Kevin, regarding the events, we receive 'touchstart' and 'touchmove' events before receiving the 'rocketbar-activating' one, so I'm not sure if we can use them.

[1] https://github.com/mozilla-b2g/gaia/blob/43f67fb074c326286bdb15b31749b0905d427558/apps/system/style/statusbar/statusbar.css#L9
Flags: needinfo?(kgrandon)
Flags: needinfo?(etienne)
Assignee

Comment 11

4 years ago
(In reply to Alberto Pastor [:albertopq] from comment #10)
> The edge gestures are already disabled via css on [1]. The patch is trying
> to solve the race condition between clicking on the rocketbar and start
> swiping. @Etienne, I'm sure you are right about forwarding, but I cannot see
> how 'this.lifecycleEnabled = false;' would solve the problem shown in the
> video. Touch events will still move the cards, won't they?

If it comes to this we can chose to ignore those touch events without forwarding them, but:

Isn't rocketbar-activating [1] being dispatched in the same tick where we set |rocketbar.active| (that you use in your initial patch)?

[1] https://github.com/mozilla-b2g/gaia/blob/00130552f1913dab822762ba85d47ba2825bf80e/apps/system/js/rocketbar.js#L132
Flags: needinfo?(etienne)
Assignee

Comment 12

4 years ago
(In reply to Alberto Pastor [:albertopq] from comment #10)
> @Kevin, regarding the events, we receive 'touchstart' and 'touchmove' events
> before receiving the 'rocketbar-activating' one, so I'm not sure if we can
> use them.

We won't start to move the sheets at the first touchmove, the delta has to be > to kSignificant.
Reporter

Comment 13

4 years ago
Etienne, it seems you are the right person to take this bug. Could you take a look? Thanks!
Flags: needinfo?(kgrandon) → needinfo?(etienne)
Reporter

Updated

4 years ago
Assignee: apastor → etienne
Assignee

Comment 15

4 years ago
Comment on attachment 8592737 [details] [review]
[gaia] etiennesegonzac:bug-1138956 > mozilla-b2g:master

(In reply to Alberto Pastor [:albertopq] from comment #13)
> Etienne, it seems you are the right person to take this bug. 

And you the right person to review it :)
Flags: needinfo?(etienne)
Attachment #8592737 - Flags: review?(apastor)
Reporter

Comment 16

4 years ago
Comment on attachment 8592737 [details] [review]
[gaia] etiennesegonzac:bug-1138956 > mozilla-b2g:master

LGTM! SheetsTransition.snapInPlace(); was what I was missing :)

Thanks!
Attachment #8592737 - Flags: review?(apastor) → review+
Assignee

Updated

4 years ago
Keywords: checkin-needed
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Please request Gaia v2.2 approval on this patch when you get a chance.
Flags: needinfo?(etienne)
Target Milestone: --- → 2.2 S10 (17apr)
Assignee

Comment 19

4 years ago
Comment on attachment 8592737 [details] [review]
[gaia] etiennesegonzac:bug-1138956 > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): rocketbar + edge gesture
[User impact] if declined: See Description, note: the user has to be really quick for the bug to happen it's not a super common scenario
[Testing completed]: STR from the bug and checking that we don't end up wrongly enabling edge gestures when we shouldn't (ie. on the homescreen)
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]: none
Flags: needinfo?(etienne)
Attachment #8592737 - Flags: approval-gaia-v2.2?
Attachment #8592737 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
NI myself for following verification.
Flags: needinfo?(hcheng)
This issue is verified fixed on Flame Master and 2.2.

Result: Edge gesture is not active after the user taps on the rocketbar. It is no longer accessible during the rocketbar expansion animation.

Environmental Variables:
Device: Flame 3.0 (KK, 319mb, full flash)
Build ID: 20150421010201
Gaia: a8e4f95dce9db727dda5d408b038f97fb4296557
Gecko: 7b823253d9f2
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Environmental Variables:
Device: Flame 2.2 (KK, 319mb, full flash)
Build ID: 20150421002501
Gaia: 828dd03a0e3b140d74b2e49355197df4d91d9227
Gecko: 36f72a3efb9b
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 37.0 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Flags: needinfo?(hcheng)
You need to log in before you can comment on or make changes to this bug.