1203574 - Use SW_SHOWNA for the temporary window we create on win7 and below to measure caption button sizes

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[:Gijs (he/him)]

According to our Microsoft contacts, this will avoid the window taking foreground, which will avoid needless foreground bouncing.

Comment 1

[:Gijs (he/him)]

Attachment: MozReview Request: Bug 1203574 - use ShowNA for everything except aero lite to get caption button metrics, r?jimm

Bug 1203574 - use SHOWNA for the temporary window we use to get caption button metrics, r?jimm

Comment 2

[:Gijs (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f997daf1d603

Comment 3

[Jim Mathies [:jimm]]

Can you test this change on windows 7 aero basic to be sure the command buttons have the correct dimensions?

Comment 4

[:Gijs (he/him)]

(In reply to Jim Mathies [:jimm] from comment #3)
> Can you test this change on windows 7 aero basic to be sure the command
> buttons have the correct dimensions?

I tested on aero and classic, but I'll check aero basic. Out of curiosity, is there a particular reason that is more likely than the others to have issues?

Comment 5

[:Gijs (he/him)]

(In reply to :Gijs Kruitbosch from comment #4)
> (In reply to Jim Mathies [:jimm] from comment #3)
> > Can you test this change on windows 7 aero basic to be sure the command
> > buttons have the correct dimensions?
> 
> I tested on aero and classic, but I'll check aero basic. Out of curiosity,
> is there a particular reason that is more likely than the others to have
> issues?

Ugh. Yeah, that doesn't work well. I'll put up a screenshot... any idea why?

Comment 6

[:Gijs (he/him)]

Attachment: wonky-closebtns.png

Comment 7

[Jim Mathies [:jimm]]

(In reply to :Gijs Kruitbosch from comment #5)
> (In reply to :Gijs Kruitbosch from comment #4)
> > (In reply to Jim Mathies [:jimm] from comment #3)
> > > Can you test this change on windows 7 aero basic to be sure the command
> > > buttons have the correct dimensions?
> > 
> > I tested on aero and classic, but I'll check aero basic. Out of curiosity,
> > is there a particular reason that is more likely than the others to have
> > issues?
> 
> Ugh. Yeah, that doesn't work well. I'll put up a screenshot... any idea why?

It's a quirk in Windows, they don't calculate the button metrics until a window is displayed that needs them. Prior to this if you query for these values what you get back are the classic desktop button dims. Why the window in question needs to be activated is a mystery. This all appears to be caused by optimizations in the handling of aero basic drawing and potentially some sort of Windows bug.

The whole purpose of this crazy code is to create and display a visible window before the main window is created that Windows accepts as one that requires these button numbers.

We could file a support request on this, I've always been curious if they could help us find a suitable work around by looking at the actually code.

Comment 8

[Justin Dolske [:Dolske]]

In lieu of a more elegant workaround, could we just hardcode a usually-correct value for this case? Then update that value once we have a suitable window? (Or not, does this value actually vary in practice?)

Comment 9

[Jim Mathies [:jimm]]

(In reply to Justin Dolske [:Dolske] from comment #8)
> In lieu of a more elegant workaround, could we just hardcode a
> usually-correct value for this case? Then update that value once we have a
> suitable window? (Or not, does this value actually vary in practice?)

Need to check if the main window calculates the values once displayed.. we don't let windows render command buttons on it so it might not and I don't remember from when I worked on this code.

However lets say we can get these numbers from the main window, if we update them after the window is displayed we would then have to force a repaint of chrome right at startup, and that would hit startup perf. :/ I *do* remember running into that problem. :)

Comment 10

[:Gijs (he/him)]

(In reply to Jim Mathies [:jimm] from comment #9)
> (In reply to Justin Dolske [:Dolske] from comment #8)
> > In lieu of a more elegant workaround, could we just hardcode a
> > usually-correct value for this case? Then update that value once we have a
> > suitable window? (Or not, does this value actually vary in practice?)
> 
> Need to check if the main window calculates the values once displayed.. we
> don't let windows render command buttons on it so it might not and I don't
> remember from when I worked on this code.
> 
> However lets say we can get these numbers from the main window, if we update
> them after the window is displayed we would then have to force a repaint of
> chrome right at startup, and that would hit startup perf. :/ I *do* remember
> running into that problem. :)

When we discussed this over IRC, we did say we think we could do this *everywhere* except aero basic, which would already be a win (this includes win7 classic/hcm and all of winxp). I also contacted folks at MS to ask about the aero-basic-specific bugginess. Hopefully they can help.

Comment 11

[:Gijs (he/him)]

Comment on attachment 8673562 [details]
MozReview Request: Bug 1203574 - use ShowNA for everything except aero lite to get caption button metrics, r?jimm

Bug 1203574 - use ShowNA for everything except aero lite to get caption button metrics, r?jimm

Comment 12

[Jim Mathies [:jimm]]

Comment on attachment 8673562 [details]
MozReview Request: Bug 1203574 - use ShowNA for everything except aero lite to get caption button metrics, r?jimm

https://reviewboard.mozilla.org/r/21947/#review20081

::: widget/windows/nsUXThemeData.cpp:204
(Diff revision 2)
> +  // for both of these

The reason is that this is the non-composited version of the aero theme. :) This isn't unexpected, so please remove this part of your comment. I would however appreciate a note here about aerolite which I had to look up, it's a special version of aero for server 2012/2013.

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/176c2e93b322

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/176c2e93b322