1682629 - Enable APZ in scrollable popups without remote content

Status: RESOLVED FIXED

(Core :: Panning and Zooming, enhancement, P2)

Description

[Botond Ballo [:botond] [away until Jun 24]]

In bug 1493208, we enabled APZ in popups with remote content.

We'd also like to enable APZ in popups without remote content, but only if they are scrollable (see discussion in bug 1562101 comment 5).

Comment 1

[Botond Ballo [:botond] [away until Jun 24]]

We'll need this to fix an S2 bug, bug 1874746.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1682629 - Set apz.popups.enabled to false for browser_partial_prerender_animation_in_popup.js. r?botond

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1682629 - Drop |aWindow| argument for flushApzRepaints. r?botond

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1682629 - Make flushApzRepaints work for popup window. r?botond

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1682629 - Make sendNativeTouchPoint work for popup window. r?botond

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1682629 - Enable apz for Panel/Menu popups. r?botond

Comment 7

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1682629 - Add a browser mochitest to test touch events in browser popup window. r?botond

Comment 8

[Hiroyuki Ikezoe (:hiro)]

The uploaded patches mostly work as expected at least on Linux, but the browser mochitest (D230881) fails on Mac for some reasons. I am going to debug it on Mac. Though on Mac we don't support touch screen at all, so that it's not a big deal I think.

Comment 9

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #8)

The uploaded patches mostly work as expected at least on Linux, but the browser mochitest (D230881) fails on Mac for some reasons. I am going to debug it on Mac. Though on Mac we don't support touch screen at all, so that it's not a big deal I think.

On Mac we don't create the compositor session for popup windows having no remote content, thus we don't even try to create the APZCTreeManager there.

I am going to skip running the browser test on Mac.

Comment 10

[Hiroyuki Ikezoe (:hiro)]

Attachment: A patch to implement SynthesizeNativeTouchPoint for Mac

(In reply to Hiroyuki Ikezoe (:hiro) from comment #9)

(In reply to Hiroyuki Ikezoe (:hiro) from comment #8)

The uploaded patches mostly work as expected at least on Linux, but the browser mochitest (D230881) fails on Mac for some reasons. I am going to debug it on Mac. Though on Mac we don't support touch screen at all, so that it's not a big deal I think.

On Mac we don't create the compositor session for popup windows having no remote content, thus we don't even try to create the APZCTreeManager there.

Note that to make the test work on Mac we also need to implement SynthesizeNativeTouchPoint for popup windows on Mac.

Attaching file is the patch, FWIW.

Comment 11

[Hiroyuki Ikezoe (:hiro)]

One more note about Mac, on Mac the popup window's WindowType is not Popup, it's Child.

Comment 12

[Hiroyuki Ikezoe (:hiro)]

https://treeherder.mozilla.org/jobs?repo=try&revision=3afd05c95e67d26529a5c0c746582f5265c49b55 is a try run including all patches here on Phab.

Comment 13

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1682629 - Make sendNativePenInput work for popup window. r?botond

There's a mochitest, test_bug1728171.html [1] excersizing native pen
events in popup context menus, so we need to make sendNativePenInput
works there before enabling APZ in the popup windows.

[1] https://searchfox.org/mozilla-central/rev/7fb746f0be47ce0135af7bca9fffdb5cd1c4b1d5/dom/events/test/test_bug1728171.html

Comment 14

[Hiroyuki Ikezoe (:hiro)]

Added D231185 in the patch series, it's just for our internal tests, it should impact on real uses.

Comment 15

[Emilio Cobos Álvarez [:emilio]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #11)

One more note about Mac, on Mac the popup window's WindowType is not Popup, it's Child.

Hm? That doesn't sound right. Well, there's both a Popup widget and an nsChildView inside (which is a Child). I guess that's what you mean?

Comment 16

[Hiroyuki Ikezoe (:hiro)]

I don't remember exactly what I checked is probably this mPopupContentView.

Comment 17

[Emilio Cobos Álvarez [:emilio]]

Yeah, that is a Child indeed.

Comment 18

[Botond Ballo [:botond] [away until Jun 24]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #12)

https://treeherder.mozilla.org/jobs?repo=try&revision=3afd05c95e67d26529a5c0c746582f5265c49b55 is a try run including all patches here on Phab.

I tested this Try build on a Linux touchscreen laptop, with the screen resolution set sufficiently low to make the application menu scrollable, and I can confirm that the application menu can be scrolled using touch input.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1a5ae2af03b7
Set apz.popups.enabled to false for browser_partial_prerender_animation_in_popup.js. r=botond
https://hg.mozilla.org/integration/autoland/rev/00b4021a2824
Drop |aWindow| argument for flushApzRepaints. r=botond
https://hg.mozilla.org/integration/autoland/rev/7b9f315333ec
Make flushApzRepaints work for popup window. r=botond
https://hg.mozilla.org/integration/autoland/rev/4855f60214d8
Make sendNativeTouchPoint work for popup window. r=botond
https://hg.mozilla.org/integration/autoland/rev/73b0c2063498
Make sendNativePenInput work for popup window. r=botond
https://hg.mozilla.org/integration/autoland/rev/3b859e813e2f
Enable apz for Panel/Menu popups. r=botond,emilio
https://hg.mozilla.org/integration/autoland/rev/9446114872ee
Add a browser mochitest to test touch events in browser popup window. r=botond

Comment 20

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/1a5ae2af03b7
https://hg.mozilla.org/mozilla-central/rev/00b4021a2824
https://hg.mozilla.org/mozilla-central/rev/7b9f315333ec
https://hg.mozilla.org/mozilla-central/rev/4855f60214d8
https://hg.mozilla.org/mozilla-central/rev/73b0c2063498
https://hg.mozilla.org/mozilla-central/rev/3b859e813e2f
https://hg.mozilla.org/mozilla-central/rev/9446114872ee