1233468 - Canvas is not drawn at the correct viewport, and changes size after refresh and/or pan/zoom

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Paul Adenot (:padenot)]

STR:
(1) Load https://paul.cx on Fennec (I'm testing current nightly)
(2) Refresh page
(3) Pan/zoom and refresh the page

Expected:
The full viewport is used each time

Actual:
At (1), only part of the viewport is used by the canvas
At (2), a bigger part of the viewport is used by the canvas
At (3), the whole viewport is used by the canvas

This might well be an APZ issue, I haven't investigated.

Comment 1

[Jamie Nicol [:jnicol]]

Prior to this push, we always used the full width, but not the full height: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=aa51330c9e0982791b7995c303339ee386ea4172&tochange=0720ea24b69a71909620ec6bb12f4ffbfe325d70

Prior to this push, we always just use the same small part of the viewport, and refreshing makes no difference: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=df9cdd1400626740fbc21f636aad8c284f0e9128&tochange=52d6ee1e55c922c78367528e8752fabe64ebab71

After that push I get pretty much the behaviour Paul is describing.

cc Kats

Comment 2

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

Sounds kind of like a race condition that gets tickled different ways after refresh. I can take this but probably won't get a chance to look into it until the new year.

Comment 3

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

So I think the problem here is that the before-first-paint event (which determines the viewport size) is getting fired after the load event (which the page is using to determine the viewport size).

In general the site should probably have a resize listener than resizes the canvas to the new innerWidth/innerHeight. That would cause it to behave more correctly in the face of device rotations and browser window resizings, which it currently doesn't handle. However that would also paper over the real gecko bug in this case.

One possible fix on the gecko side would be to listen for load events and do the same thing that we do for before-first-paint events. As long as our load event listener gets run before the content's we should have the correct innerWidth/innerHeight in place. I'll give that a shot.

Comment 4

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

Attachment: Patch

This does the job. We need to make sure we have the CSS viewport set up before content gets the load event, even if that happens before the before-first-paint event is fired (which is the case here).

Comment 5

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8706584 [details] [diff] [review]
Patch

Seems fine.

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #3)
> As long as our load
> event listener gets run before the content's we should have the correct
> innerWidth/innerHeight in place.

Hmm, is this guaranteed? Is there a way to guarantee it? I don't know how event listeners work in order to answer that. maybe smaug?

Comment 6

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

I think it is guaranteed, because I'm using a capture chrome listener which gets run before everything else. And at any rate it will be be deterministic, so if it worked in my local testing it should work always.

Comment 7

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

Try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=8ae5365a6c25 just to be safe.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a0355cdb6bdb

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a0355cdb6bdb