1047660 - [ Soft home button / edge gestures ]In Landscape, right to left edge gesture transition should originate from the the left edge of the home button bar

Status: VERIFIED FIXED

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

Description

[Rob MacDonald [:robmac]]

The behaviour is described in the soft home button spec (https://mozilla.box.com/s/hhuk707fmq6edpxhs7hl) as well as in the App to App Edge Swipe spec (https://mozilla.box.com/s/wmsw5z3zlx4s4ari3rpi).

Comment 1

[Markus Nilsson]

Attachment: Github patch

Hi Francis.

Could you take a look at this one as well?
Thanks!

Comment 2

[Francis Djabri [:djabber]]

Re-assigning to Rob since he's back from his long weekend. Rob, let me know if you're still having trouble getting your phone to work.

Comment 3

[Rob MacDonald [:robmac]]

Comment on attachment 8467601 [details] [review]
Github patch

Hi Markus...

From the UX perspective the issue with the patch is that, if you drag your finger over the soft home key, the task switcher seems to be triggered. The soft home key should not be activated by dragging. Etienne is aware of how to resolve this issue and I've flagged him on the bug.

Thanks!
Rob

Comment 4

[Markus Nilsson]

That is strange, I just retested the patch on the device I'm using and I cannot reproduce that behaviour.
Once I start the dragging it doesn't matter if the finger passes the soft home, no home action occurs.

Do you get the same behaviour without the patch? Part of the soft home is exposed then as well, so the same thing should occur.

Comment 5

[Rob MacDonald [:robmac]]

Good question and, in fact, I am getting the same behaviour without the patch. So it's definitely a broader issue. Flagging Michael in Etienne's absence. Also, I'll be away until Aug 18 so please flag Francis in the meantime.

Comment 6

[Michael Henretty [:mikehenrty][:mhenretty]]

I think the behavior Rob is seeing is when you start the drag inside the software home button, the task switcher will be displayed instead of the edge gesture. I think we need to prevent the long press when it becomes a drag.

Comment 7

[Etienne Segonzac (:etienne)]

(In reply to Michael Henretty [:mhenretty] from comment #6)
> I think the behavior Rob is seeing is when you start the drag inside the
> software home button, the task switcher will be displayed instead of the
> edge gesture. I think we need to prevent the long press when it becomes a
> drag.

We can, but the edge gesture won't be triggered (if we're detecting a long press on the software home button it means the edge swipe detector isn't getting the events).

The core issue for this bug is not swiping from the edge of the screen and going over the home button (it's not perfect but works pretty well). It's swiping from the edge of the software home button zone, which doesn't work at all.

So I think we should:
* make the #right-panel wider in landscape when the software home button is enabled
* when we detect a tap/longtap (the code is already in place in edge_swipe_detector)
  - compare it to the software home button clientRect if it's enabled (cached)
    + send a fake tap/longtap to the software home button if the finger was on it
    + compare it to the software home button _zone_ clientRect
      > do nothing if we're inside the zone
      > forward the tap/long tap to the current app otherwise

It should fix both issues :)

Comment 8

[Michael Henretty [:mikehenrty][:mhenretty]]

Markus, based on comment 7, do you have enough information to continue your patch? Are you still able to work on this?

Comment 9

[Markus Nilsson]

(In reply to Etienne Segonzac (:etienne) from comment #7)
> (In reply to Michael Henretty [:mhenretty] from comment #6)
> > I think the behavior Rob is seeing is when you start the drag inside the
> > software home button, the task switcher will be displayed instead of the
> > edge gesture. I think we need to prevent the long press when it becomes a
> > drag.
> 
> We can, but the edge gesture won't be triggered (if we're detecting a long
> press on the software home button it means the edge swipe detector isn't
> getting the events).
> 
> The core issue for this bug is not swiping from the edge of the screen and
> going over the home button (it's not perfect but works pretty well). It's
> swiping from the edge of the software home button zone, which doesn't work
> at all.
> 
> So I think we should:
> * make the #right-panel wider in landscape when the software home button is
> enabled
> * when we detect a tap/longtap (the code is already in place in
> edge_swipe_detector)
>   - compare it to the software home button clientRect if it's enabled
> (cached)
>     + send a fake tap/longtap to the software home button if the finger was
> on it
>     + compare it to the software home button _zone_ clientRect
>       > do nothing if we're inside the zone
>       > forward the tap/long tap to the current app otherwise
> 
> It should fix both issues :)

I hope your comment was of a more general nature and not addressing my suggested patch as that should have fixed swiping from the edge of the software home button zone, which you say doesn't work at all.

So your point about making #right-panel wider should have been addressed.
I still disagree about the second point. I tested that while working on Bug 1032768 and I think my comment there is still valid:

"I tested a tap forwarding solution, but I don't think that it worked very well.
The problem was the 300ms delay that meant that you couldn't tap the home button in a normal way, you had to hold the button a short while and when I did it was a little hard to judge for how long it should be held and the "open apps selector" was sometimes triggered instead."

I think that the gain of being able to start the edge swipe from the home button is too small compared to a poorer home button user experience.

Comment 10

[Etienne Segonzac (:etienne)]

I'll make a proof of concept to figure out what I'm missing / get UX feedback.

Comment 11

[Etienne Segonzac (:etienne)]

Attachment: GAIA PR

Here's a proposal, it was indeed harder than expected. But I think it's an interesting solution.

Rob, the ui-review should focus on the landscape edge swipe, software home button press and long press (in portrait and landscape). Let me know if you have any issue flashing this version.

Markus, when edge swiping in landscape I'm always swiping over the software home button since it's centered vertically. This is why I'm pushing for this solution.
But I agree that the software home button experience needs to be rock solid.
When pressing the software home button in landscape the biggest pain point in the switch to portrait / the resize, I don't think this patch is influencing the experience.

If the feedback is positive I'll add the missing tests to this patch and start the review.

Comment 12

[Markus Nilsson]

Hi Etienne.

I think that your solution works reasonably well but the long-press needs some adjustment.
When long-pressing I think that it's too easy to unintentionally start a swipe, if you press hard the finger spreads out and starts a swipe. I find that especially true when I hold the phone with both hands and press with the thumb, as it then comes in at an angle.

I've also had it happen that no long-press was triggered, but when I let go a tap was triggered (and the home screen was shown).
It's also possible to get the soft home button stuck in the active state if you release it just after the task switcher is shown.

Comment 13

[Etienne Segonzac (:etienne)]

(In reply to Markus Nilsson from comment #12)
> Hi Etienne.
> 
> I think that your solution works reasonably well but the long-press needs
> some adjustment.
> When long-pressing I think that it's too easy to unintentionally start a
> swipe, if you press hard the finger spreads out and starts a swipe. I find
> that especially true when I hold the phone with both hands and press with
> the thumb, as it then comes in at an angle.
> 
> I've also had it happen that no long-press was triggered, but when I let go
> a tap was triggered (and the home screen was shown).
> It's also possible to get the soft home button stuck in the active state if
> you release it just after the task switcher is shown.

Thanks for the feedback Markus, patch updated!

Comment 14

[Markus Nilsson]

Comment on attachment 8472966 [details] [review]
GAIA PR

I'm sorry, but with the updated patch I can still reproduce all the things I mentioned in my previous comment.

Comment 15

[Etienne Segonzac (:etienne)]

(In reply to Markus Nilsson from comment #14)
> Comment on attachment 8472966 [details] [review]
> GAIA PR - WIP
> 
> I'm sorry, but with the updated patch I can still reproduce all the things I
> mentioned in my previous comment.

Can you provide more STR / a video ?

Comment 16

[Markus Nilsson]

Attachment: Edge issues

Attached a video that shows the problems.

Comment 17

[Etienne Segonzac (:etienne)]

(In reply to Markus Nilsson from comment #16)
> Created attachment 8475174 [details]
> Edge issues
> 
> Attached a video that shows the problems.

Thanks, very helpful!
Just updated the PR.

Comment 18

[Rob MacDonald [:robmac]]

Comment on attachment 8472966 [details] [review]
GAIA PR

Sorry for the delay as I'm just back from PTO. This patch is definitely on the right track as it seems to have resolved the earlier issues identified by Markus on my build at least. 

That said, I'm still minus'ing it because the home button is still triggered in what I'll refer to as a "peek" scenario. To replicate

1) Launch two landscape apps. 
2) Edge swipe to return to the first app.
3) In landscape, swipe from the right edge, over the home button, to peek at the second app. Do not release your finger.
4) Now drag your finger back over the home button and off the edge of the screen.

In this instance, the home button is triggered and the user is returned to the grid. The desired behaviour is that the home button not be triggered and that the user remain the current sheet. 

Please NI me if this doesn't make sense. 

Thanks!
Rob

Comment 19

[Rob MacDonald [:robmac]]

Also, NI'ing Hung for visual feedback.

Comment 20

[Markus Nilsson]

The updated PR solves the problem with the thumb starting an unintended swipe.

I was able to reproduce the problem that Rob describes and I think that I narrowed down the problem:

1) Start swipe from the home button
2) Return to the same position and release

--> Tap/click is triggered

The problem that the soft home sometimes get stuck in active state after long-press remains.

Comment 21

[Etienne Segonzac (:etienne)]

Comment on attachment 8472966 [details] [review]
GAIA PR

Thanks Rob, there was actually a bug happening in portrait too!

But I can't reproduce the issue where the software home button stays highlighted :/ Markus, can you have another look?

Comment 22

[Markus Nilsson]

In the handleRedispatchedTouch function, adding touchend seems to fix the active/highlighted problem:

...
if (!this._onButton(evt)) {
  if (type === 'touchmove' || type === 'touchend') {
...

Comment 23

[Etienne Segonzac (:etienne)]

(In reply to Markus Nilsson from comment #22)
> In the handleRedispatchedTouch function, adding touchend seems to fix the
> active/highlighted problem:
> 
> ...
> if (!this._onButton(evt)) {
>   if (type === 'touchmove' || type === 'touchend') {
> ...

Makes perfect sense, I'll add this fix and start working on the tests today.

Comment 24

[Etienne Segonzac (:etienne)]

Comment on attachment 8472966 [details] [review]
GAIA PR

Added the last fix and a good amount of unit-tests :)
Starting the review process.

Comment 25

[Rob MacDonald [:robmac]]

Comment on attachment 8472966 [details] [review]
GAIA PR

Appears to have resolved the earlier issue with comment 18. Thanks!

Comment 26

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

Comment on attachment 8472966 [details] [review]
GAIA PR

r+ with nits.

As I said on github I don't really like the redispatch mechanism, but after having discussed with you IRL, I don't have anything better to suggest atm.

So let's go with it.

Comment 27

[Markus Nilsson]

Comment on attachment 8472966 [details] [review]
GAIA PR

Looks and works pretty good. If anything, add code comments describing the redispatching.

Comment 28

[Etienne Segonzac (:etienne)]

Thanks everyone, pushed the followups and added comments. Waiting for try to run before merging.

Clearing the visual feedback ni? since we're not changing any visual.

Comment 29

[Etienne Segonzac (:etienne)]

https://github.com/mozilla-b2g/gaia/commit/2724945f7d474ebe0f3ecb29817ddd65f773fdf9

Comment 30

[Adam Alldredge [:AdamA][Inactive]]

The issue is verified fixed in Flame 2.1 and Flame 2.2:

Flame 2.1 KitKat Base (319mb)(Full Flash)

Environmental Variables:
Device: Flame 2.1
BuildID: 20141007000203
Gaia: 7f738edf66b9298bceef8a4981d05d04fd04e540
Gecko: b9d04c58580a
Gonk: 2c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Flame 2.2 KitKat Base (319mb)(Full Flash)

Environmental Variables:
Device: Flame 2.2 Master
BuildID: 20141007040205
Gaia: 83de447d9ae9a59459d7a445f9348a254c661850
Gecko: 9ee9e193fc48
Gonk: 2c909e821d107d414f851e267dedcd7aae2cebf
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

The edge gesture is not being triggered on the SHB portion of the screen.