Fixup Aero Glass code

RESOLVED FIXED in mozilla1.9.3a1

Status

()

P1
normal
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: robarnold, Assigned: jimm)

Tracking

Trunk
mozilla1.9.3a1
x86
Windows 7
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.2 +

Firefox Tracking Flags

(status1.9.2 beta1-fixed)

Details

Attachments

(6 attachments, 3 obsolete attachments)

(Reporter)

Description

9 years ago
Created attachment 390348 [details] [diff] [review]
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.
Attachment #390348 - Flags: review?(jmathies)
(Assignee)

Comment 1

9 years ago
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+
(Reporter)

Comment 2

9 years ago
Created attachment 390357 [details] [diff] [review]
v1.1

Removed some unnecessary debugging cruft that got in.
Attachment #390348 - Attachment is obsolete: true
Attachment #390357 - Flags: review?(jmathies)
(Assignee)

Updated

9 years ago
Attachment #390357 - Flags: review?(jmathies) → review+
(Reporter)

Updated

9 years ago
Attachment #390357 - Flags: review?(vladimir)
(Reporter)

Comment 3

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

This fixes three aero glass bugs.
(Reporter)

Updated

9 years ago
Flags: blocking1.9.2?
Can this land?

Updated

9 years ago
Depends on: 501490

Comment 5

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

Comment 6

9 years ago
(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.

Updated

9 years ago
Flags: blocking1.9.2? → blocking1.9.2+
Priority: -- → P1
(Assignee)

Comment 7

9 years ago
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.
(Assignee)

Comment 8

9 years ago
Created attachment 403054 [details] [diff] [review]
split out patch v.1

updated, sent to try for some testing.
(Assignee)

Comment 9

9 years ago
Created attachment 403068 [details] [diff] [review]
split out patch v.2

wince touchups for the original patch. this passed try fine.
Attachment #390357 - Attachment is obsolete: true
Attachment #403054 - Attachment is obsolete: true
(Assignee)

Comment 10

9 years ago
http://hg.mozilla.org/mozilla-central/rev/634385257da9
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Assignee)

Updated

9 years ago
Attachment #403068 - Flags: approval1.9.2?
(Assignee)

Comment 11

9 years ago
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?

Comment 12

9 years ago
no tests?
(Assignee)

Comment 13

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

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

Updated

9 years ago
Target Milestone: mozilla1.9.2 → mozilla1.9.3a1

Updated

9 years ago
No longer depends on: 501490
(Assignee)

Comment 14

9 years ago
Created attachment 403117 [details]
461952 testcase
(Assignee)

Comment 15

9 years ago
Created attachment 403118 [details]
463305 testcase
(Assignee)

Comment 16

9 years ago
Created attachment 403119 [details]
473152 testcase
(Assignee)

Updated

9 years ago
Assignee: tellrob → jmathies
(Assignee)

Comment 17

9 years ago
Created attachment 403297 [details] [diff] [review]
1.9.2 patch

sent to try for some test runs...
(Assignee)

Comment 18

9 years ago
pushed to 1.9.2:
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/66855e1ac080
status1.9.2: --- → beta1-fixed

Comment 19

9 years ago
what are your plans for the tests?
(Assignee)

Comment 20

9 years ago
(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.
(Assignee)

Comment 22

9 years ago
(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;
(Assignee)

Comment 24

9 years ago
(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".
(Assignee)

Comment 26

9 years ago
Created attachment 404999 [details] [diff] [review]
[checked-in] 502 sdk patch
Attachment #404999 - Flags: review?(roc)
(Assignee)

Comment 27

9 years ago
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
(Assignee)

Updated

9 years ago
Attachment #404999 - Flags: approval1.9.2?
(Assignee)

Comment 28

9 years ago
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.