[APZC] Keyboard disappears after typing first letter

RESOLVED FIXED in Firefox 28

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gerard-majax, Assigned: botond)

Tracking

({regression})

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

Firefox Tracking Flags

(blocking-b2g:1.3+, firefox28 fixed, firefox29 fixed, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Since this weekend, I've noticed that the keyboard has a very bad behavior of hiding itself.

For now, STRs that I can get are:
0. Launch an app with text input. Messages is okay.
1. Tap into the text input.
2. Quickly, tap on a letter.
3. Then, quickly, continue to tap on other letters.

As far as I can say, reset-gaia has no impact; and it does not seems to be related to locale nor the configured keyboard. Enabling/disabling word suggestion does not help.

I do only see this in logcat, but not 100% it relates:
E/GeckoConsole(  261): [JavaScript Error: "TypeError: keyboard.sendKey(...) is undefined" {file: "app://keyboard.gaiamobile.org/js/imes/latin/latin.js" line: 406}]

I'm reproducing on several devices with gecko and gaia master: Inari, Nexus S, Peak.

For Inari, gecko@12dd20e and gaia@5ff5169.
Reporter

Updated

5 years ago
Keywords: regression
Reporter

Comment 1

5 years ago
As far as I can say, it seems that disabling APZC for Apps (in the developper menu) and then rebooting makes the issue not reproductible anymore.
Reporter

Updated

5 years ago
blocking-b2g: --- → 1.3?
status-b2g-v1.3: --- → ?
Reporter

Updated

5 years ago
Summary: Keyboard disappears after typing first letter → [APZC] Keyboard disappears after typing first letter
Reporter

Updated

5 years ago
Blocks: gaia-apzc
Assignee

Comment 2

5 years ago
I'll have a look at this.
Assignee: nobody → botond
Assignee

Comment 3

5 years ago
I tested this with Nexus 4 running master and Hamachi running 1.3. The only thing I could reproduce is, occasionally, if you tap on a input field and then quickly tap on the area of the screen where the keyboard is about to appear, before it actually appears, the keyboard will briefly flash on the screen and then disappear. However,

  1) I see this behaviour both with and without APZC.
  2) I think this behaviour is expected. Since the keyboard hasn't appeared yet when you tap
     the screen, the tap is interpreted as a tap on a non-input region which triggers the
     keyboard to disappear.

Alexandre, you said you could only see this with APZC enabled. Could you clarify whether the behaviour you saw is the one I described, or something else?
Flags: needinfo?(lissyx+mozillians)
Reporter

Comment 4

5 years ago
(In reply to Botond Ballo [:botond] from comment #3)
> I tested this with Nexus 4 running master and Hamachi running 1.3. The only
> thing I could reproduce is, occasionally, if you tap on a input field and
> then quickly tap on the area of the screen where the keyboard is about to
> appear, before it actually appears, the keyboard will briefly flash on the
> screen and then disappear. However,
> 
>   1) I see this behaviour both with and without APZC.
>   2) I think this behaviour is expected. Since the keyboard hasn't appeared
> yet when you tap
>      the screen, the tap is interpreted as a tap on a non-input region which
> triggers the
>      keyboard to disappear.
> 
> Alexandre, you said you could only see this with APZC enabled. Could you
> clarify whether the behaviour you saw is the one I described, or something
> else?

In my case, the keyboard is already appearing. I can type the first letter. Then on the second one, it disappears.

I've disabled APZC on my Nexus S, rebooted, and it's been two hours I can't reproduce while it was constant previously. Please note that the use of shmem slows down a bit the device and might help in reproducing.
Flags: needinfo?(lissyx+mozillians)
Assignee

Comment 5

5 years ago
(In reply to Alexandre LISSY :gerard-majax from comment #4)
> (In reply to Botond Ballo [:botond] from comment #3)
> > I tested this with Nexus 4 running master and Hamachi running 1.3. The only
> > thing I could reproduce is, occasionally, if you tap on a input field and
> > then quickly tap on the area of the screen where the keyboard is about to
> > appear, before it actually appears, the keyboard will briefly flash on the
> > screen and then disappear. However,
> > 
> >   1) I see this behaviour both with and without APZC.
> >   2) I think this behaviour is expected. Since the keyboard hasn't appeared
> > yet when you tap
> >      the screen, the tap is interpreted as a tap on a non-input region which
> > triggers the
> >      keyboard to disappear.
> > 
> > Alexandre, you said you could only see this with APZC enabled. Could you
> > clarify whether the behaviour you saw is the one I described, or something
> > else?
> 
> In my case, the keyboard is already appearing. I can type the first letter.
> Then on the second one, it disappears.
> 
> I've disabled APZC on my Nexus S, rebooted, and it's been two hours I can't
> reproduce while it was constant previously. Please note that the use of
> shmem slows down a bit the device and might help in reproducing.

So the first letter goes through (makes it to the input field) and the second one doesn't? If so, I haven't been able to reproduce this at all.
Reporter

Comment 6

5 years ago
Yes, that is exactly it. Having spent my day tracking this too, I can assure you it's here :).
Component: Gaia::Keyboard → Panning and Zooming
Product: Firefox OS → Core
Assignee

Comment 7

5 years ago
I updated by hamachi to the latest master in the hopes of being able to reproduce this, and I now see the keyboard crashing all the time, with the following stack trace:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 744.832]
0x41ccc1fc in js::jit::LiveInterval::start (this=0x44ffeb30) at /home/botond/dev/mozilla/central/js/src/jit/LiveRangeAllocator.h:267
267             JS_ASSERT(!ranges_.empty());
(gdb) i s
#0  0x41ccc1fc in js::jit::LiveInterval::start (this=0x44ffeb30) at /home/botond/dev/mozilla/central/js/src/jit/LiveRangeAllocator.h:267
#1  js::jit::LiveRangeAllocator<js::jit::LinearScanVirtualRegister, true>::findFirstSafepoint (this=0x44ffeb30) at /home/botond/dev/mozilla/central/js/src/jit/LiveRangeAllocator.h:721
#2  js::jit::LinearScanAllocator::populateSafepoints (this=0x44ffeb30) at /home/botond/dev/mozilla/central/js/src/jit/LinearScan.cpp:499
#3  0x41cd009c in js::jit::LinearScanAllocator::go (this=0x44ffeb30) at /home/botond/dev/mozilla/central/js/src/jit/LinearScan.cpp:1272
#4  0x41c63a2c in js::jit::GenerateLIR (mir=0x4681d170) at /home/botond/dev/mozilla/central/js/src/jit/Ion.cpp:1436
#5  0x41c63c2e in js::jit::CompileBackEnd (mir=0x4681d170, maybeMasm=0x0) at /home/botond/dev/mozilla/central/js/src/jit/Ion.cpp:1527
#6  0x41c6a6d0 in IonCompile (cx=0x404c75e0, script=..., osrFrame=<value optimized out>, osrPc=0x0, constructing=<value optimized out>, executionMode=js::SequentialExecution) at /home/botond/dev/mozilla/central/js/src/jit/Ion.cpp:1776
#7  Compile (cx=0x404c75e0, script=..., osrFrame=<value optimized out>, osrPc=0x0, constructing=<value optimized out>, executionMode=js::SequentialExecution) at /home/botond/dev/mozilla/central/js/src/jit/Ion.cpp:1979
#8  0x41c6aca0 in js::jit::CompileFunctionForBaseline (cx=0x404c75e0, script=..., frame=0x44ffef70, isConstructing=false) at /home/botond/dev/mozilla/central/js/src/jit/Ion.cpp:2145
#9  0x4203728c in EnsureCanEnterIon (cx=0x404c75e0, stub=<value optimized out>, frame=0x44ffef70, infoPtr=0x44ffef2c) at /home/botond/dev/mozilla/central/js/src/jit/BaselineIC.cpp:768
#10 DoUseCountFallback (cx=0x404c75e0, stub=<value optimized out>, frame=0x44ffef70, infoPtr=0x44ffef2c) at /home/botond/dev/mozilla/central/js/src/jit/BaselineIC.cpp:934
#11 0x4366f6b4 in ?? ()
#12 0x4366f6b4 in ?? ()
Backtrace stopped: previous frame identical to this frame (corrupt stack?)

Is it possible that this is what you're seeing? In a non-debug build, the assert wouldn't cause a crash directly, but it might down the line, or if not a crash it might cause other misbehaviour (such as the one described in this bug).
Assignee

Comment 8

5 years ago
(In reply to Botond Ballo [:botond] from comment #7)
> I updated by hamachi to the latest master in the hopes of being able to
> reproduce this, and I now see the keyboard crashing all the time, with the
> following stack trace:
> 
> Program received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 744.832]
> 0x41ccc1fc in js::jit::LiveInterval::start (this=0x44ffeb30) at
> /home/botond/dev/mozilla/central/js/src/jit/LiveRangeAllocator.h:267
> 267             JS_ASSERT(!ranges_.empty());

I talked to Kannan about this. He said it's a bug in IonMonkey (I filed bug 959328 for it), but it's very difficult to say whether a misbehaviour like the one described in this bug is related to it or not.
Reporter

Comment 9

5 years ago
I fear it's a dupe of bug 959126 then. We filed it today with nicolas :)
Reporter

Comment 10

5 years ago
Looks like a dupe of bug 959126 :)
Flags: needinfo?(nicolas.b.pierron)
Assignee

Comment 11

5 years ago
This bug is marked as affecting B2G 1.3, but the crash in question looks like a regression introduced after 1.3. 

To clarify, has someone tested this with 1.3? If so, and the bug reproduces, it's likely caused by something other than this IonMonkey crash.
Flags: needinfo?(jsmith)
We can double check this on 1.3. QA Wanted to confirm this reproduces on 1.3.
Flags: needinfo?(jsmith)
Keywords: qawanted
(In reply to Botond Ballo [:botond] from comment #7)
> Is it possible that this is what you're seeing? In a non-debug build, the
> assert wouldn't cause a crash directly, but it might down the line, or if
> not a crash it might cause other misbehaviour (such as the one described in
> this bug).

You could see if you get the same behaviour that Alexandre observed if you comment out the assert. Also see if you get the same Ion crash with APZC disabled, since Alexandre's STR involve turning on APZC.
Okay - removing qawanted per comment 13
Keywords: qawanted
Assignee

Comment 15

5 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #13)
> (In reply to Botond Ballo [:botond] from comment #7)
> > Is it possible that this is what you're seeing? In a non-debug build, the
> > assert wouldn't cause a crash directly, but it might down the line, or if
> > not a crash it might cause other misbehaviour (such as the one described in
> > this bug).
> 
> You could see if you get the same behaviour that Alexandre observed if you
> comment out the assert. 

I tried that; I got another assert in related-looking code. Rather than playing whac-a-mole with the assertions, I am now building a non-debug build and will try to repro with that.

> Also see if you get the same Ion crash with APZC
> disabled, since Alexandre's STR involve turning on APZC.

I do. I guess that suggests the crash is unrelated and I'll have better luck reproing this with a non-debug build.
Assignee

Comment 16

5 years ago
Tested a non-debug build of master on Nexus 4. No crash, but I can't repro the bug either.

Will try a non-debug build on Hamachi next.
Assignee

Comment 17

5 years ago
(In reply to Botond Ballo [:botond] from comment #16)
> Will try a non-debug build on Hamachi next.

Can't repro it there either.
Assignee

Comment 18

5 years ago
(In reply to Alexandre LISSY :gerard-majax from comment #4)
> I've disabled APZC on my Nexus S, rebooted, and it's been two hours I can't
> reproduce while it was constant previously. Please note that the use of
> shmem slows down a bit the device and might help in reproducing.

I will try on Hamachi with gralloc disabled to create more memory pressure and hopefully repro then.
Assignee

Comment 19

5 years ago
(In reply to Botond Ballo [:botond] from comment #18)
> (In reply to Alexandre LISSY :gerard-majax from comment #4)
> > I've disabled APZC on my Nexus S, rebooted, and it's been two hours I can't
> > reproduce while it was constant previously. Please note that the use of
> > shmem slows down a bit the device and might help in reproducing.
> 
> I will try on Hamachi with gralloc disabled to create more memory pressure
> and hopefully repro then.

Still doesn't repro unfortunately. I'll try to get a hold of an Inari, Nexus S, or Peak as those are the devices Alexandre could repro with.
Assignee

Comment 20

5 years ago
(In reply to Botond Ballo [:botond] from comment #19)
> Still doesn't repro unfortunately. I'll try to get a hold of an Inari, Nexus
> S, or Peak as those are the devices Alexandre could repro with.

I built B2G for Nexus S, and I am finally able to reproduce the bug on it, and I can confirm that it only happens with APZC enabled.

To debug this, I need to better understand how the keyboard works. What is the mechanism for activating and de-activating it? How is this mechanism triggered?
(In reply to Botond Ballo [:botond] from comment #20)
> (In reply to Botond Ballo [:botond] from comment #19)
> 
> To debug this, I need to better understand how the keyboard works. What is
> the mechanism for activating and de-activating it? How is this mechanism
> triggered?

The keyboard is activated when an input field got the focus (see the events in http://mxr.mozilla.org/mozilla-central/source/dom/inputmethod/forms.js#183) and is removed when this element is blurred. 

Since app are resized when the keyboard is shown a dumb theory could be that the layer tree takes some time to update and then the touch is dispatched to a different target than the keyboard. As a result something else than the text field is focused and the keyboard is removed.

Botond don't hesitate to ping me on IRC if you need more infos.
Assignee

Comment 22

5 years ago
I'm still debugging this, just wanted to document my progress: I've traced the cause of the keyboard being hidden to a 'mozvisibilitychange' event fired by Gecko in the keyboard app, which is triggered by a 'browser-element-api:call' 'set-input-method-active' IPC message from the parent process, which is ultimately triggered by Gaia code in the system app, specifically https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/keyboard_manager.js#L732 (which is in turn called from https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/keyboard_manager.js#L621). As a next step I will look into why this is happening with APZC enabled but not without.
Duplicate of this bug: 958131
blocking-b2g: 1.3? → 1.3+
Assignee

Comment 24

5 years ago
So after a lot of debugging (some of which didn't end up being relevant, sorry), it turns out the problem is very simple: the keyboard app does not have APZ turned on.

APZ is enabled for an iframe by setting the 'mozasyncpanzoom' attribute. For app iframes, this is done here [1]. The keyboard app's iframe, however, is created elsewhere [2], and 'mozasyncpanzoom' is not set on it.

The reason this is a problem is that the keyboard app, which takes up the lower part of the screen, overlaps whatever app is open (e.g. Contacts) which takes up the whole screen. Our event handling implementation does not support this use case. If two apps overlap, they must either both have APZ enabled, or neither, otherwise events can end up going to the wrong app. (That's what happened in this case - a tap event intended for the keyboard app was being routed to the app behind it and that was triggering the hiding of the keyboard.)

To fix this, I propose adding the 'mozasyncpanzoom' attribute to the keyboard app (conditioned on the apz.force-enable setting as with other apps). There may also be other apps in the same boat as the keyboard app, I propose doing the same for them.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/browser_frame.js#L71
[2] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/keyboard_manager.js#L365
Assignee

Comment 25

5 years ago
(In reply to Botond Ballo [:botond] from comment #24)
> To fix this, I propose adding the 'mozasyncpanzoom' attribute to the
> keyboard app (conditioned on the apz.force-enable setting as with other
> apps). There may also be other apps in the same boat as the keyboard app, I
> propose doing the same for them.

I tried doing this, but I ran into another problem: now that the keyboard app has APZ enabled, it eats up taps on the app above it, so the keyboard now cannot be dismissed!

It seems the keyboard app's layer takes up the whole screen, even though the keyboard only takes up the lower part of the screen.

It's not clear whether this is a Gecko issue or a Gaia issue. Vivien, would you expect the keyboard app's iframe to take up the whole screen but only have visible content at the bottom? If so, this will probably need to change, at least until bug 928833 is fixed. If not, I will look into why the layer takes up the whole screen.
Flags: needinfo?(21)
In the keyboard manager code (lives in system) we created an iframe with a fixed height (technically the keyboard app reports its height back to system). It should not take up the whole screen.
Flags: needinfo?(21)
(In reply to Botond Ballo [:botond] from comment #25)
> (In reply to Botond Ballo [:botond] from comment #24)
> > To fix this, I propose adding the 'mozasyncpanzoom' attribute to the
> > keyboard app (conditioned on the apz.force-enable setting as with other
> > apps). There may also be other apps in the same boat as the keyboard app, I
> > propose doing the same for them.
> 
> I tried doing this, but I ran into another problem: now that the keyboard
> app has APZ enabled, it eats up taps on the app above it, so the keyboard
> now cannot be dismissed!
> 
> It seems the keyboard app's layer takes up the whole screen, even though the
> keyboard only takes up the lower part of the screen.
> 
> It's not clear whether this is a Gecko issue or a Gaia issue. Vivien, would
> you expect the keyboard app's iframe to take up the whole screen but only
> have visible content at the bottom? If so, this will probably need to
> change, at least until bug 928833 is fixed. If not, I will look into why the
> layer takes up the whole screen.

It is expected that the layer takes up the whole screen. The keyboard frame also have a special property at https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/keyboard_manager.js#L369 that let events goes thought the underlying frame if the actual content beneath the touch is transparent.
I'm curious as to why this bug was so hard to reproduce (comments 16-19) since it should be happening all the time on all devices.

But regardless I wonder if it's feasible to implement a part of bug 928833 to fix this. The steps at [1] sound straightforward enough, as long as we don't bother with checking the dispatch-to-content region. It should still be strictly better than what we have now. But the risk is that the event regions stuff hasn't been exercised before and may be buggy. In particular I don't know if it deals with things like this mozpasspointerevents (first time I'm hearing of this!).

[1] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/Layers.h?rev=59f331fd7867#744
Assignee

Comment 29

5 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #28)
> I'm curious as to why this bug was so hard to reproduce (comments 16-19)
> since it should be happening all the time on all devices.

Let's say the app containing the input field is the Messaging app.

Here is a timeline of what happens when you click the input field:
  Time A - you click the input field
  Time B - the keyboard appears
  Time C - the Messaging app resizes to only take up the top half of the screen

Between B and C, the Messaging app takes up the whole screen, and the Keyboard app takes up the bottom half of the screen (in fact, its layer also takes up the whole screen, but it only has visible content on the bottom half), with the Keyboard app in front and the Messaging app behind.

To experience the bug, you need to press a key between B and C. Since the Keyboard app doesn't have an APZC, APZ hit testing will direct the tap to the Messaging app, and this will trigger the hiding of the keyboard.

If you press the key after C, APZ hit testing just ignores the tap and no hiding happens.

I guess some phones are faster than others and B and C happen too close in time to reproduce the bug on some.
Assignee

Comment 30

5 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #28)
> But regardless I wonder if it's feasible to implement a part of bug 928833
> to fix this. The steps at [1] sound straightforward enough, as long as we
> don't bother with checking the dispatch-to-content region. It should still
> be strictly better than what we have now. But the risk is that the event
> regions stuff hasn't been exercised before and may be buggy. In particular I
> don't know if it deals with things like this mozpasspointerevents (first
> time I'm hearing of this!).

I'm reading up on mozpasspointerevents now. Some relevant bugs are bug 796452 and bug 845169.

In the meantime, to assess the straightforwardness of this proposal, let's start by figuring out the answer to a straightforward question: what coordinate system is Layer::mEventRegions in?
Layer::mEventRegions is in LayoutDevicePixels

Updated

5 years ago

Updated

5 years ago
Blocks: 942267
No longer blocks: t-w732-ix-127
Assignee

Comment 32

5 years ago
A quick test is showing that mEventRegions.mHitTestRegion is currently always empty. Not sure why.

However, I have another fix in mind: the effect of mozpasspointerevents seems to be that layout sets  RenderFrameParent::mTouchRegion to be the actual touch-sensitive region of the iframe (while for iframes without mozpasspointerevents, mTouchRegion is empty). We could therefore use mTouchRegion to constrain our current hit test region if it's not empty. I'll give this a try.
Assignee

Comment 33

5 years ago
Removing old needinfo request which has become irrelevant.
Flags: needinfo?(nicolas.b.pierron)
Assignee

Comment 34

5 years ago
(In reply to Botond Ballo [:botond] from comment #32)
> However, I have another fix in mind: the effect of mozpasspointerevents
> seems to be that layout sets  RenderFrameParent::mTouchRegion to be the
> actual touch-sensitive region of the iframe (while for iframes without
> mozpasspointerevents, mTouchRegion is empty). We could therefore use
> mTouchRegion to constrain our current hit test region if it's not empty.
> I'll give this a try.

That does the trick! The attached Gecko patch, together with a Gaia patch that turns on APZ for the keyboard app (which I'll clean up and post in a moment), fixes this bug as far as I can tell.
Attachment #8361270 - Flags: review?(bugmail.mozilla)
Assignee

Comment 35

5 years ago
Here's the Gaia patch I was using for testing. I'm not sure if it's correct, and I know it duplicates code for checking the APZ setting, but it seems to work.

I saw Vivien's patch in bug 959425 which refactors how the APZ setting is queried, and tried applying that and then using Applications.useAsyncPanZoom, but that seemed not to be working very well, I couldn't even get past the lock screen page. No idea why...
Attachment #8361286 - Flags: feedback?(21)
Assignee

Comment 36

5 years ago
(In reply to Botond Ballo [:botond] from comment #35)
> Created attachment 8361286 [details] [diff] [review]
> [Gaia] Enable APZ for the keyboard app
> 
> Here's the Gaia patch I was using for testing. I'm not sure if it's correct,
> and I know it duplicates code for checking the APZ setting, but it seems to
> work.
> 
> I saw Vivien's patch in bug 959425 which refactors how the APZ setting is
> queried, and tried applying that and then using
> Applications.useAsyncPanZoom, but that seemed not to be working very well, I
> couldn't even get past the lock screen page. No idea why...

One potential problem with this patch is that I see the following in logcat:

E/GeckoConsole(   77): [JavaScript Error: "TypeError: this is undefined" {file: "app://system.gaiamobile.org/js/keyboard_manager.js" line: 170}]

It doesn't seem to cause any problems when I was testing it, but it might indicate something wrong nonetheless.
Comment on attachment 8361270 [details] [diff] [review]
[Gecko] Have APZ hit testing respect 'mozpasspointerevents'

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

Couple of points below, but otherwise looks reasonable. Minusing until comments are addressed.

::: gfx/layers/composite/APZCTreeManager.cpp
@@ +188,5 @@
> +          visible = touchSensitiveRegion
> +                  * container->GetFrameMetrics().LayersPixelsPerCSSPixel()
> +                  * LayerToScreenScale(1.0);
> +        } else {
> +          visible = ScreenRect(container->GetFrameMetrics().mCompositionBounds);

I think here we should always use mCompositionBounds as before, but if there is a TouchSensitiveRegion available, then we should intersect the mCompositionBounds with the TouchSensitiveRegion and use that intersection.

The problem with the way the code is now is that all of the frames inside a given process (which will all share the same GeckoCC) will end up with the same hit test region. Therefore, the innermost subframe will basically grab all the events for the entire area of the app.

::: layout/ipc/RenderFrameParent.cpp
@@ +1129,5 @@
>  
> +nsRegion
> +RenderFrameParent::GetTouchRegion() const
> +{
> +  return mTouchRegion;

Doesn't look like this is used anywhere, so I don't think it's needed.
Attachment #8361270 - Flags: review?(bugmail.mozilla) → review-
Assignee

Comment 38

5 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #37)
> Comment on attachment 8361270 [details] [diff] [review]
> [Gecko] Have APZ hit testing respect 'mozpasspointerevents'
> 
> Review of attachment 8361270 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Couple of points below, but otherwise looks reasonable. Minusing until
> comments are addressed.
> 
> ::: gfx/layers/composite/APZCTreeManager.cpp
> @@ +188,5 @@
> > +          visible = touchSensitiveRegion
> > +                  * container->GetFrameMetrics().LayersPixelsPerCSSPixel()
> > +                  * LayerToScreenScale(1.0);
> > +        } else {
> > +          visible = ScreenRect(container->GetFrameMetrics().mCompositionBounds);
> 
> I think here we should always use mCompositionBounds as before, but if there
> is a TouchSensitiveRegion available, then we should intersect the
> mCompositionBounds with the TouchSensitiveRegion and use that intersection.
> 
> The problem with the way the code is now is that all of the frames inside a
> given process (which will all share the same GeckoCC) will end up with the
> same hit test region. Therefore, the innermost subframe will basically grab
> all the events for the entire area of the app.

Good point, I should have thought of that!

> ::: layout/ipc/RenderFrameParent.cpp
> @@ +1129,5 @@
> >  
> > +nsRegion
> > +RenderFrameParent::GetTouchRegion() const
> > +{
> > +  return mTouchRegion;
> 
> Doesn't look like this is used anywhere, so I don't think it's needed.

That was a remnant from a previous implementation attempt. I removed it.

Updated patch to address review comments.
Attachment #8361270 - Attachment is obsolete: true
Attachment #8361328 - Flags: review?(bugmail.mozilla)
Attachment #8361328 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8361286 [details] [diff] [review]
[Gaia] Enable APZ for the keyboard app

I'm going to check-in https://bug959425.bugzilla.mozilla.org/attachment.cgi?id=8361181 for an other bug.

Instead of having each modules listen for apzc changes you just need to do a:

if (Applications.useAsyncPanZoom) {
  keyboard.setAttribute('mozasyncpanzoom', 'true');
}

and that's it!
Attachment #8361286 - Flags: feedback?(21)
(In reply to Botond Ballo [:botond] from comment #35)
> Created attachment 8361286 [details] [diff] [review]
> [Gaia] Enable APZ for the keyboard app
> 
> Here's the Gaia patch I was using for testing. I'm not sure if it's correct,
> and I know it duplicates code for checking the APZ setting, but it seems to
> work.
> 
> I saw Vivien's patch in bug 959425 which refactors how the APZ setting is
> queried, and tried applying that and then using
> Applications.useAsyncPanZoom, but that seemed not to be working very well, I
> couldn't even get past the lock screen page. No idea why...

Let me know if you still see the issue with the patch from bug 959425, it works well for me.
(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #41)
> (In reply to Botond Ballo [:botond] from comment #35)
> > Created attachment 8361286 [details] [diff] [review]
> > [Gaia] Enable APZ for the keyboard app
> > 
> > Here's the Gaia patch I was using for testing. I'm not sure if it's correct,
> > and I know it duplicates code for checking the APZ setting, but it seems to
> > work.
> > 
> > I saw Vivien's patch in bug 959425 which refactors how the APZ setting is
> > queried, and tried applying that and then using
> > Applications.useAsyncPanZoom, but that seemed not to be working very well, I
> > couldn't even get past the lock screen page. No idea why...
> 
> Let me know if you still see the issue with the patch from bug 959425, it
> works well for me.

With the latest patch on bug 961047 you won't need the Gaia patch at all. Even better.
Assignee

Comment 43

5 years ago
landing
Gecko patch is landing:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dbe8147a7981

Marking as [leave open] until bug 961047 lands as well and we can verify that the issue is fixed (I've tested with the bug 961047 patches locally).
Whiteboard: [leave open]
Assignee

Comment 44

5 years ago
Comment on attachment 8361286 [details] [diff] [review]
[Gaia] Enable APZ for the keyboard app

Obsoleted by patches in bug 9610947.
Attachment #8361286 - Attachment is obsolete: true
Assignee

Updated

5 years ago
No longer blocks: 961047
Depends on: 961047

Comment 46

5 years ago
This is holding up multiple bugs.  Needs to be resolved ASAP please.
It will be once bug 961047 lands.
No longer blocks: 959414
Duplicate of this bug: 959414
Bug 961047 has landed. Marking as resolved fixed.
OS: Linux → Gonk (Firefox OS)
Hardware: x86_64 → ARM
Target Milestone: --- → mozilla28
Target Milestone: mozilla28 → mozilla29
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [leave open]
This bug is confusing, but the patch here needed landing on aurora, so I landed it:

https://hg.mozilla.org/releases/mozilla-aurora/rev/7ab536f59d74

Comment 51

5 years ago
My bug 959414 has been resolved duped to this bug but is still present in yesterday's Buri 1.3 build. Should I reopen bug 959414 or how would you like me to move forward? 

Environmental Variables
Device: Buri v 1.3.0 Mozilla RIL
Build ID: 20140210004002
Gecko: https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/9c9382f433c0
Gaia: 5c8416fb1ea4a27f172ee6386ab3c19135448506
Platform Version: 28.0
Firmware Version: v1.2-device.cfg
Yes, please un-dupe bug 959414 and we can look into it.
You need to log in before you can comment on or make changes to this bug.