[Homescreen] Tapping icons causes swipes

VERIFIED FIXED

Status

Firefox OS
Gaia::Homescreen
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: pgravel, Assigned: crdlc)

Tracking

unspecified
x86
Linux

Firefox Tracking Flags

(blocking-b2g:tef+, b2g18 verified, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 verified)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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.
(Assignee)

Updated

5 years ago
Assignee: nobody → crdlc
Status: NEW → ASSIGNED
(Assignee)

Comment 1

5 years ago
Created attachment 716980 [details]
Patch v1

If movement over 25% of the screen size or fast movement over threshold for tapping (configurable on build time), then swipe
Attachment #716980 - Flags: review?(francisco.jordano)
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?
Attachment #716980 - Flags: review?(francisco.jordano) → review+
(Assignee)

Comment 3

5 years ago
If v1.0.1 can go to sensitive phones, it is a nice to have with null risk
(Assignee)

Comment 4

5 years ago
https://github.com/mozilla-b2g/gaia/commit/8983833fab75a3a580bb9909b610059138a52e18
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
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:
Attachment #716980 - Flags: approval-gaia-v1?
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
blocking-b2g: --- → tef+
status-b2g18: --- → affected
status-b2g18-v1.0.0: --- → wontfix
status-b2g18-v1.0.1: --- → affected
Comment on attachment 716980 [details]
Patch v1

Marking tef+ so this can be uplifted to v1.0.1 - no approval needed.
Attachment #716980 - Flags: approval-gaia-v1?
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.
status-b2g18: affected → fixed
(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>
Flags: needinfo?(crdlc)
status-b2g18-v1.0.1: affected → fixed

Comment 11

5 years ago
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.
Status: RESOLVED → VERIFIED
status-b2g18-v1.0.1: fixed → verified

Comment 12

5 years ago
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
status-b2g18: fixed → verified
You need to log in before you can comment on or make changes to this bug.