Closed
Bug 1128672
Opened 10 years ago
Closed 10 years ago
The gaia edge swipe detector eats clicks
Categories
(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)
Tracking
(blocking-b2g:2.2+, b2g-v2.2 verified, b2g-master verified)
People
(Reporter: kats, Assigned: etienne)
References
Details
(Whiteboard: [input-thread-uplift-part4])
Attachments
(3 files, 1 obsolete file)
(deleted),
text/x-github-pull-request
|
alive
:
review+
bajaj
:
approval-gaia-v2.2+
|
Details |
(deleted),
video/3gpp
|
Details | |
(deleted),
video/3gpp
|
Details |
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.
Reporter | ||
Comment 1•10 years ago
|
||
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)
Reporter | ||
Comment 2•10 years ago
|
||
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 3•10 years ago
|
||
Reporter | ||
Comment 4•10 years ago
|
||
Comment on attachment 8558144 [details]
[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•10 years ago
|
||
Comment on attachment 8558144 [details]
[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)
Comment 6•10 years ago
|
||
Assignee | ||
Comment 7•10 years ago
|
||
Comment on attachment 8558431 [details]
[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•10 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #4)
> Comment on attachment 8558144 [details]
> [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 ? :)
Updated•10 years ago
|
Attachment #8558431 -
Flags: review?(alive) → review+
Reporter | ||
Comment 9•10 years ago
|
||
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•10 years ago
|
Keywords: checkin-needed
Comment 10•10 years ago
|
||
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.
Updated•10 years ago
|
Keywords: checkin-needed
Comment 11•10 years ago
|
||
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•10 years ago
|
Component: Panning and Zooming → Gaia::System::Window Mgmt
Product: Core → Firefox OS
Version: Trunk → unspecified
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Updated•10 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 12•10 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.
Blocks: parent-process-apz
Comment 13•10 years ago
|
||
Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/0f953e0139339b5cf0c726227879bfaa670eb8be
Updated•10 years ago
|
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 14•10 years ago
|
||
Thanks!
Assignee: bugmail.mozilla → etienne
status-b2g-master:
--- → fixed
noming for 2.2 based on comments.
blocking-b2g: --- → 2.2?
Updated•10 years ago
|
blocking-b2g: 2.2? → 2.2+
Comment 16•10 years ago
|
||
(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)
Reporter | ||
Comment 17•10 years ago
|
||
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•10 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)
Updated•10 years ago
|
status-b2g-v2.2:
--- → affected
Target Milestone: --- → 2.2 S5 (6feb)
Reporter | ||
Updated•10 years ago
|
Whiteboard: [NO_UPLIFT][input-thread-uplift-part4]
Reporter | ||
Comment 19•10 years ago
|
||
Comment on attachment 8558431 [details]
[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?
Updated•10 years ago
|
Attachment #8558431 -
Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
Reporter | ||
Comment 20•10 years ago
|
||
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]
Comment 21•10 years ago
|
||
Keywords: checkin-needed
Depends on: 1134035
Comment 22•9 years ago
|
||
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
Comment 23•9 years ago
|
||
Comment 24•9 years ago
|
||
Updated•9 years ago
|
QA Whiteboard: [MGSEI-Triage+]
You need to log in
before you can comment on or make changes to this bug.
Description
•