Closed
Bug 831656
Opened 12 years ago
Closed 12 years ago
Homescreen tap/swipe breaks on non-target devices
Categories
(Firefox OS Graveyard :: Gaia::Homescreen, defect)
Firefox OS Graveyard
Gaia::Homescreen
Tracking
(blocking-b2g:-, b2g18+ fixed, b2g18-v1.0.1 fixed)
RESOLVED
FIXED
blocking-b2g | - |
People
(Reporter: timdream, Assigned: crdlc)
References
Details
Attachments
(1 file)
(deleted),
text/html
|
arcturus
:
review+
rexboy
:
feedback+
arcturus
:
approval-gaia-v1+
|
Details |
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.
Assignee | ||
Comment 1•12 years ago
|
||
Yesterday, I started to take a look to this problem, could I assign me? thanks
Reporter | ||
Comment 2•12 years ago
|
||
(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?
Assignee | ||
Comment 3•12 years ago
|
||
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 :)
Reporter | ||
Comment 4•12 years ago
|
||
(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
Assignee | ||
Comment 6•12 years ago
|
||
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?
Comment 7•12 years ago
|
||
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.
Assignee | ||
Comment 8•12 years ago
|
||
Thanks for your feedback Lee, in a couple of hours I am going to provide a patch. Thanks a lot
Assignee | ||
Comment 9•12 years ago
|
||
* 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)
Comment 10•12 years ago
|
||
Can the threshold |thresholdForTapping| depend on the icon size?
I had just filed bug 832194, seems the same symptom here.
Assignee | ||
Comment 11•12 years ago
|
||
With is patch I cannot reproduce that behavior, you can test it if you want Benjamin, thanks
Assignee | ||
Comment 12•12 years ago
|
||
sorry "With is patch" = "With this patch" :)
Assignee | ||
Comment 13•12 years ago
|
||
comments implemented, thanks for you feedback Fran!!
Comment 14•12 years ago
|
||
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+
Comment 15•12 years ago
|
||
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.
Updated•12 years ago
|
blocking-b2g: tef? → -
tracking-b2g18:
--- → +
Reporter | ||
Comment 17•12 years ago
|
||
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)
Comment 18•12 years ago
|
||
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.
Comment 19•12 years ago
|
||
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 20•12 years ago
|
||
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+
Assignee | ||
Comment 21•12 years ago
|
||
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)
Assignee | ||
Updated•12 years ago
|
Attachment #703805 -
Flags: approval-gaia-v1?(21) → approval-gaia-v1?(francisco.jordano)
Comment 22•12 years ago
|
||
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+
Assignee | ||
Comment 23•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
status-b2g18:
--- → fixed
Comment 26•12 years ago
|
||
Batch edit: bugs fixed on b2g18 since 1/25 branch of v1.0 are fixed on v1.0.1
status-b2g18-v1.0.1:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•