Build ID 20180908220614 User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:64.0) Gecko/20100101 Firefox/64.0 This is an annoyance regression bug. Browser size increases if quit browser in maximized mode. Reproducible: always, with new profile. Steps To Reproduce: 1. Switch browser to normal sizemode, if not --- Remember browser size 2. Switch the browser to maximize mode (click maximize button) 3. Quit browser(Alt File > Exit or click x button) 4. Start browser again (The browser is now in maximized mode as expected) 5. Switch browser to normal size mode(click restore button) --- Compare with the browser size at step 1 Actual Results: The size is getting bigger Expected Results: Should be same size. Threre 2 regression at least. #1 Regression window(fails to restore browser sizemode, but correct size in normal sizemode): https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=cad403cafed9b17d1b801837abcd039a0bce82d5&tochange=990a8eb972cd2a734da04ed8d787564752e8c9c7 Regressed by: Bug 1439875 #2 Regression window(browser size of normal sizemode is getting bigger): https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e21a2a57d05dfe3c5ff8ba71131127fa781ffdd0&tochange=f026ead5dbfce9d6530429ac568ecc5544cc9b3b Regressed by: f026ead5dbfc Florian Quèze — Bug 1447719 - Set browser.startup.blankWindow to true on Windows and Linux, r=mconley. :emilio , :florian, The bunch of patch causes the regression, can you please look into this?
Summary: Browser size gradually increases every restart if quit browser in maximized mode. → The browser size will increase with each reboot when you exit the browser in maximized mode.
The size seems to increase by the height of the titlebar and the width of window borders. For windows in normal sizemodes, the size is set using window.resizeTo, specifically to avoid this bug. When the window is first opened in maximized mode, we set the size using the 'height' and 'width' attributes: https://searchfox.org/mozilla-central/rev/9e7995b3c384945740985107a4de601b27904718/browser/components/nsBrowserGlue.js#349-355 Emilio, any idea of how we could avoid this problem when setting the size of a maximized window?
Thanks Emilio! Yes, using that API solves the problem.
Attachment #9008372 - Flags: review?(mconley)
Assignee: nobody → florian
Status: NEW → ASSIGNED
The patch is simple enough that I think we should uplift at least to beta.
Comment on attachment 9008372 [details] [diff] [review] Patch Review of attachment 9008372 [details] [diff] [review]: ----------------------------------------------------------------- Wow, I didn't know those getters existed. Very handy! Yeah, this looks good. Thanks florian and emilio!
Attachment #9008372 - Flags: review?(mconley) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/690dfc98faff convert the size from outer to inner window dimensions when setting the width and height to use when the user leaves the maximized mode, r=mconley.
Comment on attachment 9008372 [details] [diff] [review] Patch Approval Request Comment [Feature/Bug causing the regression]: probably bug 1444525, but the regression only became user visible when enabling the early blank window in bug 1473142. [User impact if declined]: browser window size incorrectly restored when the window was maximized at the time the browser was closed. [Is this code covered by automated tests?]: no, it's unfortunately very hard to cover things that involve a restart with automated tests. [Has the fix been verified in Nightly?]: Not yet. [Needs manual test from QE? If yes, steps to reproduce]: Yes, please. Steps in comment 0. [List of other uplifts needed for the feature/fix]: I'll request uplift in bug 1489214 too and it would make sense to QA both at the same time, but technically they could land separately. [Is the change risky?]: Low risk. [Why is the change risky/not risky?]: Self contained fix that only affects the behavior in the edge case that was broken. [String changes made/needed]: none.
Attachment #9008372 - Flags: approval-mozilla-beta?
Comment on attachment 9008372 [details] [diff] [review] Patch Low risk patch fixing a regression, uplift approved for 63 beta 6, thanks.
Attachment #9008372 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
I can confirm the reproduction of the issue in Nightly v64.0a1 (2018-09-05) and I can confirm the fix in Nightly v64.0a1 (2018-09-13). Awaiting beta build for uplift verification. Thank you.
This issue is now fixed in Beta v63.0b6 on Windows 10. Uplift successful. Thank you!
Comment on attachment 9008372 [details] [diff] [review] Patch approved for 62.0.2 based on bug 1489214 comment 21
Putting the qe-verify+ flag back for verification process.
Backed out from mozilla-release for bustage: https://hg.mozilla.org/releases/mozilla-release/rev/da3cd3cea8c0 Florian, should we also backout bug 1489214 too or are we ok with just that one?
Julien, what made you feel certain that this patch is the one causing the bustage? I couldn't spot it from the logs after a bit of examination, so I'm hoping you can tell us more? :-)
RyanVM ran a set try pushes with various backouts and this one ended up green.
The new API that this patch uses landed in 61 (in bug 1446343) so it's not the problem. My current guess is that the bustage comes from a dependency on the changes from bug 1446940 that landed in 63. If my guess is correct, .docShell.treeOwner (on 63+) needs to be .QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIWebNavigation).QueryInterface(Ci.nsIDocShellTreeItem).treeOwner on 62. (In reply to Julien Cristau [:jcristau] from comment #16) > Backed out from mozilla-release for bustage: Sorry :-(. > Florian, should we also backout bug 1489214 too or are we ok with just that > one? Please keep bug 1489214, that's the one that was causing the most support requests. The reason I requested that we uplift both patches was to have a consistent behavior across all of 62-64 to reduce confusion when dealing with (potential) future bug reports. I'm fine with wontfix here if that's the direction you want to take. That said, RyanVM offered to run the rebased patch through try, so we may have a fixed patch soon.
Looks like that modification works! https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=d95f1020f0f38a96f03bbb1764a97f2bef800ab7
(In reply to Ryan VanderMeulen [:RyanVM] from comment #20) > Looks like that modification works! > https://treeherder.mozilla.org/#/ > jobs?repo=try&group_state=expanded&revision=d95f1020f0f38a96f03bbb1764a97f2be > f800ab7 Julien, should this rebased version land asap, or do you prefer to wontfix this for 62 at this point? (either is fine with me)
Comment on attachment 9008372 [details] [diff] [review] Patch I'll land the rebased fix, thanks.
I have reproduced the issue on Firefox Release v62.0 and I have verified the fix in Firefox Release Candidate v62.0.2 on Windows 10. Uplift successful. Thank you.
You need to log in before you can comment on or make changes to this bug.