520807 - Windows 7 taskbar previews scales out of screen

Status: RESOLVED FIXED

(Firefox :: Shell Integration, defect, P2)

Description

[DG]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20091006 Minefield/3.7a1pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20091006 Minefield/3.7a1pre

When hovering over a tab in the taskbar the window becomes larger than the screen resolution on my netbook (1024x600)

Reproducible: Always

Steps to Reproduce:
1. Click on Mozilla icon in the taskbar
2. Hover over a tab
3.
Actual Results:  
Large preview scales larger than the resolution my screen.

Expected Results:  
Preview remains the same size as the app.

Comment 1

[DG]

Attachment: screenshot

Comment 2

[[not reading bugmail]]

Can you confirm its just Firefox and not also another app in W7

Comment 3

[Alex]

I see the same thing, only happens if the main Firefox window is maximised (the temporary window it shows is positioned past the screen edges)

Comment 4

[Rob Arnold [:robarnold]]

I'm not sure if this is a bug we can fix. I could only reproduce this with a maximized window (please post another screenshot if somehow that is not the only case). The DWM is drawing the non-client area of the window and it appears to be ignoring the maximized style and drawing an unmaximized window frame, thus appearing to be too large. The client area is drawn in the correct place and at the correct size nonetheless.

The DWM always draws the window as you last saw it, nonclient and client area alike. If you look carefully in the attached screenshot, you can see the close button in the right place, underneath the superimposed incorrectly placed frame.

One possible fix is to return false from PrviewController.drawPreview which tells the DWM not to draw the frame (in which case the window appears as expected and the nonclient area is painted over).

In testing, I found that if I returned false, sometimes the window frame would not be drawn at all by the DWM and that was worse since you could not see the tab title as well. I shall do some more testing to see if I can reliably trigger that bug (I found it in the RC, have not tried on RTM). If I can reproduce it, a compromise may be to return false only if maximized.

Comment 6

[Brian Carpenter [:geeknik]]

Dale, I'm using Windows 7 Pro x64 and I'm not seeing this behavior with any other application under Windows 7.

Comment 8

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Unblocking, as we're not going to ship tab previews in Firefox 3.6 (see bug 525475).

Comment 9

[bogas04]

Best way to reproduce

1#Open firefox and maximize it
2#Open multiple tabs and hover-over one of them

Result : Window goes outside the monitor

Comment 10

[bogas04]

Attachment: Bugs with Aero peek feature

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to comment #4)
> One possible fix is to return false from PrviewController.drawPreview which
> tells the DWM not to draw the frame (in which case the window appears as
> expected and the nonclient area is painted over).
> 
> In testing, I found that if I returned false, sometimes the window frame would
> not be drawn at all by the DWM and that was worse since you could not see the
> tab title as well. I shall do some more testing to see if I can reliably
> trigger that bug (I found it in the RC, have not tried on RTM). If I can
> reproduce it, a compromise may be to return false only if maximized.

I've been experimenting with this and it doesn't really help because while the DWM waits for the preview image it will display the broken window title.  This at best creates titlebar flickering and at worst doesn't help at all (e.g. while experiencing Bug 522262)

Comment 12

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Image showing bitmap provided to the DWM

If you look at this image you can see the preview bitmap the DWM is getting from Fx offset from the normal window title.

Comment 13

[[not reading bugmail]]

The top image (preview) looks like their is visible padding at the bottom of the titlebar which I thought maybe bug 520637 might have created, or patched again in bug 524718.  

I was also thinking at one point it was because the taskbar is taller on Windows 7 then it is on Vista for maximized mode showing available px height size.  I forget by exactly how much px, but it seems about the same px as the titlebar padding on the preview.

I wonder why the items also shift out also.  But it still doesn't explain why the titlebar moves 

The flicker thing is what I mentioned in bug 521954 when your previewing the maximized state.

Comment 14

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I believe I have found a way to fix this.  I hope to have a patch up late tonight.

Comment 15

[[not reading bugmail]]

I did a another visual comparison of the two window sizes while thinking about another minimize/restore up/down maximize bug today.  I just noticed non-maximized windows have a border, maximized windows do not.  Maybe the lack of a border has to be factored in?

Borders are not drawn on maximized mode on the top/bottom/left/right so the browser window is actually larger in maximized mode, than one that is the full screen size [up to the taskbar line typically, not fullscreen mode] but not maximized.

Comment 16

[Rob Arnold [:robarnold]]

I think what is happening is that the DWM using the hidden tab window's window style to draw a temporary preview before we give it the real data. IE8 uses the toolwindow exstyle and draws fairly quickly so you have to pay close attention to see its flicker. We can either accept the toolwindow style and see if that's "good enough" or perhaps we can change the tab window's style based on the parent window's style (maximized or not) either as the main window changes (ick) or as soon as we know that we need to draw a preview (possibly too late).

Playing withe DWM settings for nonclient rendering seemed to have no effect.

The toolwindow trick works fairly well. IE8 seems to get along fine with it.

Also, returning false from drawPreview appears to be the wrong idea. Sometimes the DWM draws the correct title, sometimes it does not. I suspect that this is a race between our tab switching (and hence title switching) code and it's drawing code.

Comment 17

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to comment #16)
> perhaps we can change the tab window's style based on the
> parent window's style (maximized or not) either as the main window changes
> (ick)

This is what I tried, unfortunately, it doesn't seem to be enough.  If you look closely at http://www.kylehuey.com/moz/aeropeekproblem.png you'll see that the frame is still off-screen even when it draws the maximize/restore button correctly.  (In fact, just noticed that it looks like this on builds from trunk, so apparently lack of WS_MAXIMIZE is not the problem).

As another data point, I added

> case WM_NCACTIVATE:
>   wParam = true;
>   break;

to the window procedure which should ensure that the non-client area always draws in an activated state (notably with the close button red in the Aero theme) and it has absolutely no effect.  It appears as though the DWM is bypassing some and maybe all of the normal non-client stuff when it draws the frame with DwmSetIconicLivePreviewBitmap.

Earlier when I thought I had figured it out I was trying to not draw the frame, and succeeded, but the flicker still appeared, just intermittently instead of every time and I didn't see it for a while.  The no frame approach seems to have other problems with the race condition you mentioned.

Comment 18

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

The sample program given here http://weblogs.asp.net/sergeyzwezdin/archive/2009/05/21/windows-7-programming-taskbar-part-5-customwindowsmanager.aspx has exactly the same bug.

Comment 19

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Message transcript for peek proxy window

Yeah, none of the WM_NC***** messages get sent to the proxy window.

Also, the problem is very visible in IE 8 if you peek while it's in fullscreen mode (F11).  You can definitely see the window frame get rendered before it vanishes.

Comment 20

[Rob Arnold [:robarnold]]

(In reply to comment #17)
> Earlier when I thought I had figured it out I was trying to not draw the frame,
> and succeeded, but the flicker still appeared, just intermittently instead of
> every time and I didn't see it for a while.  The no frame approach seems to
> have other problems with the race condition you mentioned.

Can you post your solution? I played around with this again tonight. I have a partial fix for the race condition but it's not perfect (IE8 has the same problem though). IE8 also appears to not be drawing the DWM frame. If the window title is updated during peek then it overrides whatever the currently focused preview is. Since we have no way of knowing when the user is previewing, we can't delay the title change. I did just come up with an idea however - if we invalidate all the previews whenever the title changes, then the DWM will be forced, when the user is previewing, to ask for updated bitmaps and it should redraw the title. I'll try this out later.

Meanwhile, I have a patch which hooks WM_WINDOWPOSCHANGED and switches the preview window's style based on the visible window.

Comment 21

[Rob Arnold [:robarnold]]

(In reply to comment #20)
> Meanwhile, I have a patch which hooks WM_WINDOWPOSCHANGED and switches the
> preview window's style based on the visible window.
I should add that this looks much better for non-maximized windows since there is no change in geometry, only color. For maximized windows, we are on par with IE8. I wish we could do better.

Comment 22

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to comment #20)
> (In reply to comment #17)
> Meanwhile, I have a patch which hooks WM_WINDOWPOSCHANGED and switches the
> preview window's style based on the visible window.

That was half of what I was doing.  The other half was messing around with the NC**** messages which never get sent anyways, so what you have is probably what I had.

(In reply to comment #21)
> (In reply to comment #20)
> > Meanwhile, I have a patch which hooks WM_WINDOWPOSCHANGED and switches the
> > preview window's style based on the visible window.
> I should add that this looks much better for non-maximized windows since there
> is no change in geometry, only color. For maximized windows, we are on par with
> IE8. I wish we could do better.

Yeah, it's a real shame that the API sucks.  I really have to wonder how much harder it would have been to have the DWM actually draw the window and use the standard complement of messages.  

It's been a while since I played with this, so perhaps I am forgetting why we can't do this, but what happens if we set DwmSetWindowAttribute DWNNCRENDERINGPOLICY to DWMNCRP_USEWINDOWSTYLE on the proxy window?  Does that convince the DWM to do this properly?  It looks like we call this on our real windows but not our proxy windows from my brief glance at the code.

Comment 23

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to comment #22)
> It's been a while since I played with this, so perhaps I am forgetting why we
> can't do this, but what happens if we set DwmSetWindowAttribute
> DWNNCRENDERINGPOLICY to DWMNCRP_USEWINDOWSTYLE on the proxy window?  Does that
> convince the DWM to do this properly?  It looks like we call this on our real
> windows but not our proxy windows from my brief glance at the code.

After trying this, it doesn't do anything interesting.

Comment 24

[Rob Arnold [:robarnold]]

Attachment: Dynamically update the proxy window's style based on the main window

Comment 25

[Rob Arnold [:robarnold]]

Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/fd270fe3c7fc

Comment 26

[LegNeato]

Comment on attachment 422101 [details] [diff] [review]
Dynamically update the proxy window's style based on the main window

a=LegNeato for 1.9.2.5. Please ONLY land this on mozilla-1.9.2 default, as we
are still working on 1.9.2.4 on the relbranch

Comment 27

[Marco Bonardo [:mak]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/3042de2e56a2