957188 - [APZC] The click events are lost in some situations

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Julien Wajsberg [:julienw]]

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.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 2

[Dale Harvey (:daleharvey)]

We started generating click events with unhandled long presses, https://bugzilla.mozilla.org/show_bug.cgi?id=942929

Comment 3

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #1)
> 500ms and longer is a "long press". It's doesn't trigger a click by design.

So... this is resolved/invalid?

Comment 4

[Dale Harvey (:daleharvey)]

(In reply to Milan Sreckovic [:milan] from comment #3)
> (In reply to Kartikaya Gupta (email:[e-mail]) 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

Comment 5

[Kartikaya Gupta (email:[e-mail])]

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?

Comment 6

[Dale Harvey (:daleharvey)]

will do

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Kartikaya Gupta (email:[e-mail]) 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.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

(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

Comment 9

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Dale Harvey (:daleharvey) from comment #4)
> (In reply to Milan Sreckovic [:milan] from comment #3)
> > (In reply to Kartikaya Gupta (email:[e-mail]) 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?

Comment 10

[Kartikaya Gupta (email:[e-mail])]

(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.

Comment 11

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 12

[Milan Sreckovic [:milan] (needinfo for best results)]

Where did 4.5 comes from in the first place? Empirical, or a manufacturer told us?

Comment 13

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 14

[Milan Sreckovic [:milan] (needinfo for best results)]

Nice!

Comment 15

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch

This should put the threshold at 0.5 inch on both metro and B2G which should be Good Enough For Anybody (TM).

Comment 16

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4210f0c10679

Comment 17

[Julien Wajsberg [:julienw]]

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").

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4210f0c10679

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8ebbca7b5855