[319MB] Having a keyboard active will disable all app interaction if the user has typed anything

VERIFIED FIXED in Firefox 38

Status

()

defect
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: dharris, Assigned: kats)

Tracking

({regression, smoketest})

unspecified
mozilla38
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.2+, firefox36 unaffected, firefox37 unaffected, firefox38 fixed, b2g-v2.2 verified, b2g-master verified)

Details

(URL)

Attachments

(3 attachments)

Description:
When the user types something into an app with the keyboard active, they will no longer be able to interact with any buttons within any app they are in. The Power button, Home button, Keyboard, and Edge Gestures will not be affected by this bug.


Repro Steps:
1) Update a Flame to 20150113010202
2) Open an app with a keyboard, such as Messages
3) Tap on the new message Icon
4) Type a phone number into the "To:" Field
5) Select the Back button, "...", "+", or attach Icon


Actual:
All app interaction will be disabled as long as the keyboard is active


Expected:
User is able to fully interact within any app with the keyboard open

Environmental Variables:
Device: Flame Master (319mb)(Kitkat)(Full Flash)
Build ID: 20150113010202
Gaia: 9946a490a9264b42e65385d703b28fa055ab2d42
Gecko: 3d846527576f
Gonk: a814b2e2dfdda7140cb3a357617dc4fbb1435e76
Version: 38.0a1 (Master)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0


Notes: This bug will occur with any active keyboard (QWERTY and number). If the user dismisses the keyboard, they will be able to interact with the app once again.


Repro frequency: 15/15 100%
See attached: Logcat, Video - http://youtu.be/scS0APMasT8
This issue does NOT reproduce on Flame 2.2

Keyboard does NOT disable user interaction within apps

Environmental Variables:
Device: Flame 2.2 (319mb)(Kitkat)(Full Flash)
Build ID: 20150112010228
Gaia: f5e481d4caf9ffa561720a6fc9cf521a28bd8439
Gecko: bb8d6034f5f2
Gonk: a814b2e2dfdda7140cb3a357617dc4fbb1435e76
Version: 37.0a1 
Firmware Version: V18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
Functional regression that breaks smoke tests.


Requesting a window.
blocking-b2g: --- → 2.2?
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
QA Contact: pcheng
blocking-b2g: 2.2? → 2.2+
b2g-inbound regression window:

Last Working Environmental Variables:
Device: Flame
BuildID: 20150109225731
Gaia: b875c1ddb0a024d1d375d549d0a0268adb27ae80
Gecko: 0d9ceb0ea832
Version: 37.0a1 (2.2 Master)
Firmware: V18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

First Broken Environmental Variables:
Device: Flame
BuildID: 20150110075632
Gaia: b875c1ddb0a024d1d375d549d0a0268adb27ae80
Gecko: 27d6d4a76992
Version: 37.0a1 (2.2 Master)
Firmware: V18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Gaia is the same so it's a Gecko issue.

Gecko pushlog:
http://hg.mozilla.org/integration/b2g-inbound/pushloghtml?fromchange=0d9ceb0ea832&tochange=27d6d4a76992

Caused by patches to bug 1119497.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
:kats, can you please look into this asap and help backout changes in https://bugzilla.mozilla.org/show_bug.cgi?id=1119497 on m-c and b2g_37 branch?
Flags: needinfo?(bugmail.mozilla)
Investigating...
Assignee: nobody → bugmail.mozilla
Flags: needinfo?(bugmail.mozilla)
Why is this 2.2+ if (as per comment 1 and the bug track flags) 2.2 is unaffected?

Also bug 1119497 did land in 2.2 so if 2.2 is unaffected then the regression window can't be correct.
Flags: needinfo?(dharris)
We don't have a blocking-3.0? option yet, will investigate what we want to do for that.

Adding qawanted to redo the 2.2 check and to verify if the build has the hash for bug 1119497.
Flags: needinfo?(dharris)
Keywords: qaurgent, qawanted
Patches for bug 1119497 didn't land in 2.2 master until 6AM of Jan. 12 (See bug 1119497 comment 8). The nightly build that :DerekH tested against was generated on 1AM of Jan. 12, which doesn't include the patches.

Re-tested on latest 2.2 (branched into b2g37) and issue DOES occur.
Device: Flame
BuildID: 20150113083630
Gaia: 7c5b27cad370db377b18a742d3f3fdb0070e899f
Gecko: 5caa5b12dfa6
Gonk: a814b2e2dfdda7140cb3a357617dc4fbb1435e76
Version: 37.0a2 (2.2)
Firmware: V18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
I'm also not able to reproduce using a local build from latest master. My base image is v188. I'll try using the latest pvtbuild.
Not able to reproduce using the latest pvtbuild either. Are there any other settings you're changing on the device? Where can I get the v18D-1 base image?
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker) → needinfo?(pcheng)
I found the v18D-1 base image and am still not able to reproduce. However if I set the layout.event-regions.enabled pref to false then I *can* reproduce exactly what you're seeing. Do you have that pref set by any chance? It should be set to true by default but in a newsgroup posting I suggested setting it to false in some cases, so it may be that it was left in that state accidentally on your devices. Please ensure that layout.event-regions.enabled is set to true.

That being said it is a bug that this manifests with that pref set to false, and I can look into it, but it shouldn't be a smoketest blocker.
Flags: needinfo?(pcheng)
Based on the prefs.js file Peter sent me it doesn't appear to be the pref. So I still can't reproduce this. I'm fine with backing out bug 1119497 until we figure this out but I would like to have STR that I can reproduce locally first.
Ah, PBylenga provided me some info over email, and it seems I have to bump the Flame memory down to 319 MB to make this reproduce, because that forces the keyboard to be in-process. I'll back out 1119497 for now, and will re-land it once I've figured out how to fix this.
Backed out bug 1119497 and part of bug 920036.

https://hg.mozilla.org/integration/b2g-inbound/rev/86bcc35413b4
Summary: [Keyboards] Having a keyboard active will disable all app interaction if the user has typed anything → [Keyboards][319MB] Having a keyboard active will disable all app interaction if the user has typed anything
Component: Gaia::Keyboard → Panning and Zooming
Product: Firefox OS → Core
Summary: [Keyboards][319MB] Having a keyboard active will disable all app interaction if the user has typed anything → [319MB] Having a keyboard active will disable all app interaction if the user has typed anything
The problem here seems to be that the keyboard is in an iframe which is the full screen, and so the hitRegion for that ViewportFrame is the full screen area, and it eats all the input events. The way this works with Gecko hit-testing code is that the mozpasspointerevents flag which is set on the keyboard iframe induces special behaviour. The event regions code however doesn't properly respect that flag.
This seems to fix it for me. I would have used GetType() == nsGkAtoms::viewportFrame but that's virtual so I tried using !GetContent instead. I don't know how correct that is.
Attachment #8548951 - Flags: review?(roc)
I wonder why intersecting the clip rect with the touch-sensitive region didn't handle this: http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/APZCTreeManager.cpp?rev=27d6d4a76992#257

(Not that we want to keep that "touch-sensitive region" around, just curious.)
(In reply to Botond Ballo [:botond] from comment #17)
> I wonder why intersecting the clip rect with the touch-sensitive region
> didn't handle this:

Touch-sensitive regions only exist in child processes but the keyboard iframe lives in the root process and sits on top of everything, so it "intercepts" the input events. I think to use the touch-sensitive region to fix this, we would have to take the inverse of the touch-sensitive region from the child process and apply it on the parent process. But there might be other cases where stuff in the root process is on top of the child process and those might break.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #18)
> (In reply to Botond Ballo [:botond] from comment #17)
> > I wonder why intersecting the clip rect with the touch-sensitive region
> > didn't handle this:
> 
> Touch-sensitive regions only exist in child processes but the keyboard
> iframe lives in the root process and sits on top of everything, so it
> "intercepts" the input events. I think to use the touch-sensitive region to
> fix this, we would have to take the inverse of the touch-sensitive region
> from the child process and apply it on the parent process. But there might
> be other cases where stuff in the root process is on top of the child
> process and those might break.

Ah, I see - the keyboard can be in-process or out-of-process, as described in comment 13, and the touch-sensitive regions only handle the in-process case.
(In reply to Botond Ballo [:botond] from comment #19)
> Ah, I see - the keyboard can be in-process or out-of-process, as described
> in comment 13, and the touch-sensitive regions only handle the in-process
> case.

Er, it only hands the out-of-process case, sorry.
Martijn, do we have automation coverage for this regression?
Flags: in-qa-testsuite?(martijn.martijn)
Comment on attachment 8548951 [details] [diff] [review]
Respect mozpasspointerevents when building event regions

Review of attachment 8548951 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/nsDisplayList.cpp
@@ +3055,5 @@
>                                       nsIFrame* aFrame)
>  {
>    NS_ASSERTION(aBuilder->FindReferenceFrameFor(aFrame) == aBuilder->FindReferenceFrameFor(mFrame),
>                 "Reference frame mismatch");
> +  if (!aFrame->GetContent()) {

Better to test aFrame->GetParent().

That viewport frames have no element is a bit of a wart that would be good to fix one day.
Attachment #8548951 - Flags: review?(roc) → review+
(Assignee)

Comment 23

4 years ago
greentry
Did a couple of try pushes. First one has the patch attached to this bug, but is opt-only. Second one includes the change roc suggested and gets some debug coverage.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d8f44d9ec471
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b3ddbc6eeb4f
https://hg.mozilla.org/mozilla-central/rev/de9ee7b2ef08
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Duplicate of this bug: 1121891
Please request b2g37 approval on this when you get a chance :) Or is this something we should consider taking on Aurora/Beta?
Flags: needinfo?(bugmail.mozilla)
Comment on attachment 8548951 [details] [diff] [review]
Respect mozpasspointerevents when building event regions

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 1119497
User impact if declined: On devices with in-process keyboard (Flame w/ 319MB or other low-mem devices), it is impossible to interact with the content area while the keyboard is up.
Testing completed: locally, on m-c
Risk to taking this patch (and alternatives if risky): I'd say it's fairly low risk. The patch is small and the code is not particularly complicated, it was just never fully implemented. It only affects platforms where layout.event-regions.enabled is true, which currently is only B2G.
String or UUID changes made by this patch: none
Flags: needinfo?(bugmail.mozilla)
Attachment #8548951 - Flags: approval-mozilla-b2g37?
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #27)
> Or is this
> something we should consider taking on Aurora/Beta?

Nah, it only affects B2G so need to take it on aurora.
Attachment #8548951 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+

Comment 31

4 years ago
Posted video verify_video.MP4
This issue has been verified successfully on Flame v2.2
See attachment:verify_video.MP4

Flame 2.2 build:
Gaia-Rev        e4f9b5da3751798f9cc5d95f302c30722cc11fca
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/75a462a58d7a
Build-ID        20150121002607
Version         37.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150121.040751
FW-Date         Wed Jan 21 04:08:02 EST 2015
Bootloader      L1TC000118D0
This issue is verified fixed on Flame 3.0 Master and on Flame 2.2.

Observed behavior: Following STR, app can be correctly interacted while the keyboard is invoked.

Device: Flame 3.0 Master (shallow flash, 319MB mem)
BuildID: 20150122143248
Gaia: 966b3a7a13a7f0d5b86cbc9e64cb78d43ec7dba8
Gecko: 568bbf124221
Version: 38.0a1 (3.0 Master)
Firmware: V18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Device: Flame 2.2 (shallow flash, 319MB mem)
BuildID: 20150122141946
Gaia: 237008137f6d72b9cad25ff4faff14ff2c40ac63
Gecko: 1090c8c5429f
Version: 37.0a2 (2.2)
Firmware: V18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Depends on: 1130038
(In reply to Peter Bylenga [:PBylenga] from comment #21)
> Martijn, do we have automation coverage for this regression?

Sorry, I haven't seen this before. I don't know if one or more of the automation tests were failing when this bug was occuring. The automation reports don't seem to indicated this was catched in automation.
So I filed bug 1130038 for creating a gaia-ui test for this.
Depends on: 1172310
(In reply to Peter Bylenga [:PBylenga] from comment #21)
> Martijn, do we have automation coverage for this regression?

Sorry, I didn't create an automated test for this and at this point, it's probably not that important anymore.
Flags: in-qa-testsuite?(martijn.martijn)
You need to log in before you can comment on or make changes to this bug.