Closed
Bug 957188
Opened 11 years ago
Closed 11 years ago
[APZC] The click events are lost in some situations
Categories
(Core :: Panning and Zooming, defect)
Tracking
()
People
(Reporter: julienw, Assigned: kats)
References
Details
Attachments
(1 file)
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)
Assignee | ||
Comment 1•11 years ago
|
||
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•11 years ago
|
||
We started generating click events with unhandled long presses, https://bugzilla.mozilla.org/show_bug.cgi?id=942929
Comment 3•11 years ago
|
||
(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: 11 years ago
Flags: needinfo?(milan)
Resolution: --- → FIXED
Comment 4•11 years ago
|
||
(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 → ---
Assignee | ||
Comment 5•11 years ago
|
||
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
Comment 6•11 years ago
|
||
will do
Comment 7•11 years ago
|
||
(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.
Assignee | ||
Comment 8•11 years ago
|
||
(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•11 years ago
|
||
(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?
Assignee | ||
Comment 10•11 years ago
|
||
(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.
Assignee | ||
Comment 11•11 years ago
|
||
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
Comment 12•11 years ago
|
||
Where did 4.5 comes from in the first place? Empirical, or a manufacturer told us?
Assignee | ||
Comment 13•11 years ago
|
||
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•11 years ago
|
||
Nice!
Assignee | ||
Comment 15•11 years ago
|
||
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)
Updated•11 years ago
|
Attachment #8357929 -
Flags: review?(botond) → review+
Assignee | ||
Comment 16•11 years ago
|
||
Reporter | ||
Comment 17•11 years ago
|
||
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•11 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Assignee | ||
Updated•11 years ago
|
status-b2g-v1.2:
--- → wontfix
status-b2g-v1.3:
--- → affected
status-b2g-v1.4:
--- → fixed
status-firefox27:
--- → wontfix
status-firefox28:
--- → affected
status-firefox29:
--- → fixed
Updated•11 years ago
|
blocking-b2g: 1.3? → 1.3+
Comment 19•11 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•