376408 - [vista] corners of tooltips not transparent (opacity wrong)

Status: VERIFIED FIXED

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

Description

[Luca D.]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; de; rv:1.8.1.3) Gecko/20070309 Firefox/2.0.0.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; de; rv:1.8.1.3) Gecko/20070309 Firefox/2.0.0.3

Vista tool tips are not rectangular, they're rounded at the corners.
With the default theme they appear as rectangle, the empty space at the corners is filled with black instead of being transparent. It's just a little cosmetic problem. (I'll add a screenshot as attachment)

Reproducible: Always

Steps to Reproduce:
1. Hover a toolbar button to see a tool tip



Default theme

Comment 1

[Luca D.]

Attachment: Screenshot

Comment 2

[Samuel Sidler (old account; do not CC)]

Don't see any dupes, but I definitely see this issue. Confirming...

Comment 3

[Jim Mathies [:jimm]]

Attachment: Comparision shots

Just confirming and adding a clearer screenshot. It seems like it wouldn't be that noticible but it is, you really pick up on it. (imho)

Comment 4

[Gábor Stefanik]

The problem is that they *are* opaque in Firefox. Fixing summary.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

As you can see in attachment 330811 [details] the corners can also have a white background.

Comment 7

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

If we can't fix the window issue easily (and I'm not sure how we'd do that -- we'd have to create the right window style possibly?), then we should stop using the rounded corners, IMO.

Comment 8

[(no longer active)]

Shouldn't this be as simple as creating a rounded-corner rectangle region (CreateRoundRectRgn [1]) and setting it as the tooltip window's region (SetWindowRgn [2])?  I just don't know where to do that, or I'd come up with a patch...

[1] http://msdn.microsoft.com/en-us/library/ms536690(VS.85).aspx
[2] http://msdn.microsoft.com/en-us/library/ms534845(VS.85).aspx

Comment 9

[James Ross]

I don't think it's that simple - the OS theme decides what's transparent and what's not; I expect you'd need to make the window an alpha blended window that's entirely transparent, then draw the theme into it. (I don't think it's necessarily limited to Vista in that regard, either.)

Comment 10

[(no longer active)]

(In reply to comment #9)
> I don't think it's that simple - the OS theme decides what's transparent and
> what's not; I expect you'd need to make the window an alpha blended window
> that's entirely transparent, then draw the theme into it. (I don't think it's
> necessarily limited to Vista in that regard, either.)

Hmmm, using SetWindowRgn does not make some areas transparent, it excludes them from the window content, so effectively whatever gets painted on them is not visible.  This is of course assuming that tooltips in Gecko on Windows are implemented using a separate window with its own HWND, etc.

Comment 11

[Conley Read]

This bug is still out there. The background of the rectangle is now white, however. Bringing Vladimir's comment again; instead of implementing rounded tooltips, how about square ones on Windows? At least they would render as intended and not sully the first impression (IMHO).

Comment 12

[Alex Faaborg [:faaborg] (Firefox UX)]

this bug is eligible for bug 462080

Comment 14

[Jim Mathies [:jimm]]

Attachment: tooltip region theme fix v.1

This addresses the problem, but unfortunately it’s not a generic solution. Some background – with the exception of text tooltips are drawn by the theme module, including the background gradient and the border. Tooltip windows do not use transparency, but they do use a window clipping region to clip out certain parts of the window from being displayed. (This should be set via SetWindowRgn.)  Setting the window region does two things – it completely clips out the corners of the tooltip window, and it causes windows to adjust the drop shadow inward so that both the tooltip corners and the surrounding drop shadow effect are displayed correctly. 

Window regions are peculiar, they need to be set and cleared at specific times – on window creation and any time the window is resized or moved. To fix the tooltip problem, the region we have to set doesn’t come from a generic region creation routine like CreateRoundedRectRgn, it comes from the native windows theme library though GetThemeBackgroundRegion.

A generic solution in nsWindow would involve calls into the theme lib to check to see if the window’s widget is themed. Unfortunately to do that we need to call nsNativeThemeWin’s GetThemePartAndState with a frame pointer and some style information indicating the type of widget being displayed. nsWindow though doesn’t have access to that info AFAICT.

So this is start on a fix. It only applies regions on tooltips, and it hard codes a couple tooltip constants that would come from GetThemePartAndState for NS_THEME_TOOLTIP. I’ll keep looking at this but I thought I’d post what I have in case anyone has any ideas / comments.

Comment 15

[Jim Mathies [:jimm]]

Attachment: before and after comparison

Comment 16

[Jim Mathies [:jimm]]

Attachment: tooltip region theme fix v.2

This is cleaned up version of this patch. I have an experimental patch that handles this in a more generic way which passes style information down through the widget interface. But in the end I decided that since transparency isn't effected in this case by the style's 'part' and 'state', there's no need to bring that information down. ['part' is the part of the overall widget we are working on (left border, right border, button face, ..) and 'state' is the state of the part (hover, disabled, enabled, ..)] In the current case with tooltips we are dealing with the entire window only, so these values are set to defaults.

Somewhere down the line widgets might have window clipping regions that change based on state, in which case nsWindow would need access to an nsIFrame pointer and the widget type. (See nsNativeThemeWin's GetThemePartAndState for more info.)

Comment 17

[Jim Mathies [:jimm]]

Attachment: tooltip region theme fix v.3

This is a slight update to the original that adds support for popup panels, which addresses a the same issue in a few other window - 

http://img234.imageshack.us/img234/4803/31416834jb2.jpg
https://bugzilla.mozilla.org/attachment.cgi?id=330811

Comment 18

[(no longer active)]

Jim, These screenshots show the problem (not the solution), right?

Comment 19

[Jim Mathies [:jimm]]

(In reply to comment #18)
> Jim, These screenshots show the problem (not the solution), right?

Correct, this patch corrects these issues.

Comment 20

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 363210 [details] [diff] [review]
tooltip region theme fix v.3

sorry, missed the review request on this -- this looks fine

Comment 21

[Robert Strong (they/them - no direct email)]

Jim, I took a look at your patch and it looks like it would be a good candidate for Firefox 3.5. Are you ok with this?

Comment 22

[Jim Mathies [:jimm]]

(In reply to comment #21)
> Jim, I took a look at your patch and it looks like it would be a good candidate
> for Firefox 3.5. Are you ok with this?

Sure, let me get it landed and lets request approval.

Comment 23

[Jim Mathies [:jimm]]

landed on mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/593d36f8c437

Comment 24

[Atsushi Sakai]

Has this caused Bug 485484?

Comment 25

[Jim Mathies [:jimm]]

(In reply to comment #24)
> Has this caused Bug 485484?

It shouldn't since the region settings are only applied when nsUXThemeData::sIsVistaOrLater is true.

Comment 26

[Jim Mathies [:jimm]]

(In reply to comment #25)
> (In reply to comment #24)
> > Has this caused Bug 485484?
> 
> It shouldn't since the region settings are only applied when
> nsUXThemeData::sIsVistaOrLater is true.

It might be the clear region call, which is applied to all windows. But I wouldn't think clearing the window region would cause problems with xp's native theme drawing of window borders. We could test to find out.

Comment 27

[Jim Mathies [:jimm]]

Attachment: xp touchup patch

Touchup patch to address xp border bug in bug 485484.

Comment 28

[Jim Mathies [:jimm]]

xp touchup patch landed on mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/9e33f1b0a4dc

Comment 29

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Jim, the rounded corners still show two white pixels for me with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20090403 Minefield/3.6a1pre. Should this be handled in another bug?

Comment 30

[Jim Mathies [:jimm]]

(In reply to comment #29)
> Jim, the rounded corners still show two white pixels for me with Mozilla/5.0
> (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20090403
> Minefield/3.6a1pre. Should this be handled in another bug?

You know I noticed this as well, and was able to deal with it somewhat by doing a double pass on the rendering. But in the end I took that out because if you look around at some other tooltips on Vista, especially down on the quick launch bar and other applications, you'll see the same anomaly. So I came to conclusion this was a short coming in the windows themeing GetBackgroundRegion code, and not in our use of it.

Comment 31

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thanks for the explanation Jim. I tried to make it visible with IE7 and its easy when hovering the home button. So everything we can do is fine.

Verified fixed with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20090403 Minefield/3.6a1pre

Comment 32

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 363210 [details] [diff] [review]
tooltip region theme fix v.3

a191=beltzner for this and the XP touchup patch

Comment 33

[Jim Mathies [:jimm]]

1.9.1:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/0141e2c925b7

Comment 34

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified on 1.9.1 with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b4pre) Gecko/20090414 Shiretoko/3.5b4pre ID:20090414044032

Comment 35

[:Gavin Sharp [email: [e-mail]]]

I had to back this out on the branch to fix bug 488311:
https://hg.mozilla.org/releases/mozilla-1.9.1/rev/bdb42566e9a8

Comment 36

[Nochum Sossonko [:Natch]]

Is there anyway this can reland on branch?

Comment 37

[Alex Faaborg [:faaborg] (Firefox UX)]

This bug's priority relative to the set of other polish bugs is:
P2 - Polish issue that is in a secondary interface, occasionally encountered, and is easily identifiable.

tooltips are secondary UI, but this bug was also easily identifiable.

Comment 38

[mixxster]

I'm no expert in bugzilla, but shouldn't this not be "VERIFIED FIXED" since it never landed?

Comment 39

[Samuel Sidler (old account; do not CC)]

This landed on trunk successfully. It was only backed out on branch. It will be fixed in Firefox 3.6.