The gaia edge swipe detector eats clicks

VERIFIED FIXED in 2.2 S5 (6feb)

Status

defect
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: kats, Assigned: etienne)

Tracking

unspecified
2.2 S5 (6feb)
All
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.2 verified, b2g-master verified)

Details

(Whiteboard: [input-thread-uplift-part4])

Attachments

(3 attachments, 1 obsolete attachment)

STR:

- In the B2G settings app, go to the developer settings
- Click on the rightmost edge of a checkbox (e.g. "Flash repainted area").

Expected:

- checkbox flips state

Actual:

- it doesn't.

Note that if you click on the "Flash repainted area" text itself it works, it's just the rightmost 30 pixels of the screen that are nonresponsive. The reason this happens is because the edge swipe areas (which you can see if actually enable paint flashing) eat the touch events as well as the mouse events that get generated by the tap. Then they inject new touch events to the content process, but no corresponding mouse events.

This probably didn't happen prior to bug 920036 because the gesture detection happened after going through the edge swipe detector. We will probably need to fiddle with injectTouchEvent to make this work.
etienne, do you have any thoughts on if it's easy/possible to redo the edge swipe detector to not rely on injectTouchEvent? That would be preferable to me if possible.
Flags: needinfo?(etienne)
A few other things I want to note:

1) There is code at [1] that can be removed now, I think with no negative (or positive) impact.

2) The edge swipe detector panels [2] are styled to be opacity:0 but they still participate in hit testing and event dispatch (obviously, otherwise they wouldn't be able to listen for touch events). However this means that even with the mouseevent preventDefault removed, the mouse events generated from a click in the root process don't end up going to the child process, because they hit the detector panels instead. If the detector panels are going to consume the mouse events like this, then they must be also be responsible for redispatching the mouse events.

[1] https://github.com/mozilla-b2g/gaia/blob/ae5a1580da948c3b9f93528146b007fc4f6a712b/apps/system/js/edge_swipe_detector.js#L99-L105
[2] https://github.com/mozilla-b2g/gaia/blob/ae5a1580da948c3b9f93528146b007fc4f6a712b/apps/system/index.html#L1219-L1220
Comment on attachment 8558144 [details] [review]
[PullReq] staktrace:forward_clicks to mozilla-b2g:master

While I would still like to get rid of the injectTouchEvents thing entirely, this seems like a low-risk fix for this bug (i.e. upliftable to 2.2). It's not perfect because the mouse events will get delivered to the child process before the corresponding touch events but that's a relatively minor issue I think.
Attachment #8558144 - Flags: review?(etienne)
(Assignee)

Comment 5

4 years ago
Comment on attachment 8558144 [details] [review]
[PullReq] staktrace:forward_clicks to mozilla-b2g:master

Sounds about right.
I'm going to go ahead and just revert part of bug 1072498 this way I don't have to bug you about tests ;)
Attachment #8558144 - Attachment is obsolete: true
Flags: needinfo?(etienne)
Attachment #8558144 - Flags: review?(etienne)
(Assignee)

Comment 7

4 years ago
Comment on attachment 8558431 [details] [review]
[PullReq] etiennesegonzac:bug-1128672 to mozilla-b2g:master

Just bringing back some code removed in bug 1072498.
Attachment #8558431 - Flags: review?(alive)
(Assignee)

Comment 8

4 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #4)
> Comment on attachment 8558144 [details] [review]
> [PullReq] staktrace:forward_clicks to mozilla-b2g:master
> 
> While I would still like to get rid of the injectTouchEvents thing entirely,
> this seems like a low-risk fix for this bug (i.e. upliftable to 2.2). 

Are we uplifting APZC in the main process to 2.2 ? :)
Attachment #8558431 - Flags: review?(alive) → review+
Hah, i forgot I removed that code! :) thanks for the patch! And yes, we are planning to uplift parent process APZ to 2.2.
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.
Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.
(Assignee)

Updated

4 years ago
Component: Panning and Zooming → Gaia::System::Window Mgmt
Product: Core → Firefox OS
Version: Trunk → unspecified
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
(Assignee)

Comment 12

4 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #9)
> Hah, i forgot I removed that code! :) thanks for the patch! And yes, we are
> planning to uplift parent process APZ to 2.2.

Cool! Marking this one as blocking so it gets picked up in the uplift.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Thanks!
Assignee: bugmail.mozilla → etienne
noming for 2.2 based on comments.
blocking-b2g: --- → 2.2?
blocking-b2g: 2.2? → 2.2+
(In reply to Etienne Segonzac (:etienne) from comment #12)
> (In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #9)
> > Hah, i forgot I removed that code! :) thanks for the patch! And yes, we are
> > planning to uplift parent process APZ to 2.2.
> 
> Cool! Marking this one as blocking so it gets picked up in the uplift.

You still have to ask for uplift approval.
Flags: needinfo?(etienne)
I'm not sure if this is safe to uplift by itself (i.e. without the parent process APZ work). If not I can request approval for it when we uplift everything else, since there's a large set of bugs that will need uplifting together.
(Assignee)

Comment 18

4 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #17)
> I'm not sure if this is safe to uplift by itself (i.e. without the parent
> process APZ work). If not I can request approval for it when we uplift
> everything else, since there's a large set of bugs that will need uplifting
> together.

Sounds good.
Flags: needinfo?(etienne)
Target Milestone: --- → 2.2 S5 (6feb)
Blocks: 1131695
Whiteboard: [NO_UPLIFT][input-thread-uplift-part4]
Comment on attachment 8558431 [details] [review]
[PullReq] etiennesegonzac:bug-1128672 to mozilla-b2g:master

[Approval Request Comment]
Bug caused by (feature/regressing bug #): required for parent-process APZ (bug 950934) and input-thread (bug 930939) which are 2.2+ features. I'm tracking the full set of bugs that will need uplifting together at http://mzl.la/1zvKgmk; requesting approval on bugs individually but will wait until everything has approval before I uplift any of it.
User impact if declined: can't have parent-process APZ or input-thread
Testing completed: on master. some of these bugs have been baking for a while; others are more recent.
Risk to taking this patch (and alternatives if risky): there's likely some edge cases that we haven't run into yet and will be uncovered with further testing. Bug 1128887 is the only known issue that we don't yet have a fix for but I consider that relatively minor and something we can fix after uplifting everything else.
String or UUID changes made by this patch: none
Attachment #8558431 - Flags: approval-gaia-v2.2?
Attachment #8558431 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
checkin-needed for uplifting this to 2.2. I just uplifted all the gecko-side patches that go with this, and it would be nice to get this into the same build as those.
Keywords: checkin-needed
Whiteboard: [NO_UPLIFT][input-thread-uplift-part4] → [input-thread-uplift-part4]
Depends on: 1134035
This bug has been verified as "pass" on latest nightly build of Flame v2.2&master and Nexus5 v2.2&master by the STR in comment 0.

Actual results: Clicking on the rightmost edge of a checkbox (ex: "Flash repainted area"), the checkbox flips state.
See attachment: verified_latest_Flame_v2.2.3gp and verified_latest_Flame_master.3gp 
Reproduce rate: 0/10


-----------------------------------------------------------------------------
Note:
This bug had been fixed on Flame master and Nexus5 master in the past, just as mentioned in comment 13, but later it has changed the design (after changed, it has no response when clicking on the rightmost edge of a checkbox). I don't know which bug modified that.

-----------------------------------------------------------------------------
Device: Flame v2.2 (Verified) 
Build ID               20150720002503
Gaia Revision          e1e6317f17a840b19af9dbb25f5a771d8d9fa161
Gaia Date              2015-07-15 21:05:11
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/e2d1f1f55803
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150720.042247
Firmware Date          Mon Jul 20 04:22:58 EDT 2015
Bootloader             L1TC000118D0

Device: Flame master (Verified)
Build ID               20150720010206
Gaia Revision          3fac3ed7b8c887351098ffc677769ddc36abb3d0
Gaia Date              2015-07-17 17:53:41
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/202e9233d130
Gecko Version          42.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150720.044158
Firmware Date          Mon Jul 20 04:42:10 EDT 2015
Bootloader             L1TC000118D0

Device: Nexus5 v2.2 (Verified) 
Build ID               20150720002503
Gaia Revision          e1e6317f17a840b19af9dbb25f5a771d8d9fa161
Gaia Date              2015-07-15 21:05:11
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/e2d1f1f55803
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150720.041924
Firmware Date          Mon Jul 20 04:19:43 EDT 2015
Bootloader             HHZ12f

Device: Nexus5 master (Verified)
Build ID               20150720160206
Gaia Revision          4fe0507781f3ed56c8ae5e66dd9489165d1ff68e
Gaia Date              2015-07-20 16:09:12
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/d00e4167b482
Gecko Version          42.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150720.191854
Firmware Date          Mon Jul 20 19:19:13 EDT 2015
Bootloader             HHZ12f
Status: RESOLVED → VERIFIED
QA Whiteboard: [MGSEI-Triage+]
You need to log in before you can comment on or make changes to this bug.