Closed Bug 1134642 Opened 6 years ago Closed 6 years ago
Unable to tap "confirm" or "cancel" on some settings dialog: APN deletion, data roaming activation
46 bytes, text/x-github-pull-request
|Details | Review|
46 bytes, text/x-github-pull-request
|Details | Review|
4.99 MB, video/3gpp
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?
Maybe. I'll check on Hamachi since it's the only one of the devices you listed that I have. Leaving ni for now.
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.
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 , InputQueue logging at , and APZCCH logging at . Grab a log with that and it should get us started with tracking down the problem.  http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/APZCTreeManager.cpp?rev=901326c8a66c#33  http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/InputQueue.cpp?rev=901326c8a66c#16  http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/util/APZCCallbackHelper.cpp?rev=901326c8a66c#18
Two dumps with all the debug bits asked for enabled. Each one should only match one tap on the Confirm button of the dialog.
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
Yes, that's in the settings app
From WebIDE I can see that Confirm and Cancel buttons are both with only one 'click' event handler.
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.
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
Can you help us?
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
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 :)
Component: Gaia::Settings → Gaia::Shared
Comment on attachment 8567488 [details] [review] [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.
(In reply to Kevin Grandon :kgrandon from comment #15) > Comment on attachment 8567488 [details] [review] > [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 :(
FYI Kevin, I have not seen the behavior you are talking about of moving to the homescreen on the devices I tested.
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.
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.
6 years ago
Depends on: 1135538
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.
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.
Could you throw an overflow:hidden on the body while the dialog is in front?
(In reply to Kartikaya Gupta (email:email@example.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.
Comment on attachment 8568877 [details] [review] [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.
Attachment #8568877 - Flags: review?(chrislord.net)
I'm going to give this a try.
Comment on attachment 8568877 [details] [review] [gaia] KevinGrandon:bug_1134642_lissyx_fix_and_overflow_fix > mozilla-b2g:master That still fixes my issue :)
Attachment #8568877 - Flags: feedback+
Comment on attachment 8568877 [details] [review] [gaia] KevinGrandon:bug_1134642_lissyx_fix_and_overflow_fix > mozilla-b2g:master LGTM.
Attachment #8568877 - Flags: review?(chrislord.net) → review+
Thanks for the review. In master: https://github.com/mozilla-b2g/gaia/commit/b56d199dd5653313bf0ab03084913fa57ec16dcb
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
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
You need to log in before you can comment on or make changes to this bug.