152280 - Avoid flashing a foreground window when a dialog is displayed

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Ere Maijala (slow)]

Apparently there is a problem in Windows 2000 and XP that causes a flashed
(blinked, using FlashWindow()) foreground window to not "unflash". This will
cause a taskbar with auto-hide enabled to stay visible until the window is
deactivated and activated. This can be avoided by not even unflashing a
foreground window.

I will be posting a patch soon.

Comment 1

[Ere Maijala (slow)]

Attachment: v1 patch to avoid even unflashing a window if it hasn't flashed

This patch changes the attention code so that ::FlashWindow(handle, FALSE) is
not called if ::FlashWindow(handle, TRUE) has not been called (usually if the
window is already on foreground). 

For some reason trying to unflash a window will always invert the taskbar
button causing an auto-hidden taskbar to be shown on Windows 2000 and XP.

Comment 2

[Cal Tinson]

Thanks so much for filing this, Ere. This is my first experience with Bugzilla,
and I'm tremendously impressed!

Comment 3

[:deanis74]

Ere, is this what I tried poorly to describe in bug 145146?

Comment 4

[Ere Maijala (slow)]

It looks like it's the same issue and this fix might help there also. I tried
the test case with the patched version and it seemed to work, although I was
using Terminal Services and it was a bit slow, so I'm not sure about the timing.
Anyway, there was nothing flashing.

CC'ing Bill Law for a review (please?).

Comment 5

[Ere Maijala (slow)]

*** Bug 145146 has been marked as a duplicate of this bug. ***

Comment 6

[:deanis74]

Do you really need a separate SetFlashed() function, or can this just be done in
the TimerProc()?

Comment 7

[Ere Maijala (slow)]

Umm.. I think it's necessary. FlashWindow(..., FALSE) must be called in the end
if FlashWindow(..., TRUE) has been called. TimerProc will be called multiple
times if the window isn't on foreground. The attention timer is the only one
that can hold all the information about the window to be flashed. There is no
other way (at least that I know of) to tell if FlashWindow(..., TRUE) has been
called to window. 

Comment 8

[:deanis74]

Comment on attachment 87980 [details] [diff] [review]
v1 patch to avoid even unflashing a window if it hasn't flashed

Right, I understand that, but I was wondering specifically why the separate
SetFlashed() function.

>+  void SetFlashed(HWND timerWindow) {
>+    TimerInfo *info = FindInfo(timerWindow);
>+    if (info)
>+      info->hasFlashed = PR_TRUE;
>+  }
....
>+    gAttentionTimerMonitor->SetFlashed(hwnd);
>+  }

Why not just do this in nsGetAttentionTimerFunc() instead of calling
SetFlashed()?

  TimerInfo *info = FindInfo(hwnd);
  if (info)
    info->hasFlashed = PR_TRUE;

Why does it need a separate function?

Comment 9

[Ere Maijala (slow)]

That would require exposing the TimerInfo structure to outsiders which is
something I'd rather avoid when there is a nice class wrapping it already. Note
that SetFlashed() belongs to nsAttentionTimerMonitor class and the TimerProc to
nsWindow.

Comment 10

[:deanis74]

Comment on attachment 87980 [details] [diff] [review]
v1 patch to avoid even unflashing a window if it hasn't flashed

Ah yes, I see that now.  I should refrain from reviewing code in the first
couple of days after spending a week in the Caribbean!

Is this bug documented anywhere?  Regardless, r=me.

Comment 11

[Ere Maijala (slow)]

I couldn't find anything official, but there are dozens of puzzled comments in
the usenet. I also built a small test program which showed the same behavior. 

Comment 12

[jag (Peter Annema)]

Comment on attachment 87980 [details] [diff] [review]
v1 patch to avoid even unflashing a window if it hasn't flashed

sr=jag

Comment 13

[Ere Maijala (slow)]

Fix checked in to trunk.

Comment 14

[:deanis74]

This fixed bug 145146 for me, so verifying fixed on Win2K using 2002062908.