Closed Bug 506124 Opened 14 years ago Closed 13 years ago

Fixup Aero Glass code

Categories

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

x86
Windows 7
defect

Tracking

()

RESOLVED FIXED
mozilla1.9.3a1
Tracking Status
status1.9.2 --- beta1-fixed

People

(Reporter: robarnold, Assigned: jimm)

References

Details

Attachments

(6 files, 3 obsolete files)

Attached patch v1.0 (obsolete) — Splinter Review
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.
Attachment #390348 - Flags: review?(jmathies)
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
Attachment #390348 - Flags: review?(jmathies) → review+
Attached patch v1.1 (obsolete) — Splinter Review
Removed some unnecessary debugging cruft that got in.
Attachment #390348 - Attachment is obsolete: true
Attachment #390357 - Flags: review?(jmathies)
Attachment #390357 - Flags: review?(jmathies) → review+
Attachment #390357 - Flags: review?(vladimir)
Comment on attachment 390357 [details] [diff] [review]
v1.1

This fixes three aero glass bugs.
Flags: blocking1.9.2?
Can this land?
Depends on: 501490
Is there any hope of emulating this on Linux? I'm sure Compiz and/or GDK provide some nice way to do it.
(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.
Flags: blocking1.9.2? → blocking1.9.2+
Priority: -- → P1
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.
Attached patch split out patch v.1 (obsolete) — Splinter Review
updated, sent to try for some testing.
wince touchups for the original patch. this passed try fine.
Attachment #390357 - Attachment is obsolete: true
Attachment #403054 - Attachment is obsolete: true
http://hg.mozilla.org/mozilla-central/rev/634385257da9
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Attachment #403068 - Flags: approval1.9.2?
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.
Attachment #403068 - Flags: approval1.9.2?
no tests?
(In reply to comment #12)
> no tests?

Possibly for bugs 463305 & 473152, I'll look into it.
Target Milestone: mozilla1.9.2 → mozilla1.9.3a1
No longer depends on: 501490
Attached file 461952 testcase
Attached file 463305 testcase
Attached file 473152 testcase
Assignee: tellrob → jmathies
Attached patch 1.9.2 patchSplinter Review
sent to try for some test runs...
what are your plans for the tests?
(In reply to comment #19)
> what are your plans for the tests?

On my todo list for this week.
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.
(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?
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;
(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.
#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".
Attachment #404999 - Flags: review?(roc)
Comment on attachment 404999 [details] [diff] [review]
[checked-in] 502 sdk patch

http://hg.mozilla.org/mozilla-central/rev/e2f0a8ec44c5
Attachment #404999 - Attachment description: follow up 502 sdk patch for margins struct → [checked-in] 502 sdk patch
Attachment #404999 - Flags: approval1.9.2?
Comment on attachment 404999 [details] [diff] [review]
[checked-in] 502 sdk patch

err, sorry, no need for that, it's blocking.
Attachment #404999 - Flags: approval1.9.2?
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.
You need to log in before you can comment on or make changes to this bug.