Closed Bug 831656 Opened 11 years ago Closed 11 years ago

Homescreen tap/swipe breaks on non-target devices

Categories

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

defect
Not set
normal

Tracking

(blocking-b2g:-, b2g18+ fixed, b2g18-v1.0.1 fixed)

RESOLVED FIXED
blocking-b2g -
Tracking Status
b2g18 + fixed
b2g18-v1.0.1 --- fixed

People

(Reporter: timdream, Assigned: crdlc)

References

Details

Attachments

(1 file)

Bug 796242 landed a change, which, doesn't take screen resolution into account on the distance threshold for distinguishing tap or swipe, making icons hard to tap on non-otoro/unagi devices. Tapping an icon almost always trigger a swipe (and cancels the tap).

Rex also suspect this is a result of different sensitively of the mouse/touch events, he will look into this for us on that first.

Obviously this is not a v1.0 bug.
Yesterday, I started to take a look to this problem, could I assign me? thanks
(In reply to crdlc from comment #1)
> Yesterday, I started to take a look to this problem, could I assign me?
> thanks

Do you have a device other than otoro/unagi? What is the spec?
Yes, I have. Yesterday I was in Madrid and Daniel explained me the problem and I started working on it. In general, I detected this problem with all mobiles except Unagi/otoro :)
(In reply to crdlc from comment #3)
> Yes, I have. Yesterday I was in Madrid and Daniel explained me the problem
> and I started working on it. In general, I detected this problem with all
> mobiles except Unagi/otoro :)

Cool :)
Assignee: rexboy → crdlc
thanks a lot Tim
Status: NEW → ASSIGNED
I was talking to Daniel Coloma and I guess that it should be implemented yes or yes for v1, the behavior is horrible (unusable) for non-otoro/unagi devices
blocking-b2g: --- → tef?
It's good to hear you're working at this, Crdlc.  Let me do a little explain on what I found before here for reference: After tapping an icon, if any mousemove event is triggered between mousedown and mouseup, the target of mouseup would no longer keep on the icon and thus it's wrongly recognized as tapping 'through' the icon.  As a result even 1 pixel mousemove becomes swiping.  On unagi this doesn't become a problem because of hardware limitation, which needs a relatively longer move to make mousemove triggered.
Thanks for your feedback Lee, in a couple of hours I am going to provide a patch. Thanks a lot
Attached file Patch v1
* Current implementation:

** If movement is 0 implies tap or tap&hold depending on press time

** If movement is different than 0 implies panning and never tap/tap&hold

Problem: this approach works fine on Unagi/Otoro devices but for the rest of devices with more density of pixels or more touch sensibility doesn't work. When an user touches the screen the movement is between 0-10 pixels and we should consider that how a tap or tap&hold depending on the press time.

* The patch:

** Requirement: the panning algorithm should be performed from the minute 0, not waiting until that the movement is bigger than 10px (tap threshold).

** If movement is between [0-10] implies tap or tap&hold depending on press time but maybe the grid is moved (panning effect) so we should recover the position

** If movement is bigger than 10px implies panning and never tap/tap&hold

--------

In order to implement this new behavior, I've had to migrate mouse events to touch events. It is due to that we should cancel the panning when we consider that a gesture has been a tap&hold event (edit mode). During the panning the library called mouse shim captures all mouse events and the drag&drop library doesn't work because it doesn't receive mousemove events without a terrible workaround. Mouse shim doesn't release mouse events until next mouseup event.

The solution is obvious: mouse events migration. With this approach, we don't need the mouse shim library which penalizes due to more allocations while panning and for each touchmove an elementFromPoint method is performed in the drag&drop (edit-mode). Note: the library is awesome and useful, thanks Flanagan but from now on it is not needed in Homescreen (if the patch is landed :) )

Furthermore, IMHO, we should migrate from mouse events to touch events because it is the natural way of implementing this.

--------

Please, Jordano, there is a constant that defines the drag threshold, currently it is 10, if you realize that we need more, please tell me.

Thanks a lot for the review
Attachment #703805 - Flags: review?(francisco.jordano)
Blocks: 832194
Can the threshold |thresholdForTapping| depend on the icon size?
I had just filed bug 832194, seems the same symptom here.
With is patch I cannot reproduce that behavior, you can test it if you want Benjamin, thanks
sorry "With is patch" = "With this patch" :)
comments implemented, thanks for you feedback Fran!!
Comment on attachment 703805 [details]
Patch v1

r=me

Good change, and thanks for adding the comments.

I've been trying in otoro/unagi and another phone and works pretty good in all the cases.

Thanks for great job Cristian!
Attachment #703805 - Flags: review?(francisco.jordano) → review+
We do not have access yet to the final Hardware(s), so I think this is critical to be on the safe side and ensure this works in any screen regardless pixel density or touch sensibility.
blocking-b2g: tef? → -
tracking-b2g18: --- → +
No longer blocks: 832194
Comment on attachment 703805 [details]
Patch v1

Rex, would you verify that this patch does fix the problem on our non-unagi/otoro device? If not there might be other issues with the device. Thanks.
Attachment #703805 - Flags: feedback?(rexboy)
I've tested the patch on two devices.  One of them works well.  But another one is still panning if I tap the icon quickly.  It's annoying, but holding finger on an icon for a short while can usually fix it.  Drag threshold of 10 pixel works reasonable for both devices even though their DPI is 1.5x higher than unagi.  But I agree it's a good idea if it can be adjusted dynamically.

I think we can further do some investigate on the threshold of "quickly touch" introduced by kPageTransitionDuration (which is 300ms) to eliminate the problem I mentioned above.  The appropriate threshold may be hardware-dependent so that not all devices suffer this problem.
Hi Rex!

As you say the threshold should be customisable per device, that's something that Cristian already took into account.

There was a discussion in github, during the review where we comment about making that threshold configurable per device during build time.

There is another bug that already adds some configuration to the homescreen, bug 829050, so we decided that is better to land this one, and once the bug 829050 lands, add there this threshold as parameter.

Hope you like the idea ;)

Cheers!
Comment on attachment 703805 [details]
Patch v1

Oops.  Sorry I didn't change the flag.  I'm OK with this patch landed and we may fire another bug tracking the time threshold problem.
Thanks for the great works!
Attachment #703805 - Flags: feedback?(rexboy) → feedback+
Comment on attachment 703805 [details]
Patch v1

NOTE: If blocking-basecamp+ is set, just land it for now.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: 
Testing completed: 
Risk to taking this patch (and alternatives if risky):
Attachment #703805 - Flags: approval-gaia-v1?(21)
Attachment #703805 - Flags: approval-gaia-v1?(21) → approval-gaia-v1?(francisco.jordano)
Comment on attachment 703805 [details]
Patch v1

We need this in order to support the developer previews

a=me
Attachment #703805 - Flags: approval-gaia-v1?(francisco.jordano) → approval-gaia-v1+
https://github.com/mozilla-b2g/gaia/commit/7a505331d9fbdd277e5d1c59a9f230200d494c93
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Depends on: 836893
Batch edit: bugs fixed on b2g18 since 1/25 branch of v1.0 are fixed on v1.0.1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: