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

RESOLVED FIXED in Firefox 46

Status

()

Core
Layout
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: padenot, Assigned: kats)

Tracking

(Depends on: 1 bug)

45 Branch
mozilla46
Points:
---

Firefox Tracking Flags

(firefox45 affected, firefox46 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
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.
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
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.
Assignee: nobody → bugmail.mozilla
Depends on: 1126346
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.
Created attachment 8706584 [details] [diff] [review]
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).
Attachment #8706584 - Flags: review?(tnikkel)
status-firefox45: --- → affected
status-firefox46: --- → affected
Component: Canvas: 2D → Layout
Comment on attachment 8706584 [details] [diff] [review]
Patch

Seems fine.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) 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?
Attachment #8706584 - Flags: review?(tnikkel) → review+
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 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a0355cdb6bdb
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox46: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.