Closed
Bug 1233468
Opened 10 years ago
Closed 9 years ago
Canvas is not drawn at the correct viewport, and changes size after refresh and/or pan/zoom
Categories
(Core :: Layout, defect)
Tracking
()
RESOLVED
FIXED
mozilla46
People
(Reporter: padenot, Assigned: kats)
References
Details
Attachments
(1 file)
5.16 KB,
patch
|
tnikkel
:
review+
|
Details | Diff | Splinter Review |
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•10 years ago
|
||
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
Assignee | ||
Comment 2•10 years ago
|
||
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
Assignee | ||
Comment 3•9 years ago
|
||
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.
Assignee | ||
Comment 4•9 years ago
|
||
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)
Assignee | ||
Updated•9 years ago
|
Comment 5•9 years ago
|
||
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+
Assignee | ||
Comment 6•9 years ago
|
||
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.
Assignee | ||
Comment 7•9 years ago
|
||
Try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=8ae5365a6c25 just to be safe.
Comment 9•9 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in
before you can comment on or make changes to this bug.
Description
•