1650488 - Make dom/html/test/test_fullscreen-api.html pass with apz.allow_zooming=true

Status: RESOLVED FIXED

(Core :: Layout: Scrolling and Overflow, task)

Description

[Kartikaya Gupta (email:[e-mail])]

When apz.allow_zooming is turned on on desktop tests, there's a failure in dom/html/test/test_fullscreen-api.html. Specifically in the top-layer subtest, it fullscreens an element and then dispatches a mouse event to ensure the event target is the fullscreen element, here.

However, it looks like the nsDisplayAsyncZoom has a clip rect on it that is 500x500 CSS pixels, rather than the "fullscreened" size of the element, and so when the displaylist-based hit-testing happens, it never descends into the async zoom item and just returns the root element. This causes the test to fail.

I'm not sure what the correct fix here is. The way fullscreen is implemented is by making the fullscreen element a fixed-position item expanded to all sides, and that is reflected in the display list. But the FixedPosition display items are nested under the AsyncZoom item. So I guess I should look into how the clip is calculated for those items and try and make it work the same way for the AsyncZoom item.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Looked into this more. One line of investigation was that conceptually the fullscreened element is "hoisted out" of the page and therefore is not zoomable (neither Chrome nor FF allow zooming while an element is in fullscreen mode) and so the display list should reflect that. In particular, the nsDisplayPositionFixed items should be outside the nsDisplayAsyncZoom. This can be achieved pretty easily, because the fullscreened elements are added as part of this call and moving that to below the async zoom block does the job. However, there are two problems here.

One is that the set of "top layer" elements includes not just fullscreened elements but modal HTMLDialogElements and we should allow zooming on those (as does Chrome). So this would require separating the two kinds of "top layer" elements in the display list and putting the modal dialog items inside the nsDisplayAsyncZoom while the fullscreened elements go outside.

The other problem is that this causes an assertion failure in AsyncCompositionManager. This happens because this code expects zoomContainer to be an ancestor of layer which it isn't any more. So that can probably be corrected without too much trouble, perhaps by checking for that case in the IsFixedToZoomContainer lambda.

I'm gonna think about this a bit more before committing to a plan.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

I think what I described in comment 1 is the right way to go. At least, I couldn't think of any other option that seemed as reasonable. Certainly changing the clip on the nsDisplayAsyncZoom item seems fraught with peril and does seem as conceptually correct for fullscreen items. The fixes for both of the problems I outlined are also simpler than I originally thought they would be, so that's nice.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

A try push with this approach turned up more problems. There's another assertion in APZ that fails because the fixed-pos items always have a scroll id for the root scrollable layer, and end up outside the async zoom container.

Botond suggested just not having the nsDisplayAsyncZoom item at all in the fullscreen case, and that also seems reasonable. I'll try that and see if I run into problems.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1650488 - Don't build an async zoom container if a document is in fullscreen. r?botond

When a document has a fullscreen element, zooming is disabled, so we don't need
to build the async zoom container. But also, fullscreen is implemented via
hoisting the fullscreen element out of the normal layout flow, and making it
position:fixed. If the nsDisplayAsyncZoom display item is still in the display
list, it has a clip that doesn't account for fullscreen-ness, but still wraps
the nsDisplayPositionFixed items that are rendering the fullscreen elements.
This can result in hit-testing failing on those fullscreen items, because the
nsDisplayAsyncZoom item incorrectly clips away hit-test points. Instead, a
simple solution here is to just skip building the nsDisplayAsyncZoom item
entirely when in a fullscreen state.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/56b372581da9
Don't build an async zoom container if a document is in fullscreen. r=botond

Comment 6

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/56b372581da9