1853078 - window.innerHeight is 0 in window.onload when page is opened in the background on Android

Status: NEW

(GeckoView :: General, defect, P2)

Description

[Botond Ballo [:botond]]

Steps to reproduce

1. In Firefox for Android, open https://theres-waldo.github.io/mozilla-testcases/bug1788530.html
2. Long tap on the link
3. In the popup that comes up, select "Open link in new tab"
4. Switch to the newly opened tab
5. Observe the value of window.innerHeight as printed in the page's window.onload handler

Expected results

The value of window.innerHeight reflects the height of the viewport (the layout viewport to be precise, though on this page it's the same as the visual viewport).

This is the behaviour observed in Chrome.

It's also the behaviour observed in Firefox if the page is opened in the foreground rather than the background.

Actual results

The value of window.innerHeight is 0.

Additional notes

• The value of document.body.offsetHeight in window.onload is also higher (4098 on my device) than expected, and than the value when the page is opened in the foreground (1242 on my device).
• This appears to be the cause of bug 1788530, which has high performance impact. The reduced testcase I linked to was originally posted in bug 1788530 comment 7.

Comment 1

[Botond Ballo [:botond]]

I think this should be an S2 because it blocks an S2.

Comment 2

[Boris Chiou [:boris]]

Right. Change to S2.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Though I am still in the middle of the way tracking down what's going on there, I suspect this is a GeckoView issue. What I've tracked down so far is, while the new document is in a background tab, the docshell's mBounds doesn't have a meaningful bounds, it's zero size, thus window.innerHeight is also zeo.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Moving into GeckoView:Core component. I think this issue comes from the difference between desktops and GeckoView.

On GeckoView surprisingly we have a nsWindow instance for each tab rather than each browser window, thus when a new tab opens in background, we do create a new instance of nsWindow. And from the perspective of layout to get the proper window.innerHeight value needs that this onBoundsChanged() needs to be called with the proper value. But during the tab is in the background state, it never gets called.

Ideally we should avoid generating nsWindow instance for each tab, but I am not sure whether it's reasonable or not since I don't know the reason why we didn't take the approach.

There may be some workarounds just like the onBoundsChanged() gets called even in background. CCing a couple of people who might have some ideas to do the workarounds or might know the feasibility of the ideal state of nsWindow.

Comment 5

[jackyzy823]

Ideally we should avoid generating nsWindow instance for each tab, but I am not sure whether it's reasonable or not since I don't know the reason why we didn't take the approach.

I'm also curious about the reason of the design, some reference:

Quote from https://bugzilla.mozilla.org/show_bug.cgi?id=1613227#c1

Having 1 tab = 1 window is pretty central in GeckoView's design, so it's not something we can easily change. We can use this bug to reduce the overhead of creating a new window.

---- Updated at 20231108 ----
More context: https://bugzilla.mozilla.org/show_bug.cgi?id=1584252#c9

Comment 6

[Cathy Lu [:calu]]

Thanks for the helpful testcase, Botond.

GeckoView is built on the assumption that the embedder may not be a browser and each GeckoSession represents a website instance, and so a window with a single tab. There's more information in the architecture docs. Currently there aren't any plans to change this.

It looks like onBoundsChanged() is called here, but the height stored, mHeight, is already 0. Can we update the height once a background tab is in foreground? Do you know where that is happening?

Comment 7

[Hiroyuki Ikezoe (:hiro)]

A somewhat related bit I know of is isActive in CanonicalBrowsingContext.

That's being said, I think we shouldn't do such GeckoView specific tweaks in Gecko, given that GeckoView changes foreground/background tabs, thus GeckoView just can forcibly invoke onBoundsChanged at that time?

Comment 8

[Cathy Lu [:calu]]

Similarly, GeckoView setActive calls GeckoSession setActive when a GeckoView or tab becomes active/in the foreground display. I tried invoking onWindowBoundsChanged there but it has no effect on window.innerHeight.

Comment 9

[Hiroyuki Ikezoe (:hiro)]

I am sorry I was confused. A solution for the test case in comment 0 is to invoke onBoundsChanged() with the proper bounds metrics when a new GeckoSession is created before onload event happens, not when making it a foreground tab.

Comment 10

[Hiroyuki Ikezoe (:hiro)]

It looks like more or less issues in between Fenix (android-components) and GeckoView.

In GeckoView:

1. The GeckoView class is the class maintaining the application window size
2. Each GeckoSession doesn't know the size unless the relationship between GeckoSession and GeckoView is established by a GeckoView.setSession call

In Fenix (android-components):

1. GeckoView.setSession gets called in render() which looks like it's invoked when the session becomes foreground?

Comment 11

[Cathy Lu [:calu]]

In GeckoView.setSession called in render(), the display is acquired and in onGlobalLayout, I logged the width and height with GeckoView.this.mSurfaceWrapper.getView().getWidth(), but am getting consistent values for both foreground and background tabs put into foreground, for example: Width=1080, Height=2125.