506124 - Fixup Aero Glass code

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect, P1)

Description

[Rob Arnold [:robarnold]]

Attachment: v1.0

The alt-tab window doesn't use WS_THICKFRAME so it's possible to have a glass popup without it. WS_THICKFRAME introduces a frame that the user can use to resize the window; this is undesireable.

This fix involves forcing the DWM to ignore the window style and render the window border.

Comment 1

[Jim Mathies [:jimm]]

Comment on attachment 390348 [details] [diff] [review]
v1.0

Looks good to me except, I think there's a typo - we don't have a MOZ_NTDDI_VISTA define.

+#if MOZ_WINSDK_TARGETVER >= MOZ_NTDDI_VISTA

Comment 2

[Rob Arnold [:robarnold]]

Attachment: v1.1

Removed some unnecessary debugging cruft that got in.

Comment 3

[Rob Arnold [:robarnold]]

Comment on attachment 390357 [details] [diff] [review]
v1.1

This fixes three aero glass bugs.

Comment 4

[Dão Gottwald [:dao]]

Can this land?

Comment 5

[HyperHacker]

Is there any hope of emulating this on Linux? I'm sure Compiz and/or GDK provide some nice way to do it.

Comment 6

[Rob Arnold [:robarnold]]

(In reply to comment #5)
> Is there any hope of emulating this on Linux? I'm sure Compiz and/or GDK
> provide some nice way to do it.

Most of the work to achieve the effect is done by Windows - we merely tell it where and paint with an alpha channel. If Compiz or GDK (GTK?) exposes a similar API, it probably wouldn't be too hard to imitate this feature for those systems.

If you find some Linux APIs that seem promising, by all means please file a bug (CC me) and link to the documentation in it. This bug is for the implementation of the Windows feature.

Comment 7

[Jim Mathies [:jimm]]

Rob, does this really rely on bug 501490? It looks like it's pretty stand alone and could go in on it's own. It's blocking 1.9.2 and blocks 473152 since it might address that. If you want I can take a shot at getting it updated and landed so we can clear these out.

Comment 8

[Jim Mathies [:jimm]]

Attachment: split out patch v.1

updated, sent to try for some testing.

Comment 9

[Jim Mathies [:jimm]]

Attachment: split out patch v.2

wince touchups for the original patch. this passed try fine.

Comment 10

[Jim Mathies [:jimm]]

http://hg.mozilla.org/mozilla-central/rev/634385257da9

Comment 11

[Jim Mathies [:jimm]]

Comment on attachment 403068 [details] [diff] [review]
split out patch v.2

I guess this doesn't need approval since it's already blocking, but I'm going to wait a day or so before landing it on branch.

Comment 12

[Robert Sayre]

no tests?

Comment 13

[Jim Mathies [:jimm]]

(In reply to comment #12)
> no tests?

Possibly for bugs 463305 & 473152, I'll look into it.

Comment 14

[Jim Mathies [:jimm]]

Attachment: 461952 testcase

Comment 15

[Jim Mathies [:jimm]]

Attachment: 463305 testcase

Comment 16

[Jim Mathies [:jimm]]

Attachment: 473152 testcase

Comment 17

[Jim Mathies [:jimm]]

Attachment: 1.9.2 patch

sent to try for some test runs...

Comment 18

[Jim Mathies [:jimm]]

pushed to 1.9.2:
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/66855e1ac080

Comment 19

[Robert Sayre]

what are your plans for the tests?

Comment 20

[Jim Mathies [:jimm]]

(In reply to comment #19)
> what are your plans for the tests?

On my todo list for this week.

Comment 21

[Mats Palmgren (inactive)]

FWIW, this broke my Firefox trunk build on Windows XP.  The problem is the
added #ifdef WINCE around struct MARGINS at the top of nsUXThemeData.h.
Apparently MARGINS is not a known symbol on my system and it's needed
for the GetThemeMarginsPtr typedef later in the file.
If I remove the #ifdef I can build and run it without problem.

Comment 22

[Jim Mathies [:jimm]]

(In reply to comment #21)
> FWIW, this broke my Firefox trunk build on Windows XP.  The problem is the
> added #ifdef WINCE around struct MARGINS at the top of nsUXThemeData.h.
> Apparently MARGINS is not a known symbol on my system and it's needed
> for the GetThemeMarginsPtr typedef later in the file.
> If I remove the #ifdef I can build and run it without problem.

Hmm, what windows sdk are you building with?

Comment 23

[Mats Palmgren (inactive)]

INCLUDE=C:\Program Files\Microsoft Platform SDK for Windows Server 2003 R2\\include;C:\Program Files\Microsoft Platform SDK for Windows Server 2003 R2\\include\atl;C:\Program Files\Microsoft Visual Studio 8\VC\INCLUDE;

Comment 24

[Jim Mathies [:jimm]]

(In reply to comment #23)
> INCLUDE=C:\Program Files\Microsoft Platform SDK for Windows Server 2003
> R2\\include;C:\Program Files\Microsoft Platform SDK for Windows Server 2003
> R2\\include\atl;C:\Program Files\Microsoft Visual Studio 8\VC\INCLUDE;

I don't have an env. to test this, curious if you can try replacing that ifdef with the ws03 define?

#if MOZ_WINSDK_TARGETVER = MOZ_NTDDI_WS03
struct MARGINS
..
#endif

(or)

#if MOZ_WINSDK_TARGETVER < MOZ_NTDDI_LONGHORN
#endif

and make sure you have 

ac_add_options --with-windows-version=502

in your .mozconfig when you configure.

Comment 25

[Mats Palmgren (inactive)]

#if MOZ_WINSDK_TARGETVER == MOZ_NTDDI_WS03
works (note the double equal-sign)

#if MOZ_WINSDK_TARGETVER < MOZ_NTDDI_LONGHORN
also works

Yes, I have "ac_add_options --with-windows-version=502" in my .mozconfig

and to be a bit more precise regarding my environment, it is "Visual C++ 8 (VS2005) Express" with the "Windows Server 2003 R2 Platform SDK".

Comment 26

[Jim Mathies [:jimm]]

Attachment: [checked-in] 502 sdk patch

Comment 27

[Jim Mathies [:jimm]]

Comment on attachment 404999 [details] [diff] [review]
[checked-in] 502 sdk patch

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

Comment 28

[Jim Mathies [:jimm]]

Comment on attachment 404999 [details] [diff] [review]
[checked-in] 502 sdk patch

err, sorry, no need for that, it's blocking.

Comment 29

[Jim Mathies [:jimm]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/cb5bfea4b8aa

Comment 30

[[e-mail]]

Comment on attachment 403068 [details] [diff] [review]
split out patch v.2

>   static PRBool CheckForCompositor() {
>     BOOL compositionIsEnabled = FALSE;
>     if(dwmIsCompositionEnabledPtr)
>       dwmIsCompositionEnabledPtr(&compositionIsEnabled);
>     return sHaveCompositor = (compositionIsEnabled != 0);
>   }
>+#endif // MOZ_WINSDK_TARGETVER >= MOZ_NTDDI_LONGHORN
Unfortunately not all the calls to CheckForCompositor() are inside #ifdef although I was able to get my build working by removing all the #ifdef except the one guarding the dwmapi.h include.