843832 - [Homescreen] Tapping icons causes swipes

Status: VERIFIED FIXED

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

Description

[pgravel]

Any slight horizontal movement while trying to tap app icons on the homescreen can cause a page swipe. This can be more evident on some devices than others depending on how sensitive the touch panel is.

Tried pulling in bug 837062, but it does not solve this problem.

The core of the issue is that the logic in homescreen/js/grid.js relies on either movement or speed, instead of a combination of both. As such, even a 1 pixel horizontal movement will cause a page swipe if it happens quickly enough. 

The following small patch can be used to prevent any accidental swiping while tapping.

diff --git a/apps/homescreen/js/grid.js b/apps/homescreen/js/grid.js
index efa51bf..5b5fad5 100644
--- a/apps/homescreen/js/grid.js
+++ b/apps/homescreen/js/grid.js
@@ -221,9 +221,11 @@ const GridManager = (function() {
 
   function onTouchEnd(deltaX, evt) {
     var page = currentPage;
-    /* Bigger than panning threshold or fast gesture */
-    if (Math.abs(deltaX) > panningThreshold ||
-        touchEndTimestamp - touchStartTimestamp < kPageTransitionDuration) {
+    /* If slow movement over 50% of the screen size or
+       fast movement over 25% of the screen size, then swipe */
+    if ((Math.abs(deltaX) > 2 * panningThreshold) ||
+        (Math.abs(deltaX) > panningThreshold &&
+        touchEndTimestamp - touchStartTimestamp < kPageTransitionDuration)) {
       var forward = dirCtrl.goesForward(deltaX);
       if (forward && currentPage < pages.length - 1) {
         page = page + 1;


Distance thresholds could likely be tweaked for optimal user experience.

Comment 1

[Cristian Rodriguez (:crdlc)]

Attachment: Patch v1

If movement over 25% of the screen size or fast movement over threshold for tapping (configurable on build time), then swipe

Comment 2

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 716980 [details]
Patch v1

Working nice on unagi.

BTW, Cristian, do we need to uplift this patch to v1.0.0 and v1.0.1?

Or we can live without it?

Comment 3

[Cristian Rodriguez (:crdlc)]

If v1.0.1 can go to sensitive phones, it is a nice to have with null risk

Comment 4

[Cristian Rodriguez (:crdlc)]

https://github.com/mozilla-b2g/gaia/commit/8983833fab75a3a580bb9909b610059138a52e18

Comment 5

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 716980 [details]
Patch v1

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: 
Testing completed: 
Risk to taking this patch (and alternatives if risky):
String or UUID changes made by this patch:

Comment 6

[Francisco Jordano [:arcturus] [:francisco]]

Sorry forgot to comment:

[Approval Request Comment]
Bug caused by (feature/regressing bug #): new phones with different density like the geeksphone
User impact if declined: user won't be able to use the homescreen propertly
Testing completed: yes
Risk to taking this patch (and alternatives if risky):almost nothing, is just an adjustment in the algorithm to detect tapping or swipes
String or UUID changes made by this patch:

The risk is really low, the patch as you can see are 4 lines.

Thanks

Comment 7

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 716980 [details]
Patch v1

Marking tef+ so this can be uplifted to v1.0.1 - no approval needed.

Comment 8

[John Ford [:jhford] CET/CEST Berlin Time]

v1-train: dd294523f32061936709fa49a8578a6d968cb9f3

On the v1.0.1 branch, there is a merge conflict in "apps/homescreen/js/grid.js".  I have a couple bugs that need to be uplifted to v1.0.1, so I'll comment back if I was able to resolve this issue by uplifting those.

Comment 9

[John Ford [:jhford] CET/CEST Berlin Time]

(In reply to John Ford [:jhford] from comment #8)
> v1-train: dd294523f32061936709fa49a8578a6d968cb9f3
> 
> On the v1.0.1 branch, there is a merge conflict in
> "apps/homescreen/js/grid.js".  I have a couple bugs that need to be uplifted
> to v1.0.1, so I'll comment back if I was able to resolve this issue by
> uplifting those.

Nope, this fails to apply cleanly to v1.0.1.  There is a small merge conflict in apps/homescreen/js/grid.js.

Can you please merge this?  You might be able to do this by running:

cd gaia &&
git checkout v1.0.1
git cherry-pick -x dd294523f32061936709fa49a8578a6d968cb9f3
<RESOLVE MERGE CONFLICT>

Comment 10

[Cristian Rodriguez (:crdlc)]

Resolved

https://github.com/mozilla-b2g/gaia/commit/823ae4703347ceb585f5d2de764d9d4138a6e892

Comment 11

[Mila Davydova]

This issue no longer reproduces on Unagi device.
Build ID: 20130313070202
Kernel Date: Dec 5
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/e74dafa6b2d9
Gaia: b34e726147f8e671ad8c538b50900ccfbffcb084

A slight horizontal movement while trying to tap app icons on the homescreen does not cause a page swipe. 
Verified fixed.

Comment 12

[Mila Davydova]

This issue also appears fixed on Unagi build: 20130326070204
Kernel Date: Dec 5
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/28b048ffb7a7
Gaia: ace1eb32a313da1232bbdf9cff2581a4b036356d