Closed Bug 957188 Opened 10 years ago Closed 10 years ago

[APZC] The click events are lost in some situations

Categories

(Core :: Panning and Zooming, defect)

Product:
Core
Component:
Panning and Zooming
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla29
Project Flags:
blocking-b2g 1.3+
Tracking Flags:
Tracking Status
firefox27 --- wontfix
firefox28 --- fixed
firefox29 --- fixed
b2g-v1.2 --- wontfix
b2g-v1.3 --- fixed
b2g-v1.4 --- fixed

People

(Reporter: julienw, Assigned: kats)

References

Details

Attachments

(1 file)

Patch
1017 bytes, patch
botond
: review+
Details | Diff | Splinter Review 
Hi,

the following STR fails about 80% of the time with APZC turned on. It never fails with APZC turned off.

1. check that APZC is turned on (should be the default on current builds)
2. launch the Messages app
3. press the "new message" button with the thumb well pressed, and keeping it pressed for about 500ms/1 sec

=> The click is not registered.

It does not happen on this button only but everywhere, this button is just the easiest and fastest way to make it happen.

Also, it does not happen if you press quickly, it happens only if you press not so quickly (but 200ms triggers it for me as well).

Also it happens if you press quite strongly, so that the touch surface is bigger. I think the issue is really related to this. Could be a wrong association with event fluffing.

NI Milan because this happens only when APZC is enabled.
1.3? because APZC is enabled on 1.3.

Note that this happens to me all the time, I assume different people are pressing buttons differently and therefore this can be more or less visible depending on the user using the device.
Flags: needinfo?(milan)
500ms and longer is a "long press". It's doesn't trigger a click by design. 200ms should still trigger a click. I don't believe we use the touch radius anywhere so I don't think pressing "strongly" would make a difference.
We started generating click events with unhandled long presses, https://bugzilla.mozilla.org/show_bug.cgi?id=942929
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #1)
> 500ms and longer is a "long press". It's doesn't trigger a click by design.

So... this is resolved/invalid?
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: needinfo?(milan)
Resolution: --- → FIXED
(In reply to Milan Sreckovic [:milan] from comment #3)
> (In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #1)
> > 500ms and longer is a "long press". It's doesn't trigger a click by design.
> 
> So... this is resolved/invalid?

nope

(Dale Harvey (:daleharvey) from comment #2)
> We started generating click events with unhandled long presses,
> https://bugzilla.mozilla.org/show_bug.cgi?id=942929
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Dale, mind taking a look? You added tests for this so I think it's less likely to be a regression in the APZC code. It might be related to bug 942246?
Assignee: nobody → dale
will do
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #5)
> Dale, mind taking a look? You added tests for this so I think it's less
> likely to be a regression in the APZC code. It might be related to bug
> 942246?

It does not happen without APZC so I may not only be a fluffling issue.
(In reply to Julien Wajsberg [:julienw] from comment #0)
> Also it happens if you press quite strongly, so that the touch surface is
> bigger. I think the issue is really related to this. Could be a wrong
> association with event fluffing.

Actually the one thing that might match the symptoms you are seeing is that when you press "strongly" the hardware is more likely to send move events. If our distance threshold is too small then we will interpret it as a pan rather than a click. You can try increasing the apz.touch_start_tolerance pref (or the hard-coded default value for gTouchStartTolerance in AsyncPanZoomController.cpp) to see if that fixes it. If so it's just a simple matter of setting that pref on b2g. The APZ threshold [1] for this value is definitely different (and I think smaller) than the non-APZ threshold [2] for this value.

[1] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/ipc/AsyncPanZoomController.cpp?rev=031f4999c5fe#136
[2] http://mxr.mozilla.org/mozilla-central/source/dom/ipc/TabChild.cpp?rev=031f4999c5fe#1757
(In reply to Dale Harvey (:daleharvey) from comment #4)
> (In reply to Milan Sreckovic [:milan] from comment #3)
> > (In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #1)
> > > 500ms and longer is a "long press". It's doesn't trigger a click by design.
> > 
> > So... this is resolved/invalid?
> 
> nope
> 
> (Dale Harvey (:daleharvey) from comment #2)
> > We started generating click events with unhandled long presses,
> > https://bugzilla.mozilla.org/show_bug.cgi?id=942929

If this is not resolved invalid, then bug 942929 caused this regression?  Why aren't we backing that out?
(In reply to Milan Sreckovic [:milan] from comment #9)
> If this is not resolved invalid, then bug 942929 caused this regression? 
> Why aren't we backing that out?

No; this bug is claiming that bug 942929 was not properly fixed. This is basically the same bug as that - either something regressed 942929 and should be backed out, or (more likely, IMO) we need to twiddle a pref as in comment 8.
I added some logging and sure enough on the Hamachi the hardware is sending us touch points that randomly jump by 7 pixels. For example when I do a "heavy" tap on the new message icon, I see the touch start and a bunch of touchmove events come in at 235,60 and then partway through the series of touchmove events, they jump to 238,67 and then I get the touchend.

The non-APZC code path treats it as a tap because it allows a variance of up to 25 pixels in either axis. The APZC code path doesn't think this is a tap because the touchmove strays more than the 4.5 pixel threshold that is used by default (on Hamachi; it's dependent on DPI).

I think the Hamachi hardware is just bad here, and if we could increase the threshold on just that device then I would do so. But I don't think we have per-device prefs so I'll bump up the pref on b2g in general to cover this.
Assignee: dale → bugmail.mozilla
Where did 4.5 comes from in the first place? Empirical, or a manufacturer told us?
Looks like it was a mistake, actually. In https://hg.mozilla.org/mozilla-central/rev/934749cb1749 it was changed from dpi/2 to dpi/16. There's no discussion in the bug as to why this was changed. In my local testing changing it back to dpi/2 fixes most instances of this bug so I'll go ahead and do that.
Attached patch Patch — — Splinter Review 
This should put the threshold at 0.5 inch on both metro and B2G which should be Good Enough For Anybody (TM).
Attachment #8357929 - Flags: review?(botond)
Attachment #8357929 - Flags: review?(botond) → review+
Thanks for the quick fix, I'll be sure to verify once it lands in central.

Just a precision from my comment 0:

> Also, it does not happen if you press quickly, it happens only if you press not so quickly
> (but 200ms triggers it for me as well).

I meant: 200ms triggers the bug for me, (and not: "200ms triggers the click").
https://hg.mozilla.org/mozilla-central/rev/4210f0c10679
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
blocking-b2g: 1.3? → 1.3+
You need to log in before you can comment on or make changes to this bug.