Unable to tap "confirm" or "cancel" on some settings dialog: APN deletion, data roaming activation

RESOLVED FIXED

Status

RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: gerard-majax, Assigned: gerard-majax)

Tracking

({regression})

unspecified
ARM
Gonk (Firefox OS)
regression
Dependency tree / graph

Firefox Tracking Flags

(b2g-master verified)

Details

(Whiteboard: [systemsfe])

Attachments

(3 attachments, 3 obsolete attachments)

(Assignee)

Description

4 years ago
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)
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.
Flags: needinfo?(bugmail.mozilla)
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

4 years ago
Created attachment 8566608 [details]
bug-confirm2.log
(Assignee)

Comment 5

4 years ago
Created attachment 8566609 [details]
bug-confirm.log
(Assignee)

Comment 6

4 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.
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

4 years ago
Yes, that's in the settings app
(Assignee)

Comment 9

4 years ago
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.
(Assignee)

Updated

4 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

4 years ago
Can you help us?
Flags: needinfo?(ejchen)
Flags: needinfo?(arthur.chen)
(Assignee)

Updated

4 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

4 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

4 years ago
Assignee: nobody → lissyx+mozillians
(Assignee)

Updated

4 years ago
Depends on: 918288
Created attachment 8567488 [details] [review]
[gaia] lissyx:bug1134642 > mozilla-b2g:master
(Assignee)

Comment 14

4 years ago
Created attachment 8567489 [details] [review]
Gaia PR
Attachment #8566608 - Attachment is obsolete: true
Attachment #8566609 - Attachment is obsolete: true
Attachment #8567489 - Flags: review?(kgrandon)
(Assignee)

Updated

4 years ago
Attachment #8567489 - Attachment is obsolete: true
Attachment #8567489 - Flags: review?(kgrandon)
(Assignee)

Updated

4 years ago
Attachment #8567488 - Flags: review?(kgrandon)
(Assignee)

Updated

4 years ago
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.
Flags: needinfo?(kgrandon)
Flags: needinfo?(jlal)
Flags: needinfo?(bugmail.mozilla)
Attachment #8567488 - Flags: review?(kgrandon)
(Assignee)

Comment 16

4 years ago
(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 :(
(Assignee)

Comment 17

4 years ago
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.
Flags: needinfo?(bugmail.mozilla)
(Assignee)

Updated

4 years ago
Blocks: 1131249
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: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)
Created attachment 8568877 [details] [review]
[gaia] KevinGrandon:bug_1134642_lissyx_fix_and_overflow_fix > mozilla-b2g:master
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.
Flags: needinfo?(kgrandon)
Attachment #8568877 - Flags: review?(chrislord.net)
(Assignee)

Comment 26

4 years ago
I'm going to give this a try.
(Assignee)

Comment 27

4 years ago
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
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [systemsfe]
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
Created attachment 8683564 [details]
verified_Aries KK_v2.6.3gp
You need to log in before you can comment on or make changes to this bug.