User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:28.0) Gecko/20100101 Firefox/28.0 Waterfox/28.0 (Beta/Release) Build ID: 20140331235340 Steps to reproduce: When i enable title bar on Firefox 29,and i search for example with google and set google to open pages in new window,the pages are not open maximized like before,if i disable title bar it's working. Expected results: To open maximized like before.

Yes but i don't know if the problem is from title bar. Let's see the answer.

The problem is when it's enabled not disabled.

I mean when Title Bar is enabled.

(In reply to Black_Ps` from comment #4) > I mean when Title Bar is enabled. Right, which means that the tabs are not in the titlebar. :-)

Are you going to fix it? I need to have the title bar enabled for the "hide tab bar with one tab" addon (and because I like my title bar)...

(In reply to Novgorod from comment #6) > Are you going to fix it? I need to have the title bar enabled for the "hide > tab bar with one tab" addon (and because I like my title bar)... Are we generally going to fix it? I'm fairly certain we will. How quickly and/or am I personally going to be fixing it? I don't know. I've put it in our backlog, so it will get /some/ level of attention, at least.

we are having this issue too - hyperlink with target="_blank" does not open new window as maximized

Gavin, what do I do if I think this is serious enough that it should be on our prio backlog?

If you think you need to take it now, just do that, and then give it a point estimate and needinfo Marco to add it to the current iteration. If you think we should add it for the next iteration, just mark it backlog+ and let dolske or I know you think it should be prioritized for the next iteration.

(In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #10) > If you think you need to take it now, just do that, and then give it a point > estimate and needinfo Marco to add it to the current iteration. > > If you think we should add it for the next iteration, just mark it backlog+ > and let dolske or I know you think it should be prioritized for the next > iteration. It needs investigation, and my plate is pretty full, plus I'm not 100% sure where to start (although a regression window would be a helpful beginning, I suppose), so I'd want to do the latter. Consider this comment a "letting you know" :-)

Hi, just wondering if this is still in the lineup for fixing?

I expect this is the result of the same underlying issue as bug 1063121, which we're actively working on.

(In reply to :Gijs Kruitbosch from comment #13) > I expect this is the result of the same underlying issue as bug 1063121, > which we're actively working on. Bug 1063121 didn't fix this. :-( Gavin, can I renom this for backlog for next iteration?

I just wanted to add that this bug also affects windows XP 64 as in the description says "x86 Windows XP". I can reproduce it in firefox 33 and in firefox 34 beta 8.

"Nom for backlog" means "add to the bottom of https://docs.google.com/a/mozilla.com/spreadsheets/d/16zM07MpI4jXHH1-ZNFm61erlRJmAqvX6266GJm-Pr20/edit?pli=1#gid=0". As usual, you don't need things to be prioritized by me to take them.

Hi when will this be fixed??

(In reply to Joako from comment #17) > Hi when will this be fixed?? When someone writes a patch. Unless someone outside the core Firefox Desktop team steps up to investigate the issue and/or write the patch, there's no clear timeline for this, though I've nominated it for inclusion in the work done by the desktop team for Firefox 38 (which doesn't necessarily mean it'd get accepted to be picked up then, and if it did, that 38 would be the first release with the fix - it could be either sooner or later, depending on how complex the fix is and so on). What would already be helpful is if someone narrowed down the regression range of when this broke using mozregression and the UX repository (assuming this broke with the Australis merge, which may or may not be the case in itself... ).

I've run through mozregression to isolate the range. It does appear to be the Australis release. It's my first time using this utility so apologies if I have made an error. This is what I received from running the command 'mozregression --good-release 27 --bad-release 29' on a Windows 7 Pro x86 machine. 7:20.29 LOG: MainThread Bisector INFO Last good revision: a475f94bb1b1 7:20.29 LOG: MainThread Bisector INFO First bad revision: f2adb62d07eb 7:20.29 LOG: MainThread Bisector INFO Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a475f94bb1b1&tochange=f2adb62d07eb For each test I would open about:config set browser.tabs.drawInTitlebar to false set browser.link.open_newwindow to 2 browse to http://jsbin.com/yobatehube/1/edit and try the test link. Hope it helps.

I forgot to add that the very first thing I would do is to maximize the Firefox window. Then I would open about:config.

(In reply to ron from comment #19) > I've run through mozregression to isolate the range. It does appear to be > the Australis release. It's my first time using this utility so apologies > if I have made an error. This is what I received from running the command > 'mozregression --good-release 27 --bad-release 29' on a Windows 7 Pro x86 > machine. > > 7:20.29 LOG: MainThread Bisector INFO Last good revision: a475f94bb1b1 > 7:20.29 LOG: MainThread Bisector INFO First bad revision: f2adb62d07eb > 7:20.29 LOG: MainThread Bisector INFO Pushlog: > http://hg.mozilla.org/mozilla-central/ > pushloghtml?fromchange=a475f94bb1b1&tochange=f2adb62d07eb > > > For each test I would > > open about:config > set browser.tabs.drawInTitlebar to false > set browser.link.open_newwindow to 2 > > browse to http://jsbin.com/yobatehube/1/edit and try the test link. > > Hope it helps. This means it broke in the Australis merge, but it doesn't help in figuring out which part of Australis broke this (as you can see, there are a *lot* of changes in that regression range). I'll have a look to see if I have time to run that regression analysis myself later today.

It seems what's happening here is: 1) the JS calls updateTitlebarDisplay 2) this removes the chromemargin attribute because drawintitlebar is false. This is required because this attributed is there by default. 3) this calls into C++ code, specifically SetNonClientMargins. Let's pause here for a second: at this point, the XUL, C++ and everything else agree that the window is maximized. 4) SetNonClientMargins gets called with a "reset" value (-1,-1,-1,-1), which means it calls ResetLayout() to "force a reflow of content based on the new client dimensions" 5) this calls: // This will trigger a frame changed event, triggering // nc calc size and a sizemode gecko event. SetWindowPos(mWnd, 0, 0, 0, 0, 0, SWP_FRAMECHANGED|SWP_NOACTIVATE|SWP_NOMOVE| SWP_NOOWNERZORDER|SWP_NOSIZE|SWP_NOZORDER); 6) this triggers windows sending a WM_WINDOWPOSCHANGED message 7) this message gets here: http://hg.mozilla.org/mozilla-central/annotate/ec1351f9bc58/widget/windows/nsWindow.cpp#l5882 // Handle window size mode changes if (wp->flags & SWP_FRAMECHANGED && mSizeMode != nsSizeMode_Fullscreen) { // Bug 566135 - Windows theme code calls show window on SW_SHOWMINIMIZED // windows when fullscreen games disable desktop composition. If we're // minimized and not being activated, ignore the event and let windows // handle it. if (mSizeMode == nsSizeMode_Minimized && (wp->flags & SWP_NOACTIVATE)) return; WINDOWPLACEMENT pl; pl.length = sizeof(pl); ::GetWindowPlacement(mWnd, &pl); // Windows has just changed the size mode of this window. The call to // SizeModeChanged will trigger a call into SetSizeMode where we will // set the min/max window state again or for nsSizeMode_Normal, call // SetWindow with a parameter of SW_RESTORE. There's no need however as // this window's mode has already changed. Updating mSizeMode here // insures the SetSizeMode call is a no-op. Addresses a bug on Win7 related // to window docking. (bug 489258) if (pl.showCmd == SW_SHOWMAXIMIZED) mSizeMode = (mFullscreenMode ? nsSizeMode_Fullscreen : nsSizeMode_Maximized); else if (pl.showCmd == SW_SHOWMINIMIZED) mSizeMode = nsSizeMode_Minimized; else if (mFullscreenMode) mSizeMode = nsSizeMode_Fullscreen; else mSizeMode = nsSizeMode_Normal; which sets the window's mSizeMode. Now, I don't know if the SetWindowPos call really changes the sizemode and we just need to deal with that and set it back afterwards, or if it doesn't in this case and this code is just wrong in assuming that that will always be the case. The MSDN docs (if I'm reading them right) do seem to indicate that it is necessary to call SetWindowPos when you change the NC area. In any case, this code sets the size mode to "normal" and from that point on we've lost (everything gets updated to "normal window size now!"). In summary, it seems that calling SetNonClientMargins *after* the sizemode has settled but *before* the window is visible (see below) is currently a Bad Idea. I'm not sure what the best way to fix this is. Ideas include: 1) make the C++ code not set mSizeMode in this specific case by keeping track of why we're firing SetWindowPos. I don't know enough about the win32 api to know if this is possible or if at the point in step 7 windows already really wants to make this a normal-sized window. 2) make the C++ code keep track of this from SetNonClientMargins and re-set the sizemode back to what it was 3) change the XUL persistence code to persist the clientmargins attribute on Windows and the native code (!) that calls nsWindow with this info to call SetNonClientMargins before it updates the window with the new sizemode I don't know this code well enough to decide what is best. Jim, thoughts? Oh, some final info: in the ctrl-n case (ie opening a new browser window using ctrl-n), which does work, it seems we (for whatever reason) don't get around to sending the updateTitlebarDisplay thing until after the window has appeared in the taskbar, at which point the message we get in the equivalent of step 7 no longer wants to resize the window to 'normal'.

jsbin tries to defeat window.open calls and so the testcase didn't work for me. I stuck in a simple data:text/html one instead.

What are the steps to reproduce this bug? When I click on the link in the testcase, it opens a new tab.

(In reply to Jim Mathies [:jimm] from comment #25) > What are the steps to reproduce this bug? When I click on the link in the > testcase, it opens a new tab. See comment 19. Turn off opening _blank links in a new tab in about:config / about:preferences, and turn on the normal Windows titlebar (either in about:config or about:customizing).

WFM on win7. STR: 1) set browser.tabs.drawInTitlebar = false 2) set browser.link.open_newwindow = 2 3) restart 4) open test case here 5) set window to maximized 5) click on the 'Click' link in the test case result: a new, mimimalist, chrome window opens maximized on the desktop on top of the previously maximized window. Wondering what I'm missing here? FWIW - this is an incredibly obscure bug, we should try to avoid mucking around in that widget code for this if possible - every time we tweak it we fix one bug and introduce five.

I created a ~2:00 demonstration video of the bug. https://youtu.be/i-mm324sNT8 The test URL in the header of this page has been truncated so I used jsbin.

Gijs: We are cleaning items left in an untriage state - can you please provide assistance... This appears to be Core: XUL related, Perhaps it can also be closed. I have confirmed fix Version 43.0.4 User Agent Mozilla/5.0 (Windows NT 5.1; rv:43.0) Gecko/20100101 Firefox/43.0 Version 46.0a1 User Agent Mozilla/5.0 (Windows NT 5.1; rv:46.0) Gecko/20100101 Firefox/46.0

Please let me know if I am not being helpful. This bug does not appear fixed in 46.0a1 (2016-01-07) on my platform (Win 7). I've created screenshots of the bug in this version. http://imgur.com/a/toDVg The images are of the following: 0 - (not pictured) Maximize the Firefox window 1 - Version 2 - Disable the 'Open new windows in a new tab instead' option 3 - Enable the 'Title Bar' 4 - Test site - in this case http://www.w3schools.com/tags/att_a_target.asp 5 - New window does not open maximized If the Title Bar is changed back to disabled then the new window in image 5 opens maximized.

(In reply to Michelle Funches - QA from comment #29) > Gijs: We are cleaning items left in an untriage state - can you please > provide assistance... > This appears to be Core: XUL related, Perhaps it can also be closed. I have > confirmed fix > Version 43.0.4 > User Agent Mozilla/5.0 (Windows NT 5.1; rv:43.0) Gecko/20100101 Firefox/43.0 > Version 46.0a1 > User Agent Mozilla/5.0 (Windows NT 5.1; rv:46.0) Gecko/20100101 Firefox/46.0 I'm fairly sure this is either Fx::somethingorother or core::widget win32, but it's definitely still valid as per comment #30. I would have liked to look at this today but I was too busy. I'll try again on Monday. Leaving needinfo.

I can still reproduce this on both win8 and win7. Going to see quickly if there's a sensible way to avoid this issue from the JS side.

Review commit: https://reviewboard.mozilla.org/r/30549/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/30549/

Comment on attachment 8707007 [details] MozReview Request: Bug 1005098 - don't update titlebar attributes before the window has been activated, r?MattN Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30549/diff/1-2/

FWIW, it might be interesting to write a test for this, which I haven't done yet, but should be possible.

Comment on attachment 8707007 [details] MozReview Request: Bug 1005098 - don't update titlebar attributes before the window has been activated, r?MattN https://reviewboard.mozilla.org/r/30549/#review27921 ::: browser/base/content/browser-tabsintitlebar.js:270 (Diff revision 2) > + if (AppConstants.platform == "win" && Services.focus.activeWindow != window) { > + if (!TabsInTitlebar._titlebarUpdateListener) { > + TabsInTitlebar._titlebarUpdateListener = e => { > + window.removeEventListener("activate", TabsInTitlebar._titlebarUpdateListener); > + updateTitlebarDisplay(); > + }; > + window.addEventListener("activate", TabsInTitlebar._titlebarUpdateListener); > + } > + return; > + } > + TabsInTitlebar.haveUpdatedTitlebarDisplay = true; This is quite complicated to understand with the nesting, three variables (`haveUpdatedTitlebarDisplay`, `_titlebarUpdateListener`, and `Services.focus.activeWindow != window`), and the `return` but I think this makes sense. It seems to work on Windows and is basically a no-op on non-Windows.

Backout: https://hg.mozilla.org/integration/fx-team/rev/020242b2d903

(In reply to Pulsebot from comment #38) > Backout: > https://hg.mozilla.org/integration/fx-team/rev/020242b2d903 So the width of some tests here no longer was what we expected it to be (only happened on win7 - winxp and win8 were fine): https://treeherder.mozilla.org/logviewer.html#?job_id=6667719&repo=fx-team https://treeherder.mozilla.org/logviewer.html#?job_id=6668347&repo=fx-team and this test broke because of height differences, too (win7 and win8, not winxp): https://treeherder.mozilla.org/logviewer.html#?job_id=6670394&repo=fx-team I *think* this could be fixed by making the whole sha-doodle dependent on whether TabsInTitlebar.enabled isn't true, but that seems hacky as ... I'm also a little suspicious about how we have no real test coverage for that case and so maybe there are issues in that case, too, but we just don't find them. I did try to run the test in question with TabsInTitlebar disabled, and it passed both with and without this change... Matt, thoughts?

(In reply to :Gijs Kruitbosch from comment #39) > So the width of some tests here no longer was what we expected it to be > (only happened on win7 - winxp and win8 were fine): > > https://treeherder.mozilla.org/logviewer.html#?job_id=6667719&repo=fx-team > https://treeherder.mozilla.org/logviewer.html#?job_id=6668347&repo=fx-team FYI this fails for me locally on Windows 8.1 Pro in the same way (and passes before your patch). > and this test broke because of height differences, too (win7 and win8, not > winxp): > > https://treeherder.mozilla.org/logviewer.html#?job_id=6670394&repo=fx-team Likewise for this test. > I *think* this could be fixed by making the whole sha-doodle dependent on > whether TabsInTitlebar.enabled isn't true, but that seems hacky as ... Made what dependent on "TabsInTitlebar.enabled isn't true"? These tests or the code you added? > I'm also a little suspicious about how we have no real test coverage for > that case and so maybe there are issues in that case, too, but we just don't > find them. I did try to run the test in question with TabsInTitlebar > disabled, and it passed both with and without this change... Sorry, which case? Have you convinced yourself that the expected test output is actually correct? Perhaps the test is wrong? Feel free to focus on one test file at a time to simplify. I think maybe we should talk on Vidyo tomorrow as I'm not understanding the whole situation so ping me tomorrow.

(In reply to Matthew N. [:MattN] from comment #40) > (In reply to :Gijs Kruitbosch from comment #39) > > So the width of some tests here no longer was what we expected it to be > > (only happened on win7 - winxp and win8 were fine): > > > > https://treeherder.mozilla.org/logviewer.html#?job_id=6667719&repo=fx-team > > https://treeherder.mozilla.org/logviewer.html#?job_id=6668347&repo=fx-team > > FYI this fails for me locally on Windows 8.1 Pro in the same way (and passes > before your patch). Both? They're the same except for the extension (one is .html and one is .xul), and I remember I ran one when this broke when I landed, and it passed on my win8.1 pro machine... > > and this test broke because of height differences, too (win7 and win8, not > > winxp): > > > > https://treeherder.mozilla.org/logviewer.html#?job_id=6670394&repo=fx-team > > Likewise for this test. Yes, I can reproduce this, too. > > I *think* this could be fixed by making the whole sha-doodle dependent on > > whether TabsInTitlebar.enabled isn't true, but that seems hacky as ... > > Made what dependent on "TabsInTitlebar.enabled isn't true"? These tests or > the code you added? The code I added. Sorry for being so vague and unclear. :-( > > I'm also a little suspicious about how we have no real test coverage for > > that case and so maybe there are issues in that case, too, but we just don't > > find them. I did try to run the test in question with TabsInTitlebar > > disabled, and it passed both with and without this change... > > Sorry, which case? The !TabsInTitlebar.enabled case. I don't think any of our window opening tests verify what happens when that is set. > Have you convinced yourself that the expected test output is actually > correct? Perhaps the test is wrong? Feel free to focus on one test file at a > time to simplify. Not in-depth, but considering the bug as described here, and the cause being updating the chrome margins at the "wrong" time, I wouldn't be at all surprised that changing the timing causes minor changes in how we compute widths/heights of windows, especially to do with window border sizes (where I think at least on win7 we use smaller sizes than is the default, but I could be misremembering that). If we make such changes at a later/earlier time than we used to, that could explain us being off by a couple of pixels compared to the current case, and also why some of these tests were intermittent before and already have a ~2px fault tolerance. > I think maybe we should talk on Vidyo tomorrow as I'm not understanding the > whole situation so ping me tomorrow. Sure! I'll ping you when you're more likely to be awake. :-)

(In reply to Jim Mathies [:jimm] from comment #27) > FWIW - this is an incredibly obscure bug, we should try to avoid mucking > around in that widget code for this if possible - every time we tweak it we > fix one bug and introduce five. This sounds like a rather unhealthy strategy. The situation is only going to get worse if we add platform-specific hacks as Gijs did in his patch, because then we have obscure front-end code interacting with obscure widget code and nobody dares to touch anything anymore.

Not actively working on this.