499816 - Minimizing Firefox does not release window focus

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Timothy Pelayo]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1pre) Gecko/20090622 Shiretoko/3.5pre (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1pre) Gecko/20090622 Shiretoko/3.5pre (.NET CLR 3.5.30729)

Minimizing Firefox seems to cause weird behavior with Steam.  It can cause previously closed Steam windows to reopen and sometimes even access parts of the menu of currently open Steam windows.  

Reproducible: Always

Steps to Reproduce:
1.  Click the X to close any Steam windows, leaving behind only the system tray icon for it.
2.  Now minimize Firefox

Actual Results:  
One of the previously closed Steam windows will automatically reopen and gain focus

Expected Results:  
Steam windows should have stayed minimized.

I'm using Vista x64 here.

The first nightly build with this bug was the 2009-05-21-04 nightly (http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2009/05/2009-05-21-04-mozilla-1.9.1/)

Interestingly enough, one of the changes in that build was a fix for a similar problem with Live messenger.    
https://bugzilla.mozilla.org/show_bug.cgi?id=472874


There are several others confirming this bug in the Mozillazine Firefox Builds forum as well as on the Steam forums.
http://forums.mozillazine.org/viewforum.php?f=23
http://forums.steampowered.com/forums/showthread.php?t=895712

I posted about it in the Steam forums with several others confirming the bug and got a response from a Valve employee who believes it is a problem with Firefox.
http://forums.steampowered.com/forums/showpost.php?p=10417467&postcount=71
"Sounds like FF posting windows messages like WM_SETFOCUS or such incorrectly and having them hit whatever window has taken over after it's minimized. Can't really see how it would be our bug from your descriptions, but I'll see if I can repro when I get a chance."

Comment 1

[AnAkkk]

I got the same problem. I'm using XP Pro 32bit here.

Comment 2

[Timothy Pelayo]

I think this is also the cause for little text boxes from Steam getting stuck in the upper left corner of the desktop.  I just noticed it right after minimizing Firefox.  Very annoying...

http://i43.tinypic.com/ncflnc.jpg

Comment 3

[Andrew]

Hi,

I noticed a similar issue with Steam that may help identify the source of this bug.

1) Ensure you have a Firefox window open and visible
2) Ensure your main Steam window (with your games) is visible (mine is in the mini-game list mode).
3) With the main steam window in the foreground, and the firefox window in the background, minimise firefox
4) For me, Steam's "File" menu appears as if I had clicked on it

Comment 4

[Matthew Middleton (:zzxc)]

Confirming the bug based on the number of reports, this has come up on support.mozilla.com as well

Comment 5

[Cobra]

I suspect it has to do with how Firefox deals with memory swap in Windows when it minimizes. In the mean time, you can fix this problem using a Boolean setting.

In Firefox, type "about:config" in the address bar. When you get to the list of settings, right click anywhere and select "New -> Boolean".
For the name type "config.trim_on_minimize" and set it to True. Restart Firefox and the bug is gone.

When Firefox minimizes, this setting now allows Windows to memory swap whatever FF was using from the RAM to HDD and free up that RAM for something else. The downside to this is maximizing FF may take .5secs longer to maximize again as the memory has to be swapped back from the HDD to the RAM. Once Mozilla fixes this, this setting can be either deleted or changed back to false.

Comment 6

[oscar.coleman]

I have this problem in Windows 7 RC.

Please fix it.

http://forums.steampowered.com/forums/showthread.php?t=895712&page=2

Comment 7

[12iceman]

I am also experiencing this bug under Windows Vista 32bit with Firefox 3.5.

Comment 8

[Andrew Roeder]

Recreated this bug with Windows XP x64 and Firefox 3.5

Comment 9

[Masatoshi Kimura [:emk]]

1.9.1 activate patch v.1 attached in bug 472874 doesn't look correct to me.
> -          while (hwndBelow && (!::IsWindowVisible(hwndBelow) ||
> +          while (hwndBelow  && !IsWindowEnabled(hwndBelow) && (!::IsWindowVisible(hwndBelow) ||
                                ::IsIconic(hwndBelow))) {
This will find the enabled Window even if it is minimized. It should be:
          while (hwndBelow  && (!IsWindowEnabled(hwndBelow) || !::IsWindowVisible(hwndBelow) ||
                                ::IsIconic(hwndBelow))) {

Comment 10

[Masatoshi Kimura [:emk]]

Attachment: patch (for trunk)

I've made a trunk patch first because bug 472874 is stuck.

Comment 11

[Masatoshi Kimura [:emk]]

Attachment: patch (for 1.9.1 branch)

Comment 12

[mario_bjs]

bug with Windows XP sp3 x86 and Firefox 3.5

Comment 14

[Samuel Sidler (old account; do not CC)]

I don't recall why I nominated this for blocking, but it's not. Definitely wanted though and we'll consider a reviewed, tested, baked patch.

Comment 15

[Leo Davidson]

(In reply to comment #9)
> This will find the enabled Window even if it is minimized.

IMO the bug is in Steam. When you "close" its window the window actually stays open and, crucially, keeps the input focus. You can see this using Spy++ where after closing the Steam window pushing keys stills ends Steam keyboard messages. You can also see that no other window becomes active when Steam's window is closed.

So Firefox may be doing something wrong as well, or maybe it's worth working around programs which do this wrong thing even if what FF is doing is "correct," but the underlying fault most definitely lies with Steam.

Comment 16

[Leo Davidson]

Replying to myself after a bit more thought/discussion/reading:

There's definitely a bug in Steam (it's not passing on the input focus when its window is hidden), but I think even if that was fixed Firefox may still trigger the problem.

i.e. Looks like there are bugs in both Steam and Firefox, to me. Sorry for spamming the thread; just wanted to correct myself.

Comment 17

[AnAkkk]

I think the Steam developers already looked into this. The bug is with Firefox.

It started happening with Firefox 3.5, there was no problems before.

Comment 18

[Cobra]

After talking with Mike Blas from Valve, this was his response:


FireFox is pretty odd.

When you minimize an application in Windows, it autoamtically trims the working set. This means that it takes memory the process is using and swaps it to disk so that the memory is available for other processes. Back when memory tight, this was a very important thing to do. Now, it doesn't matter so much. I haven't gotten my copy of Windows Internals 5 yet, but I believe that Vista doesn't trim the working set as aggressively. Computers have more memory these days, and faulting the pages back into memory when you start using the application again takes time and makes it feel slower.

Anyway, I took a look at the FireFox source code. They check this setting out whenever they create this special, hidden window:

Code:

  if (gTrimOnMinimize == 2 && mWindowType == eWindowType_invisible) {
    /* The internal variable set by the config.trim_on_minimize pref
       has not yet been initialized, and this is the hidden window
       (conveniently created before any visible windows, and after
       the profile has been initialized).

       Default config.trim_on_minimize to false, to fix bug 76831
       for good.  If anyone complains about this new default, saying
       that a Mozilla app hogs too much memory while minimized, they
       will have that entire bug tattooed on their backside. */

    gTrimOnMinimize = 0;
    nsCOMPtr<nsIPrefService> prefs = do_GetService(NS_PREFSERVICE_CONTRACTID);
    if (prefs) {
      nsCOMPtr<nsIPrefBranch> prefBranch;
      prefs->GetBranch(0, getter_AddRefs(prefBranch));
      if (prefBranch) {

        PRBool temp;
        if (NS_SUCCEEDED(prefBranch->GetBoolPref("config.trim_on_minimize",
                                                 &temp))
            && temp)
          gTrimOnMinimize = 1;

        if (NS_SUCCEEDED(prefBranch->GetBoolPref("intl.keyboard.per_window_layout",
                                                 &temp)))
          gSwitchKeyboardLayout = temp;

        if (NS_SUCCEEDED(prefBranch->GetBoolPref("mozilla.widget.disable-native-theme",
                                                 &temp)))
          gDisableNativeTheme = temp;
      }
    }
  }

In that acerbic comment, they mention FireFox Bug #76831, which seems to be the lightning rod bug for all the various complainsts about the notoriously high memory usage and poor performance that FireFox has. It seems like the developers went after a silver bullet fix rather than addressing the core architecture of the program and how it fits with Windows.

The silver bullet they sought was apparently this automatic trimming feature that Windows does. Later on, apparently when they're handling the commands to minimize, maximize, or restore a window, they check that flag and screw around with the next window in the list of all windows on the system. Someone spent time and effort to write that cute, stupid comment about tattoos, but didn't spend any time to explain this deficient skulduggery, which makes the tattoo comment seem even less constructive.

Code:

NS_IMETHODIMP nsWindow::SetSizeMode(PRInt32 aMode) {

  nsresult rv;

  // Let's not try and do anything if we're already in that state.
  // (This is needed to prevent problems when calling window.minimize(), which
  // calls us directly, and then the OS triggers another call to us.)
  if (aMode == mSizeMode)
    return NS_OK;

#ifdef WINCE
  // on windows mobile, dialogs and top level windows are full screen
  // This is partly due to the lack of a GetWindowPlacement.
  if (mWindowType == eWindowType_dialog || mWindowType == eWindowType_toplevel) {
    aMode = nsSizeMode_Maximized;
  }
#endif

  // save the requested state
  rv = nsBaseWidget::SetSizeMode(aMode);
  if (NS_SUCCEEDED(rv) && mIsVisible) {
    int mode;

    switch (aMode) {
      case nsSizeMode_Maximized :
        mode = SW_MAXIMIZE;
        break;
#ifndef WINCE
      case nsSizeMode_Minimized :
        mode = gTrimOnMinimize ? SW_MINIMIZE : SW_SHOWMINIMIZED;
        if (!gTrimOnMinimize) {
          // Find the next window that is visible and not minimized.
          HWND hwndBelow = ::GetNextWindow(mWnd, GW_HWNDNEXT);
          while (hwndBelow  && !IsWindowEnabled(hwndBelow) && (!::IsWindowVisible(hwndBelow) ||
                               ::IsIconic(hwndBelow))) {
            hwndBelow = ::GetNextWindow(hwndBelow, GW_HWNDNEXT);
          }

          // Push ourselves to the bottom of the stack, then activate the
          // next window.
          ::SetWindowPos(mWnd, HWND_BOTTOM, 0, 0, 0, 0,
                         SWP_NOACTIVATE | SWP_NOMOVE | SWP_NOSIZE);
          if (hwndBelow)
            ::SetForegroundWindow(hwndBelow);

          // Play the minimize sound while we're here, since that is also
          // forgotten when we use SW_SHOWMINIMIZED.
          ::PlaySoundW(L"Minimize", nsnull, SND_ALIAS | SND_NODEFAULT | SND_ASYNC);
        }
        break;
#endif
      default :
        mode = SW_RESTORE;
    }
    ::ShowWindow(mWnd, mode);
  }
  return rv;
}

But what this code does is deliberately find another window, change its minimized/maximized state, set it to the foreground, and then carry on with life. Setting the trim_on_minimize option avoids this completely arbitrary code. I would guess that it is meant to counteract the code they wrote to implement the feature in the first place, which seems like it might be the root cause of the bug -- but I'm not sure, since I can't find the call to DefaultWindowProc() that's supposed to be happening:


Code:

#ifndef WINCE
    case WM_SYSCOMMAND:
      // prevent Windows from trimming the working set. bug 76831
      if (!gTrimOnMinimize && wParam == SC_MINIMIZE) {
        ::ShowWindow(mWnd, SW_SHOWMINIMIZED);
        result = PR_TRUE;
      }
      break;
#endif

So, in the end, we're both right. This setting does stop FireFox from screwing up other application's windows. But there's absolutely no relationship between trimming the working set of one process and what window should be made active when another window is minimized by the user.

The only role Steam has in this is that it doesn't actually close windows. This isn't uncommon, but it's rather unique. The idea is that an application with multiple top-level Windows (like Steam) doesn't want to switch its window list around. Otherwise, the shell tends to get confused about which one is active, which should be shown in the alt-tab list, and so on. When you close a window in Steam, then, it's parked as hidden and minimized and don't activate me unless I tell you.

Problem is, this silly code in FireFox comes along and finds the window (it's the "next" or "previous" window) and then activates it, then brings it to the foreground. For no good reason.

Comment 19

[Yan Mosyagin]

On 16 July Steam released a regular Steam Client beta: http://forums.steampowered.com/forums/showthread.php?t=919433

One of the notes in changelog is:

- Workaround for a Firefox 3.5 bug where they activate random windows after minimizing which would confuse the state of some Steam windows

We need someone who can check this.

Comment 20

[Masatoshi Kimura [:emk]]

We still need fix this on our side for Win7. See bug 472874.
Vlad, do you have time to take a look?

Comment 21

[Manuel]

> We need someone who can check this.
Just opt'ed in for the Steam Beta. Everything works now. No strange rectangle anymore and the Steam-popups-on-Fx-minimize is also gone.

Using WinXP Pro SP3 x86 w/ Fx 3.5.1

Comment 22

[blacksheep987]

Pretty Goddamn annoying
Fix this mozilla

Comment 23

[Leman Bennett [Omega]]

Please do not spam the bug with unnecessary comments. This will get fixed eventually. Be patient.

Comment 24

[John McCaskey]

(In reply to comment #19)
> On 16 July Steam released a regular Steam Client beta:
> http://forums.steampowered.com/forums/showthread.php?t=919433
> 
> One of the notes in changelog is:
> 
> - Workaround for a Firefox 3.5 bug where they activate random windows after
> minimizing which would confuse the state of some Steam windows
> 
> We need someone who can check this.

The fix on the Steam side of things is just to prevent Steam getting an activation on a non top-level window (like our tooltips or menu dropdowns) causing Steam to get in a broken state where that tooltip or menu was stuck in the top corner of the users screen.

Steam (and many other apps) will still respond to the random activation from Firefox on minimize for top level windows and those windows will popup unexpectedly.  We still have users reporting this happening with Steam + Firefox in our beta client.  

So it's true that this really still needs a proper fix on the Firefox side, we just wanted to act to minimize it's negative impact on Steam for our users ASAP.

Comment 25

[Michael Kirbach]

A similar problem exists with Oracle Forms 6i applications which are using Headstart calendar functionality. On minimize of Firefox and switching back to the Oracle Forms application, a not initialized calendar will automatically pop up. This calendar can not be closed anymore and the application must be terminated by the windows task manager. This behaviour can be reproduced and is new since Firefox 3.5. Setting the mentioned parameter config.trim_on_minimize to 'true' will solve the problem. I just want to mention the strange Oracle Forms behaviour for reference just in case someone else is encountering the same problem.

Regards,
Michael

Comment 26

[Masatoshi Kimura [:emk]]

Comment on attachment 386855 [details] [diff] [review]
patch (for trunk)

Vlad looks to be busy. Seeking other super-reviewer.

Comment 27

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 386855 [details] [diff] [review]
patch (for trunk)

You don't actually need sr for this.

But indent ::IsIconic by one more space so it lines up with the !::IsWindowEnabled.

Comment 28

[Masatoshi Kimura [:emk]]

Attachment: fix indent

Comment 29

[Jim Mathies [:jimm]]

http://hg.mozilla.org/mozilla-central/rev/4b3bdf8ce056

Comment 30

[Daniel Veditz [:dveditz]]

Comment on attachment 391445 [details] [diff] [review]
fix indent

Approved for 1.9.1.4, a=dveditz for release-drivers

Comment 31

[Marco Zehe (:MarcoZ)]

Pushed to 1.9.1 branch: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/4fe257416935.

Comment 32

[Marco Zehe (:MarcoZ)]

Attachment: 1.9.1 patch updated to latest revision

The original patch no longer applied cleanly, this is the patch that got pushed in the end.

Comment 33

[Marco Zehe (:MarcoZ)]

Verified fixed in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.4pre) Gecko/20090827 Shiretoko/3.5.4pre (.NET CLR 3.5.30729)

Comment 34

[[e-mail]]

(In reply to comment #18)
> When you minimize an application in Windows, it autoamtically trims the working set.
This isn't quite true. When you minimize a *window* in Windows, it automatically trims the working set. So if you have more than one window, you lose out, because your other windows have to wait for pages to fault back in.

Comment 35

[Rich Parker]

We seem to be going in circles.  My Firefox no longer triggers the XP "Minimize" sound after upgrading to 3.5.4.  The sounds for Maximize, Restore Down, and Restore Up still play as expected.  And when I click the "_" button in the top-right corner of the window, the task bar button remains sunk into the task bar as if it's still the active window.  See Bug 300689.

Comment 36

[Rich Parker]

Correction:  The minimize ("_") button no longer deactivates the task bar button even in the final 3.5 release (before 3.5.1).  I confiremd that it worked correctly in 3, so that apparently broke somewhere in 3.5.  The "Minimize" sound, however, seems to have stopped working more recently.

Comment 37

[alex.w.lu]

Is it me or was this bug not resolved in 3.5.4?

Comment 38

[Rich Parker]

Nobody seems to be acknowledging the regression either.  I just confirmed that the "Minimize" sound plays properly in my Firefox 3.5 Portable (200906nn) and stops working after moving to 20091016.

Comment 39

[Jim Mathies [:jimm]]

(In reply to comment #38)
> Nobody seems to be acknowledging the regression either.  I just confirmed that
> the "Minimize" sound plays properly in my Firefox 3.5 Portable (200906nn) and
> stops working after moving to 20091016.

Is there an easy way to test for this? I'm not very familiar with "steam".

Comment 40

[Rich Parker]

I'm not able to replicate the Steam issue; alex2364, what are the steps?

As for the sound regression, click the task bar button to minimize Firefox.  The Windows "Minimize" system sound doesn't play (make sure there's one set up in Control Panel > Sounds and Audio Devices > Sounds > Program events > Windows > Minimize).  This is in XP Pro.

Comment 41

[Jim Mathies [:jimm]]

(In reply to comment #40)
> I'm not able to replicate the Steam issue; alex2364, what are the steps?
> 
> As for the sound regression, click the task bar button to minimize Firefox. 
> The Windows "Minimize" system sound doesn't play (make sure there's one set up
> in Control Panel > Sounds and Audio Devices > Sounds > Program events > Windows
> > Minimize).  This is in XP Pro.

That code is right here - 

http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsWindow.cpp#1476

Have you tried flipping the config.trim_on_minimize pref?

Comment 42

[alex.w.lu]

You don't need Steam to reproduce this bug. Just open Firefox and another program. If you have both windows open and Firefox is on top of the other program, minimize Firefox, focus should be brought to the other program. But this isn't happening. Instead, when Firefox is minimized, it keeps focus, and I have to click on the other program for the window to be active. I have reproduced this many times when the other program is Thunderbird.

Right now I have to use config.trim_on_minimize so that Firefox behaves correctly.

Comment 43

[Rich Parker]

config.trim_on_minimize also takes care of the sound issue, but that's an old work-around.  It was fixed at some point so that the work-around was no longer necessary.  It's broken again in 3.5.4.

Comment 44

[Jim Mathies [:jimm]]

Is this a *different* bug from "Minimizing Firefox opens and gives focus to minimized Steam windows"? I'm trying to figure out if this bug should be re-opened (from the title and description in comment 42 I'd say not) or a new bug posted and blocking requested.

Comment 45

[James Ross]

If I understand everything correctly, something has regressed both bug 300689 and bug 295355 (I can reproduce both problems on Windows 7 with Firefox 3.5.4). Steam isn't involved in these regressions, but the patch in this bug is being implicated.

Comment 46

[Jim Mathies [:jimm]]

I can confirm.

Comment 47

[Jim Mathies [:jimm]]

Note, setting config.trim_on_minimize to true does solve the problem.

  	
Masatoshi, do want to keep this? If not I can look into it.

Comment 48

[Masatoshi Kimura [:emk]]

I think the cause of regression is bug 499816 (not this bug). Otherwise it can't be explained why bug 295355 has regressed. Due to a fix of bug 499816, SetSizeMode will early return here:
http://mxr.mozilla.org/mozilla1.9.1/source/widget/src/windows/nsWindow.cpp#1854
But I have no time to investigate further. Please take a look.

Comment 49

[Masatoshi Kimura [:emk]]

(In reply to comment #48)
> I think the cause of regression is bug 499816 (not this bug).
Sorry, I meant to say bug 489258.

Comment 50

[Jim Mathies [:jimm]]

Attachment: tom active window patch v.1

Bug 489258 did trigger this. However the assumption there was sane - SetSizeMode is an internal call to change the mode of the window, but we were relying on  it to touch up minimization functionality when sTrimOnMinimize is false.

This patch reorganizes things so that SetSizeMode does what it should - *requests* the window be minimized. The active window touch-up and sound playback work are handed when windows tells us it *has* minimized the window. This way we get that functionality regardless of how the window is minimized, without having to call SetSizeMode when window has already minimized the window. ack!

Pushed the patch to try, will post builds here in a bit.

Comment 51

[Jim Mathies [:jimm]]

Attachment: t.o.m. active window patch v.1

Fixed a few comment typos and made the helper static. This passed try server cleanly.

Comment 52

[Jim Mathies [:jimm]]

This should block 1.9.2, it's a rather annoying bug once you notice it.

Comment 53

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 411837 [details] [diff] [review]
t.o.m. active window patch v.1

>-        break;
>+        break;
one minor nit, this looks like a while space change of some sort, but I can't figure out what. Just make sure there isn't a screwy line ending or tab here.

Comment 54

[Jim Mathies [:jimm]]

(In reply to comment #53)
> (From update of attachment 411837 [details] [diff] [review])
> >-        break;
> >+        break;
> one minor nit, this looks like a while space change of some sort, but I can't
> figure out what. Just make sure there isn't a screwy line ending or tab here.

Must have been some diff funnyness, spacing was ok.

http://hg.mozilla.org/mozilla-central/rev/fe80dd3ebe74

Comment 55

[Mike Beltzner [:beltzner, not reading bugmail]]

Sure, let's get this into 1.9.2 - blocking, so you can remove the approval requests and just land it.

Comment 56

[Jim Mathies [:jimm]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/537ac894e256

Comment 57

[Daniel Veditz [:dveditz]]

Comment on attachment 411837 [details] [diff] [review]
t.o.m. active window patch v.1

Approved for 1.9.1.8, a=dveditz for release-drivers

Comment 58

[Daniel Veditz [:dveditz]]

Comment on attachment 411837 [details] [diff] [review]
t.o.m. active window patch v.1

removing 1.9.1 approvals for non-blocking patches. please renominate for the next release if we need to fix this.

Comment 59

[Marco Bonardo [:mak]]

Jim, should this land on 1.9.1 (if so, ask approval again?)? if the 1.9.1 status is fine and not requiring further patches can we mark status1.9.1 as fixed or wontfix?

The status of this bug is quite strange, it has one patch ported to 1.9.1 and one not, causing pollution in bugzilla queries for 1.9.1. It is usually better to separate bugs in such cases.