Closed
Bug 748495
Opened 13 years ago
Closed 12 years ago
"onresize" called 3 times while loading the page
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 23
People
(Reporter: paul, Assigned: kats)
References
()
Details
Attachments
(1 file)
|
(deleted),
patch
|
cwiiis
:
review+
|
Details | Diff | Splinter Review |
This code:
<!DOCTYPE html>
<meta charset="utf-8">
<title>xxx</title>
<p>Resize should not be called on load</p>
<script>
var i = 0;
window.onresize = function() {
i++;
}
window.setTimeout(function() {
alert("resize called " + i + " times");
}, 2000);
</script>
reports "3" on Firefox Fennec. 0 on stock browser and desktop browsers.
I can give more information form a test I ran:
When it's called three times, it actually has a different width on the first call of onresize. That appears to be a cached version of what the width was previously and in the case of the first loading of the entire it was the double-pixel width (i.e. on a Nexus 4, the available resolution is 768 x 1196, but the browser reports it normally as half that - most smartphones do this - so 384 x 598). So you'll from the following steps, you'll get:
Refresh Page (We're starting in Portrait)
1. 768
2. 384
3. 384
Rotate (Now in landscape)
1. 384
2. 598
3. 598
Rotate (Now in Portrait)
1. 598
2. 384
3. 384
Rotate (Now in landscape)
1. 384
2. 598
3. 598
Refresh (Still in landscape, notice the double pixels)
1. 1196
2. 598
3. 598
Hopefully this will help narrow the issue down.
And some more information:
this 3 calls also occurs on Chrome on Android and on Dolphin, it's 2 calls.
orientationchange works correctly on Chrome but incorrectly on Dolphin (it's always one orientationchange behind on dolphin).
Correction to previous: Dolphin correctly calls resize only once. Chrome calls 3 times like Firefox though
|
Assignee | |
Comment 5•12 years ago
|
||
Two of these calls are expected - see the comment at http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#3462 for why. It looks like the first in your batches of three (i.e. the "cached" one) is wrong and should be eliminated. I can look into why that's happening.
Assignee: nobody → bugmail.mozilla
|
|
Assignee | |
Comment 6•12 years ago
|
||
Btw, could you attach the test case you used to test this? Just so I know we're looking at the same thing.
Flags: needinfo?(donrhummy)
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #5)
> Two of these calls are expected - see the comment at
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/
> browser.js#3462 for why. It looks like the first in your batches of three
> (i.e. the "cached" one) is wrong and should be eliminated. I can look into
> why that's happening.
I see why the browser's internal code requires two calls, but for the web page's code, the only accurate firing of the resize event is that last one. The first one should be hidden since it's apparently "releasing" incorrect information. It's also misleading to the web page as it tells the code there's been two resizings, but there have not. There was only one resizing and then a second "sanity" check masquerading as a resizing event.
Flags: needinfo?(donrhummy)
I simply added this code into a script tag at the bottom of the body tag in an html page:
window.addEventListener( "resize", function() { alert( document.body.clientWidth ); }, false );
|
|
Assignee | |
Updated•12 years ago
|
|
|
Assignee | |
Comment 9•12 years ago
|
||
Something I noticed while debugging this is that triggering an alert during the page load process itself causes the before-first-paint event to get re-dispatched, and triggers an extra resize. So that's bad. Other than that it seems like the resize events are pretty much directly related to us calling setBrowserSize in browser.js, and adding a check to not redo all that work if the size isn't changing should fix most of the problem.
|
|
Assignee | |
Comment 10•12 years ago
|
||
Attachment #737553 -
Flags: review?(chrislord.net)
|
||
Comment 11•12 years ago
|
||
Comment on attachment 737553 [details] [diff] [review]
Guard against calling setCSSViewport multiple times with the same size
Review of attachment 737553 [details] [diff] [review]:
-----------------------------------------------------------------
I saw this before and promptly forgot about it :/ Nice catch, r+ with comment addressed.
::: mobile/android/chrome/content/browser.js
@@ +3540,5 @@
> });
> },
>
> setBrowserSize: function(aWidth, aHeight) {
> + if (this.browserWidth == aWidth && this.browserHeight == aHeight) {
Do we want to use fuzzyEquals or round here? The calculations before this point involve floating point multiplications I think?
Attachment #737553 -
Flags: review?(chrislord.net) → review+
|
|
Assignee | |
Comment 12•12 years ago
|
||
|
|
Assignee | |
Comment 13•12 years ago
|
||
(Landed with fuzzy-equals checks)
|
||
Comment 14•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 23
|
|
Assignee | |
Updated•8 years ago
|
|
||
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•