Closed
Bug 1134642
Opened 10 years ago
Closed 10 years ago
Unable to tap "confirm" or "cancel" on some settings dialog: APN deletion, data roaming activation
Categories
(Firefox OS Graveyard :: Gaia::Shared, defect)
Tracking
(b2g-master verified)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
b2g-master | --- | verified |
People
(Reporter: gerard-majax, Assigned: gerard-majax)
References
Details
(Keywords: regression, Whiteboard: [systemsfe])
Attachments
(3 files, 3 obsolete files)
When some system dialog are exposed, I have a very hard time tapping on the "Confirm" or "Cancel" on those.
STR:
0. Use a device that reproduce. For sure, Aries and Shinano does, I'm pretty sure Hamachi does also. We have not been able to reproduce this on Flame.
1. Trigger one of the system dialog: data roaming confirmation, APN deletion
Expected:
I can tap "Cancel" or "Confirm" easily
Actual:
I'm unable to trigger those touch target, most of the time. After trying dozen of times, if I tap exactly on the text, it seems to be able to trigger the action.
This is a regression for sure, given we see the behavior on Hamachi and it was not there. I'm pretty sure I saw this at least one or two months ago.
Kats, can it be related to touch target work that has been done recently?
Flags: needinfo?(jlorenzo)
Flags: needinfo?(bugmail.mozilla)
Comment 1•10 years ago
|
||
Maybe. I'll check on Hamachi since it's the only one of the devices you listed that I have. Leaving ni for now.
Comment 2•10 years ago
|
||
As hamachi is not really supported any more and I can't get it to build successfully I can't investigate myself. At best I can provide a patch for additional logging. Let me know if you want to go that route.
Flags: needinfo?(bugmail.mozilla)
Comment 3•10 years ago
|
||
So I think the best first step would be to (a) enable the layers tree dumping from the developer settings, (b) enable the APZCTM logging at [1], InputQueue logging at [2], and APZCCH logging at [3]. Grab a log with that and it should get us started with tracking down the problem.
[1] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/APZCTreeManager.cpp?rev=901326c8a66c#33
[2] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/InputQueue.cpp?rev=901326c8a66c#16
[3] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/util/APZCCallbackHelper.cpp?rev=901326c8a66c#18
Assignee | ||
Comment 4•10 years ago
|
||
Assignee | ||
Comment 5•10 years ago
|
||
Assignee | ||
Comment 6•10 years ago
|
||
Two dumps with all the debug bits asked for enabled. Each one should only match one tap on the Confirm button of the dialog.
Comment 7•10 years ago
|
||
Not sure if you saw my IRC messages:
13:01:40 kats: gerard-majax: so yeah it looks like the touch events are getting preventDefaulted. but also it looks like the touch events are hitting stuff in the child process. if this is a system dialog then i would expect that to hit stuff in the parent process
13:02:48 kats: gerard-majax: from the layer dump i see most of the stuff in the parent process is in the top 100 pixels of the screen
13:03:26 kats: so if there's a dialog on the screen it's gotta be in the child process
Assignee | ||
Comment 8•10 years ago
|
||
Yes, that's in the settings app
Assignee | ||
Comment 9•10 years ago
|
||
From WebIDE I can see that Confirm and Cancel buttons are both with only one 'click' event handler.
Comment 10•10 years ago
|
||
Not sure what to tell you then. There's definitely a listener somewhere that's calling preventDefault. Maybe WebIDE can't see it because it's registered at the window level or something.
Assignee | ||
Updated•10 years ago
|
Component: Gaia::System::Window Mgmt → Gaia::Settings
Summary: Unable to tap "confirm" or "cancel" on some system dialog → Unable to tap "confirm" or "cancel" on some settings dialog
Assignee | ||
Comment 11•10 years ago
|
||
Can you help us?
Flags: needinfo?(ejchen)
Flags: needinfo?(arthur.chen)
Assignee | ||
Updated•10 years ago
|
Summary: Unable to tap "confirm" or "cancel" on some settings dialog → Unable to tap "confirm" or "cancel" on some settings dialog: APN deletion, data roaming activation
Flags: needinfo?(jlorenzo)
Assignee | ||
Comment 12•10 years ago
|
||
Woot, indeed I found the culprit: the workaround of bug 918288 in https://github.com/mozilla-b2g/gaia/blob/master/shared/elements/gaia_confirm/script.js#L20
If I remove the 'touchmove' from there, then it works :)
Flags: needinfo?(kgrandon)
Flags: needinfo?(jlal)
Flags: needinfo?(ejchen)
Flags: needinfo?(arthur.chen)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → lissyx+mozillians
Comment 13•10 years ago
|
||
Assignee | ||
Comment 14•10 years ago
|
||
Attachment #8566608 -
Attachment is obsolete: true
Attachment #8566609 -
Attachment is obsolete: true
Attachment #8567489 -
Flags: review?(kgrandon)
Assignee | ||
Updated•10 years ago
|
Attachment #8567489 -
Attachment is obsolete: true
Attachment #8567489 -
Flags: review?(kgrandon)
Assignee | ||
Updated•10 years ago
|
Attachment #8567488 -
Flags: review?(kgrandon)
Assignee | ||
Updated•10 years ago
|
Component: Gaia::Settings → Gaia::Shared
Comment 15•10 years ago
|
||
Comment on attachment 8567488 [details]
[gaia] lissyx:bug1134642 > mozilla-b2g:master
It appears that even though bug 918288 has landed, if you remove this workaround you re-introduce what it was trying to fix, the scrolling under the dialog =(
Kats - is there more work needed for APZ testing? Any chance you could give this patch a try and see what's causing us to scroll on the home screen after opening up a confirmation dialog? You should see the dialog if you try to delete an application or smart collection.
Flags: needinfo?(kgrandon)
Flags: needinfo?(jlal)
Flags: needinfo?(bugmail.mozilla)
Attachment #8567488 -
Flags: review?(kgrandon)
Assignee | ||
Comment 16•10 years ago
|
||
(In reply to Kevin Grandon :kgrandon from comment #15)
> Comment on attachment 8567488 [details]
> [gaia] lissyx:bug1134642 > mozilla-b2g:master
>
> It appears that even though bug 918288 has landed, if you remove this
> workaround you re-introduce what it was trying to fix, the scrolling under
> the dialog =(
Then we need another fix, because this is badly breaking some features on Shinano and Aries devices, at least :(
Assignee | ||
Comment 17•10 years ago
|
||
FYI Kevin, I have not seen the behavior you are talking about of moving to the homescreen on the devices I tested.
Comment 18•10 years ago
|
||
Ok, I tested this again, and it appears to not happen in *all* cases. It does happen on a flame though when trying to remove a smart collection. It won't happen with an app uninstall dialog as we've moved that to the system process, so I guess the dialog must be in the same process, and probably on a scrollable page.
There may be another workaround we can use, but I haven't figured out what that would be yet.
Comment 19•10 years ago
|
||
I looked into this and it appears the dialog screen is a fixed-position child of the root scrollable element. That's why scrolling on the dialog causes the root scrollable element to scroll (because it's the nearest scrolling ancestor). The same behaviour occurs on e.g. http://chrislord.net/files/mozilla/fixed.html if you start a scroll from on top of one of fixed-position corner blocks. I can see how this is unexpected though, so maybe we should detect that in the APZ code and not let scrolling happen. I can write a patch to do this if that makes sense.
Flags: needinfo?(bugmail.mozilla)
Assignee | ||
Updated•10 years ago
|
Blocks: shinano-backlog
Comment 20•10 years ago
|
||
Thinking about this more I'm not sure we should change this on the APZ side. Open to discussion over in bug 1135538. The alternative for this bug would be to actually push those dialogs into the parent process, or somehow lift them out of the homescreen's scrollable element. I'm guessing that's the <body> which would would mean we'd have to push the icons into a <div> instead and have the dialog live as a sibling of that div. Not pretty, which is why I'd rather use a parent-process dialog instead.
Comment 21•10 years ago
|
||
We don't want to put the homescreen scrollable area in a div right now because we are trying to capture mozbrowser scroll events for it in the system app.
Moving the dialogs out of the home screen might be an option, but that's something a third party developer couldn't really do so it would be good to find a solution for them as well.
Comment 22•10 years ago
|
||
Could you throw an overflow:hidden on the body while the dialog is in front?
Comment 23•10 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #22)
> Could you throw an overflow:hidden on the body while the dialog is in front?
It should be possible. I will try to take a look at this later.
Flags: needinfo?(kgrandon)
Comment 24•10 years ago
|
||
Comment 25•10 years ago
|
||
Comment on attachment 8568877 [details]
[gaia] KevinGrandon:bug_1134642_lissyx_fix_and_overflow_fix > mozilla-b2g:master
I think this works well for now, and is probably a good idea to implement on gaia side so the platform doesn't diverge from Fennec/Android.
Chris - Can you review the home screen changes here? The switch statement abuse is starting to feel a bit bad, but refactoring it is a bigger change than I really want to make right now.
I don't really think this is worth writing a test for yet since the APZ logic is so different on desktop vs device.
Flags: needinfo?(kgrandon)
Attachment #8568877 -
Flags: review?(chrislord.net)
Assignee | ||
Comment 26•10 years ago
|
||
I'm going to give this a try.
Assignee | ||
Comment 27•10 years ago
|
||
Comment on attachment 8568877 [details]
[gaia] KevinGrandon:bug_1134642_lissyx_fix_and_overflow_fix > mozilla-b2g:master
That still fixes my issue :)
Attachment #8568877 -
Flags: feedback+
Comment 28•10 years ago
|
||
Comment on attachment 8568877 [details]
[gaia] KevinGrandon:bug_1134642_lissyx_fix_and_overflow_fix > mozilla-b2g:master
LGTM.
Attachment #8568877 -
Flags: review?(chrislord.net) → review+
Comment 29•10 years ago
|
||
Thanks for the review. In master: https://github.com/mozilla-b2g/gaia/commit/b56d199dd5653313bf0ab03084913fa57ec16dcb
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Whiteboard: [systemsfe]
Comment 30•9 years ago
|
||
This bug has been verified as "pass" on the latest build of Aries KK v2.6 by the STR in comment 0.
Actual results: user can tap "Cancel" or "Confirm" easily.
See attachment: verified_Aries KK_v2.6.3gp.
Reproduce rate: 0/10.
Device: Aries KK v2.6(master) (Pass)
Build ID 20151105105137
Gaia Revision e68d693cb55fb5d8946498eb2bdb63f55116d38e
Gaia Date 2015-11-05 01:05:27
Gecko Revision n/a
Gecko Version 45.0a1
Device Name aries
Firmware(Release) 4.4.2
Firmware(Incremental) eng.rose.20151103.153542
Firmware Date Tue Nov 3 15:36:03 CST 2015
Bootloader s1
QA Whiteboard: [MGSEI-Triage+]
status-b2g-master:
--- → verified
Comment 31•9 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•