307204 - Support for transparent windows on Mac OS X

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Anand Aiyer]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; SV1; .NET CLR 1.1.4322)
Build Identifier: 

Support for Translucent windows in Mac is needed.

Reproducible: Always

Steps to Reproduce:

Comment 1

[Ben Fowler]

If by translucent windows you mean this Cocoa feature in Quartz:
 ,
then I would probably vote against it. Translucency without speed/performance
penalties seems to require 2D acceleration 
http://www.macintouch.com/mosxreaderjaguar02.html, and will therefore leave
many Mac users behind http://www.lowendmac.com/myturn/02/1031.html ,

Comment 2

[Anand Aiyer]

Support for translucent windows already exists in other platforms (windows and 
linux) and has been pretty stable for a while, we just need to have it for mac 
so that XUL apps can use this feature on mac. "hidechrome" also doesnt work in 
mac. I don't think we need to use Quatz for this, translucent windows in mac 
has been around far longer than in other platforms and is used by many apple 
apps like iTunes, and I dont think this added feature will leave many users 
behind.

Comment 3

[mig]

If you call window.open with "titlebar=no" you essentially get the hidechrome feature under the mac.  However, insofar as our use of xulrunner is concerned, I would agree that the API should be the same across all platforms.

Comment 4

[mig]

Oh, also, I don't know the mac graphics APIs well enough, but if this is an issue of 8 bit alpha vs 1 bit alpha (layers vs regions, like Bug 298889), it would be very useful as an interim solution to at least have "background-color: transparent;" working.

Comment 5

[Kathleen :Brade]

confirming Mac needs this fix

Comment 8

[Stuart Parmenter]

we probably want this for platform parity

Comment 9

[Colin Barrett [:cbarrett]]

It's a single method call to set the opacity of a window in Cocoa.

Comment 10

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

(In reply to comment #9)
> It's a single method call to set the opacity of a window in Cocoa.

It's not the opacity of the whole window we want to set, though; we basically want to draw RGBA to a window's region, which should be fully transparent by default.

Comment 11

[Colin Barrett [:cbarrett]]

(In reply to comment #10)
> (In reply to comment #9)
> > It's a single method call to set the opacity of a window in Cocoa.
> 
> It's not the opacity of the whole window we want to set, though; we basically
> want to draw RGBA to a window's region, which should be fully transparent by
> default.

Ah, well in that case, you're going to want to setOpaque:NO, and probably remove setBackgroundColor:[NSColor whiteColor] in nsCocoaWindow, and probably fiddle with the opacity of the plain NSView that our content views are made subviews of (except in the case of popups)

Comment 12

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

You (we?) really want this now that the better autoscroll implementation from SeaMonkey was backported to toolkit.

Comment 13

[Colin Barrett [:cbarrett]]

A couple days ago I got an email from a guy developing a XUL app saying the lack of this on Mac is a showstopper for him.

Is it possible we could do this for 1.9.1 (is that even happening)? It doesn't look like on the Mac side we'll have time to get to wanted-1.9 things, and this is 1.9-.

Comment 14

[Mark Finkle (:mfinkle) (use needinfo?)]

This is one of the most commonly asked for features from mainly xul-app developers and some extension developers. As Stuart mentions in comment #8, this is a glaring platform parity problem too.

I was hoping for 1.9, but I could live with 1.9.1

Comment 15

[Benjamin Smedberg]

Not sure 1.9.1 scope has been fully decided, but IMO this would definitely be in scope for 1.9.1

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We could take this for 1.9 if someone can find a contributor to implement it. It's not hard.

Comment 17

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

This is also required by bug 303110, which is a really-really-want for Firefox 3, and potentially a blocker.

Comment 18

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

In addition to unified toolbar, it would be useful to have this in 1.9 so Mozilla Labs can launch a keyboard UI prototype during the Firefox 3 life cycle.
(http://labs.mozilla.com/2007/07/the-graphical-keyboard-user-interface/)

really-really-really want

Comment 19

[Håkan Waara]

Attachment: I have a XUL octopus living on my desktop!

Comment 20

[Håkan Waara]

Attachment: Patch v0.7 (proof of concept)

So here's a patch that works. Functionally it's smooth as silk, but I have an architectural problem with it:

nsIWidget's SetWindowTranslucency/GetWindowTranslucency are called on a nsChildView's widget impl, as well as on the nsCocoaWindow's widget impl, so the nsChildView needs to change the window's opacity. It's not really nice for a view to change its window's properties without knowing you are allowed to. For example, is it really OK to suddenly make an embeddor's native window translucent?

The easiest solution would be if nsChildView could easily find the nsCocoaWindow in which it is in. I tried using mParentWidget but apparently this doesn't lead up to it.

I see two other simple solutions:

1. implement a new mozWindow API for this, to make sure only mozWindows' opacity can be changed by the views. 

2. always make nsCocoaWindow's NSWindow's inherit from a dummy NSWindow class so we simply can check on runtime that this is indeed "our" window, before doing any changes to it.

I'd prefer #2, but ideally there's simply a way for my widget to find the nsCocoaWindow widget. Please enlighten me if that is the case.

Another open issue: Should we turn off shadows when transparency is on?

Last note: seems like I didn't have to implement nsIWidget's UpdateTranslucentWindowAlpha(). Is it necessary?

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

> I'd prefer #2, but ideally there's simply a way for my widget to find the
> nsCocoaWindow widget. Please enlighten me if that is the case.

Colin and Josh should be able to help here. If there's no way to currently find the nsCocoaWindow widget, we should be able to create one.

> Another open issue: Should we turn off shadows when transparency is on?

I'm not sure, but I think probably yes. (Maybe there should be a way for the author to control this, perhaps based on window types.)

The title bar should also go away.

> Last note: seems like I didn't have to implement nsIWidget's
> UpdateTranslucentWindowAlpha(). Is it necessary?

No, that is never called anymore and should actually be removed everywhere.

It looks to me from your screenshot like the updating of the window's alpha mask is lagging behind the actual drawing of the octopus, is that correct? If so, that's unfortunate, but we should still take the patch because most uses of this will not require animation of the alpha mask.

Have you tested translucency? Try replacing the octopus with a translucent PNG...

Comment 22

[Colin Barrett [:cbarrett]]

(In reply to comment #21)
> > Another open issue: Should we turn off shadows when transparency is on?
> 
> I'm not sure, but I think probably yes. (Maybe there should be a way for the
> author to control this, perhaps based on window types.)

Recalcuating shadows is expensive and slow, and doesn't really work all that well. We have tons of problems with them in Adium.
 
> The title bar should also go away.

I agree -- although once the window is set up to have a titlebar, it can't be removed without recreating the window, and vice versa.
 
> It looks to me from your screenshot like the updating of the window's alpha
> mask is lagging behind the actual drawing of the octopus, is that correct? If
> so, that's unfortunate, but we should still take the patch because most uses of
> this will not require animation of the alpha mask.

I suspect that turning off shadows will fix that.

Comment 23

[Colin Barrett [:cbarrett]]

(In reply to comment #20)
> The easiest solution would be if nsChildView could easily find the
> nsCocoaWindow in which it is in. I tried using mParentWidget but apparently
> this doesn't lead up to it.

"Apparently"?

Comment 24

[Colin Barrett [:cbarrett]]

(In reply to comment #23)
> (In reply to comment #20)
> > The easiest solution would be if nsChildView could easily find the
> > nsCocoaWindow in which it is in. I tried using mParentWidget but apparently
> > this doesn't lead up to it.
> 
> "Apparently"?

from nsChildView [[self nativeWindow] delegate] will be of class WindowDelegate, and [[[self nativeWindow] delegate] geckoWindow] will get you the nsCocoaWindow.

Comment 25

[Håkan Waara]

Attachment: Happy transparent and animated octopuses living side by side

The bad alpha mask was the window not updating its shadow. Turning that off did the trick. Transparency works fine, as you can see in the screenshot.

Unfortunately removing the titlebar will, as Colin points out, be difficult... We'd have to recreate the entire window, setting it all up again. And we don't want to loose any information on the current window state... I don't know how much work that will be. Hopefully not too many things would keep stale pointers to it. :-/

Comment 26

[Håkan Waara]

Hrm, of course, if the widget would know upon creation that it wanted to be translucent, this would be another deal. I think things would have to change in view/gfx then, roc?

Comment 27

[Håkan Waara]

Attachment: Patch v1.0

Changes:

* Let nsCocoaWindow take care of changing the window, and make sure we don't change an embeddor's window.
* Turn on/off shadows properly
* Remove UpdateTranslucentWindowAlpha() API from nsIWidget
* Remove old drawRect:/quickdraw hack that shouldn't be needed anymore, now that Cocoa can properly find out whether our view is opaque/transparent.

Turns out that the reason I had a titlebar was that I tested this with open('https://bugzilla.mozilla.org/attachment.cgi?id=111901', '', 'chrome,dialog=no') -- basically, creating a window with the titlebar on by default.

If I do open('https://bugzilla.mozilla.org/attachment.cgi?id=111901', '', 'chrome,dialog=no,titlebar=no') the window opens without a titlebar.

So I think the titlebar issue is actually a non-issue, unless roc meant that if a window becomes transparent its titlebar should always hide? If so, I suggest a new bug for that. Removing the titlebar on-the-fly is not possible in Cocoa, so we would need a mechanism for being able to recreate the native window on demand.

So I think this is ready for review - anyone disagree?

Comment 28

[Håkan Waara]

Attachment: Screenshot of titlebarless transparent window

Comment 29

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yeah, we should get this in, it looks good. Disabling the title bar via window.open is fine. In fact it's good because it gives the author the option.

-  // Workaround for the fact that NSQuickDrawViews can't be opaque; see if the rect
-  // being drawn is covered by a subview, and, if so, just bail.
-  if ([self isRectObscuredBySubview:aRect])
-    return;

Why are you removing this? I'm not sure why it's there myself...

Comment 30

[Håkan Waara]

Comment on attachment 277094 [details] [diff] [review]
Patch v1.0

(In reply to comment #29) 
> Why are you removing this? I'm not sure why it's there myself...

CC sfraser, maybe he can clarify and say if I'm wrong here.

This is complicated -- I'm not sure I understand the details of it myself, here's from what I gather:

Pre-quartz, when we were using Quickdraw, we had to mark all ChildViews as transparent, even if they were not. (History in bug 166932.) 

Now that we can correctly mark them opaque/transparent based on the situation, Cocoa will do the clipping for us and NOT call our drawRect: if the view is hidden behind another view.

However, ChildView's isOpaque today always returns YES, so Cocoa should do its thing and the workaround would not do anything (other than slow down drawing). 

Discussing this with smorgan, it seems like our views always being opaque may be wrong, because some views are still using QuickDraw. Stuart filed bug 392620 for this.

I now realize that removing isOpaque in my patch is wrong. It will make the view transparent by default, which is bad for perf. We should mark the view as opaque in all cases unless the window is explicitly set to transparent, for perf reasons.

I'll make a new patch.

Comment 31

[Håkan Waara]

Attachment: Patch v1.1

Changes:

* Always mark our ChildViews as opaque unless our window is explicitly told to be translucent, for performance.

Comment 32

[Kathleen :Brade]

Nit: Please begin comments that are sentences begin with a capital letter.

Example:
+NS_IMETHODIMP nsChildView::SetWindowTranslucency(PRBool aTranslucent)
+{
+    BOOL currentTranslucency = ![[mView nativeWindow] isOpaque];
+    nsresult rv = NS_OK;
+    if (aTranslucent != currentTranslucency) {
+        // find out if this is ... WindowDelegate. if it is,
+        // let the nsCocoaWindow take ... window's translucency.


THANK YOU!!!

Comment 33

[Asaf Romano (gone)]

So, I was kinda hoping this would replace the popups-specific hack with a one-liner style rule in Pinstripe ;)

Comment 34

[Simon Fraser [no longer active]]

Comment on attachment 277122 [details] [diff] [review]
Patch v1.1

>Index: widget/src/cocoa/nsChildView.h
>===================================================================
>RCS file: /cvsroot/mozilla/widget/src/cocoa/nsChildView.h,v
>retrieving revision 1.70
>diff -u -8 -p -r1.70 nsChildView.h
>--- widget/src/cocoa/nsChildView.h	16 Aug 2007 20:30:50 -0000	1.70
>+++ widget/src/cocoa/nsChildView.h	17 Aug 2007 18:50:36 -0000
>@@ -300,16 +300,20 @@ public:
>   NS_IMETHOD        SetAnimatedResize(PRUint16 aAnimation);
>   NS_IMETHOD        GetAnimatedResize(PRUint16* aAnimation);
> 
>   // nsIPluginWidget
>   NS_IMETHOD        GetPluginClipRect(nsRect& outClipRect, nsPoint& outOrigin, PRBool& outWidgetVisible);
>   NS_IMETHOD        StartDrawPlugin();
>   NS_IMETHOD        EndDrawPlugin();
>   
>+  // translucency
>+  NS_IMETHOD        GetWindowTranslucency(PRBool& aTranslucent);
>+  NS_IMETHOD        SetWindowTranslucency(PRBool aTranslucent);

These names are wrong. If you are setting the translucency, you're really passing
an alpha value that ranges between 0 and 1. If you are just making it transparent,
say so. It's really "SetHasTransparentBackground()". And I agree with nsIWidget
being the wrong place for this; it's only relevant on the top-level window.

>Index: widget/src/cocoa/nsChildView.mm
>===================================================================

> - (BOOL)isOpaque
> {
>-  // this will be NO when we can do transparent windows/views
>+  // we're opaque, unless our window is explicitly marked as transparent. 
>+  if (mGeckoChild) {
>+    PRBool isTransparent = PR_FALSE;
>+    mGeckoChild->GetWindowTranslucency(isTransparent);
>+    return !isTransparent;
>+  }
>   return YES;
> }

I think this is wrong. Whether an NSView is opaque has nothing to do with
window transparency. The View Manager should really be the one telling
a given nsIWiget whether it's opaque or not. The only view for which you may
care about opaqueness is the window's root view.


> // The display system has told us that a portion of our view is dirty. Tell
> // gecko to paint it
> - (void)drawRect:(NSRect)aRect
> {
>   PRBool isVisible;
>   if (!mGeckoChild || NS_FAILED(mGeckoChild->IsVisible(isVisible)) || !isVisible)
>     return;
>-  
>-  // Workaround for the fact that NSQuickDrawViews can't be opaque; see if the rect
>-  // being drawn is covered by a subview, and, if so, just bail.
>-  if ([self isRectObscuredBySubview:aRect])
>-    return;

It's fine to remove this. All the NSQuickDrawView-related hacks should be removed.

>Index: widget/src/cocoa/nsCocoaWindow.mm
>===================================================================

>+NS_IMETHODIMP nsCocoaWindow::GetWindowTranslucency(PRBool& aTranslucent)
>+{
>+  aTranslucent = ![mWindow isOpaque];   
>+  return NS_OK;
>+}
>+
>+NS_IMETHODIMP nsCocoaWindow::SetWindowTranslucency(PRBool aTranslucent)
>+{
>+  BOOL currentTranslucency = ![mWindow isOpaque];
>+  if (aTranslucent != currentTranslucency) {
>+    [mWindow setBackgroundColor:(aTranslucent ? [NSColor clearColor] : [NSColor whiteColor])];
>+    [mWindow setHasShadow:!aTranslucent];
>+    [mWindow setOpaque:!aTranslucent];
>+  }
>+  return NS_OK;
>+}

I've seen the discussions relating to shadows, but I don't think shadows should
be indescriminately turned off for windows with transparent backgrounds.

Did you experiment with -[NSWindow invalidateShadow] ?

Comment 35

[Simon Fraser [no longer active]]

Since you're touching nsIWidget.h, maybe use this opportunity to rename GetWindowTranslucency?

Comment 36

[Håkan Waara]

(In reply to comment #34)
> >+  // translucency
> >+  NS_IMETHOD        GetWindowTranslucency(PRBool& aTranslucent);
> >+  NS_IMETHOD        SetWindowTranslucency(PRBool aTranslucent);
> 
> These names are wrong. If you are setting the translucency, you're really
> passing
> an alpha value that ranges between 0 and 1. If you are just making it
> transparent,
> say so. It's really "SetHasTransparentBackground()". And I agree with nsIWidget
> being the wrong place for this; it's only relevant on the top-level window.

Sure, I can do the renaming. Anyone against Set/GetHasTransparentBackground? 

I agree that ideally the API would be on the top-level widget. When roc changes widget to be one per-window (or was it one-per-content-view? I don't remember), this will almost be true. :)

> > - (BOOL)isOpaque
> 
> I think this is wrong. Whether an NSView is opaque has nothing to do with
> window transparency. The View Manager should really be the one telling
> a given nsIWiget whether it's opaque or not. The only view for which you may
> care about opaqueness is the window's root view.

What about all subviews that may contain transparent content? Since we're not told about that on a per-widget basis, we need to make sure the NSViews *can* contain transparent stuff when the window is marked as such, right? I agree that it would be a nice optimization if the widget knew that it could be opaque in the "translucent window mode".

> I've seen the discussions relating to shadows, but I don't think shadows should
> be indescriminately turned off for windows with transparent backgrounds.
> 
> Did you experiment with -[NSWindow invalidateShadow] ?

I left it off by default: Colin's "expensive, slow and buggy" in comment 22, and because it is simpler for now (it will be hard to know when to invalidate the shadow to fix the redrawing problems in comment 21, without invalidating on every drawRect: which will be bad perf-wise :-/).

But this is something that the author should be able to decide. We should expose a new window open() property, something like shadow=on/off. The widget part of fixing that is trivial, we just need the code for the new property basically. Let's file a spin-off bug for that if people agree.

Comment 37

[Håkan Waara]

(In reply to comment #33)
> So, I was kinda hoping this would replace the popups-specific hack with a
> one-liner style rule in Pinstripe ;)

Not sure I understand what you mean. Can you elaborate?

Comment 38

[Robert O'Callahan (:roc) (email my personal email if necessary)]

> And I agree with nsIWidget
> being the wrong place for this; it's only relevant on the top-level window.

We don't have a cross-platform interface specifically for top-level windows; if we did I'd use it.

The compositor work planned for after 1.9 will mostly solve this because the only nsIWidgets will be top-level windows and plugins. We might in fact give plugins their own interface instead of nsIWidget.

Gecko assumes that all non-top-level windows are opaque and draws accordingly. Don't violate that.

Comment 39

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Håkan, we've seen a lot of trouble with titlebar=no windows and keyboard events... Like they don't get sent to the window. Try adding a textbox to your testcase :)

I'm going to file that bug in a sec.

Comment 40

[Neil Deakin]

> 
> I agree that ideally the API would be on the top-level widget.

I assume by 'top-level' you mean any window, rather than the 'eWindowType_toplevel' type, as 'eWindowType_dialog' and eWindowType_popup' (and possibly eWindowType_sheet?) should also support transparency.

Comment 41

[Robert O'Callahan (:roc) (email my personal email if necessary)]

yeah

Comment 42

[Neil Deakin]

This patch doesn't seem to work for transparent popups though, only toplevel windows. Transparent popups are desired for a firefox UI notification feature.

Comment 43

[Håkan Waara]

(In reply to comment #42)
> This patch doesn't seem to work for transparent popups though, only toplevel
> windows. Transparent popups are desired for a firefox UI notification feature.

I think that is trivial to fix. But I'd need a testcase; how did you test it?

The latest status on this was that I was trying hard to make a nsChildView know whether it is toplevel (without having to traverse the native view hierarchy every time). If anyone know of a simple way to do that, it would be helpful.

Comment 44

[Neil Deakin]

Attachment: test for transparent popups

Comment 45

[Colin Barrett [:cbarrett]]

(In reply to comment #43)
> The latest status on this was that I was trying hard to make a nsChildView know
> whether it is toplevel (without having to traverse the native view hierarchy
> every time). If anyone know of a simple way to do that, it would be helpful.

Best guess is if it's a top level nsChildView, its parent will be [window
contentView]

http://mxr.mozilla.org/seamonkey/source/widget/src/cocoa/nsChildView.mm#402

But I would check and make sure.

Comment 46

[Håkan Waara]

(In reply to comment #45)
> (In reply to comment #43)
> > The latest status on this was that I was trying hard to make a nsChildView know
> > whether it is toplevel (without having to traverse the native view hierarchy
> > every time). If anyone know of a simple way to do that, it would be helpful.
> 
> Best guess is if it's a top level nsChildView, its parent will be [window
> contentView]

That might work, but it's fragile; as soon as you put the view somewhere else in the hierarchy it would break. I'm not even sure we're the contentView, or its subview always today... Also, think about embedding situations.

That's why it would be more robust to go the gecko widget way and find out if we're the top-level widget (except for the nsCocoaWindow, which probably is the truly top-level nsIWidget). One would guess that nsChildView's mParentWidget would be the nsCocoaWindow in that case, but alas, it's null...

Comment 47

[Brian King [:kinger]]

v1.1 of the patch is not applying cleanly for me on the trunk.

Could that code have changed so much?

mac-g5:~/snack/mozilla blaz$ patch -p0 -u -i translucent.diff 
patching file widget/public/nsIWidget.h
Hunk #1 FAILED at 90.
1 out of 2 hunks FAILED -- saving rejects to file widget/public/nsIWidget.h.rej
patching file widget/src/cocoa/nsChildView.h
Hunk #1 succeeded at 297 with fuzz 2 (offset -3 lines).
patching file widget/src/cocoa/nsChildView.mm
Hunk #1 succeeded at 662 (offset 5 lines).
Hunk #2 succeeded at 2110 (offset -7 lines).
Hunk #3 succeeded at 2242 (offset -7 lines).
patching file widget/src/cocoa/nsCocoaWindow.h
Hunk #1 FAILED at 187.
1 out of 1 hunk FAILED -- saving rejects to file
widget/src/cocoa/nsCocoaWindow.h.rej
patching file widget/src/cocoa/nsCocoaWindow.mm
Hunk #1 succeeded at 293 (offset -3 lines).
Hunk #2 succeeded at 608 (offset -3 lines).
patching file widget/src/gtk2/nsWindow.cpp
patching file widget/src/gtk2/nsWindow.h
patching file widget/src/windows/nsWindow.cpp
Reversed (or previously applied) patch detected!  Assume -R? [n] 
Apply anyway? [n] n
Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file
widget/src/windows/nsWindow.cpp.rej
patching file widget/src/windows/nsWindow.h
Hunk #1 FAILED at 210.
1 out of 1 hunk FAILED -- saving rejects to file
widget/src/windows/nsWindow.h.rej
patching file widget/src/xpwidgets/nsBaseWidget.cpp
patching file widget/src/xpwidgets/nsBaseWidget.h

Comment 48

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #43)
> The latest status on this was that I was trying hard to make a nsChildView know
> whether it is toplevel (without having to traverse the native view hierarchy
> every time). If anyone know of a simple way to do that, it would be helpful.

Can't you use nsIWidget::GetWindowType on its nsIWidget and check for eWindowType_toplevel, eWindowType_dialog, and eWindowType_popup?

I think we still really want this for 1.9, but it has to happen soon...

Comment 49

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

>I think we still really want this for 1.9, but it has to happen soon...

Yeah to put this in context, the UX team has multiple changes to primary UI planned in Firefox 3 that are depending on this bug getting resolved.  So we really-really want this for 1.9.

Comment 50

[Håkan Waara]

OK, I will try to find some time for this this week.

Comment 51

[Håkan Waara]

Ok, I got normal non-popup transparent windows working as they should.

The remaining issue is when I'm trying to deal with transparent popups (comment 42).

In the translucent popup case, the call to SetWindowTranslucency() goes to the popup's nsCocoaWindow. However, later when the paint event is dispatched, and the popup is painted, GetWindowTranslucency() is called on the nsChildView of the *parent* window of the popup.

The parent window's main view, which is not transparent, returns "no, I'm not transparent", and the popup then also becomes non-transparent. 

It seems that all event handling for popups is handled by tricking gecko that it is in fact handling the parent window, and not the popup, so all these calls are done by gecko on the wrong nsIWidget.

http://mxr.mozilla.org/seamonkey/source/widget/src/cocoa/nsChildView.mm#1333

Ideas?

Comment 52

[Simon Fraser [no longer active]]

General comment: are there security concerns with allowing windows with an alpha value of 0?

Comment 53

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Is the functionality limited to chrome popups on other platforms?

Comment 54

[Håkan Waara]

Only chrome windows can have a transparent background. This is enforced by gecko.

Comment 55

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The code says:
    // use the parent popup's widget if there is no view

so why isn't there a view? I think there should be...

Comment 56

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I mean a Gecko view, i.e., non-null GetClientData.

Comment 57

[Håkan Waara]

Attachment: WIP patch

This is what I have so far. 

I discovered that popups get a black background color even if I hardcode all widgets to return true for translucency. I am testing popup transparency by launching firefox with the -chrome flag and Enn's testcase.

Comment 58

[Colin Barrett [:cbarrett]]

Usually that is a sign that your window doesn't have the opaque flag set correctly. You seem to be setting it -- try moving it be earlier in MakeWindowTranslucent, before you set the background color? 

Comment 59

[Håkan Waara]

(In reply to comment #58)
> Usually that is a sign that your window doesn't have the opaque flag set
> correctly. You seem to be setting it -- try moving it be earlier in
> MakeWindowTranslucent, before you set the background color? 

That made no difference.

Comment 60

[Colin Barrett [:cbarrett]]

Popups shouldn't have a weird forced 0.95 alpha anymore. If chrome wants the popup to be that way, they now can set it with this API, no?

In the case of context menus, we should be calling CGContextClearRect before drawing into them. see bug 391984.

Also, in looking at your patch, please make sure to follow the style of the files you're in -- for example, instance variables should be prefixed with m (for member). mIsTransparent, not isTransparent.

Comment 61

[Colin Barrett [:cbarrett]]

So I applied the WIP patch and nothing happened. The testcase Nail attached looked the same.

Post an updated/complete patch?

Comment 62

[Håkan Waara]

(In reply to comment #61)
> So I applied the WIP patch and nothing happened. The testcase Nail attached
> looked the same.

What do you mean by nothing happened? The patch doesn't fix transparency for popups. That's what's remaining...

Comment 63

[Colin Barrett [:cbarrett]]

According to your earlier comments, translucent popups were all black?

Perhaps I'm misunderstanding them.

Comment 64

[Matt Crocker]

Hi Håkan,

Your last patch seems to work just fine.  We're going to use it as-is for Songbird 0.3.

Thanks!

Comment 65

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

Requesting blocking since the UX team would like to use this for all of our notifications, the site menu, and the tab filter interface in Firefox 3.

Comment 66

[Josh Aas]

Once again UI people are requesting major platform features very late in the game. This poor planning can't continue.

Comment 67

[Josh Aas]

Håkan - can you update your patch so that it applies cleanly?

Comment 68

[Håkan Waara]

New patch coming up today or tomorrow, thank you for the patience

Comment 69

[Håkan Waara]

Attachment: Latest patch (-up8)

So, I'm pretty surprised. Either my previous testing was incorrect, or I'm doing something wrong now, or my upgrade to Leopard has affected something.

Because invisible popups now work!! I would like someone else to test and confirm, to make sure I'm not crazy.

The patch is not 100% done yet. But if it does work, there are trivial things left to do.

Here's how to test:

* See comment 27 for how to test "normal" XUL windows
* To test with Enn's translucent popup testcase, I open the Error console and run this: open('https://bugzilla.mozilla.org/attachment.cgi?id=278621', '', 'chrome=yes')

Let me know how it goes. If all is well, I will fix the minor remaining things and put up a new patch for review this week.

As a mental note, here are some things left to do off the top of my head:

* Go ahead with Simon's suggested renaming of SetWindowTranslucency
* Rev nsIWidget's IID
* Make sure it compiles on windows/linux after nsIWidget changes

Comment 70

[Håkan Waara]

So, to sum up: Please apply and test the patch, and we might get this in soonish.

Comment 71

[Shawn Wilsher :sdwilsh]

I went ahead and submitted the patch to the try server, so in an hour or so the build will show up here:
https://build.mozilla.org/tryserver-builds/

I won't be around to direct link to it, but hopefully it'll get more people testing it.

Comment 72

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

(In reply to comment #66)
> Once again UI people are requesting major platform features very late in the
> game. This poor planning can't continue.

You mean in August?  See comments 17 and 18, for example.  I don't think that's "very late" to be requesting features, nor would it have been too early to register your apparent objection to their request, but what do I know?

Comment 73

[Shawn Wilsher :sdwilsh]

Mac build with hwaara's patch:
https://build.mozilla.org/tryserver-builds/2007-10-31_11:[e-mail]-transparent_popup_fix/[e-mail]-transparent_popup_fix-firefox-try-mac.dmg

Comment 74

[Håkan Waara]

So I had a friend test this on Tiger: where translucent popups won't work. On Leopard it's working perfectly.

Comment 75

[Josh Aas]

Drive-by review, only things you didn't say in your comment 69...

First off, are you sure you are using the words "translucent" and "transparent" correctly and consistently in your comments and your code?

+NS_IMETHODIMP nsChildView::SetWindowTranslucency(PRBool aTranslucent)
+{
+  BOOL currentTranslucency = ![[mView nativeWindow] isOpaque];
+  nsresult rv = NS_OK;

The rv here isn't really used. You can just return NS_OK at the end.

As for the currentTranslucency variable, you can get rid of some double ! and drop that variable by just including the call to isOpaque in the 'if' test.

+    // XXXhakan: this should only be done on the screen showing the menubar

We should get your xxx comments taken care of.

A final note - can you break out the cleanup from this patch and we'll try to land that first? The stuff that can land whether or not we take the rest of the patch for Gecko 1.9. If you post that we can push it through now. I'm talking about stuff like removing UpdateTranslucentWindowAlpha from nsIWidget, there is some other stuff too.

Comment 76

[Mike Schroepfer]

Are we still depending on this for firefox 3 front end features?

Comment 77

[Josh Aas]

(In reply to comment #76)
> Are we still depending on this for firefox 3 front end features?

I didn't mark this blocking myself because I was waiting on an answer to that question. I was told it would be discussed in a Firefox team planning meeting this week but I never got an answer.

Comment 78

[Madhava Enros [:madhava]]

(In reply to comment #76)
> Are we still depending on this for firefox 3 front end features?
> 

The doorhanger notifications and tab-preview (because of rounded pop-up corners) needed this.  If either of those is in firefox 3, then yes.

Comment 79

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

The context has not changed.

Without this patch, UI is limited in terms of what it can do with theming on OSX, since we can't get transparencies. This will affect:
 - the ways we can style the starring panel
 - the ways we can style the identity box
 - the ability for tab-preview to render with rounded corners on mac
 - extension developers and any other future UI enhancements

We'll live without it, but as Alex said, it was a really-really want, and will really help our ability to look polished on the Mac.

> I didn't mark this blocking myself because I was waiting on an answer to that
> question. I was told it would be discussed in a Firefox team planning meeting
> this week but I never got an answer.

Who were you waiting on here? Generally speaking, if you're waiting for a front end decision and it's holding up your work, please email me or mconnor or both.

Comment 80

[Josh Aas]

That is what I did - I had talked to mconnor he told me to wait for the decision before marking blocking or not.

Comment 81

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

OK, I'll push on mconnor :)

Comment 82

[Colin Barrett [:cbarrett]]

This doesn't apply cleanly.

Comment 83

[Brian King [:kinger]]

Attachment: Update patch (-up8) 2007-12-04

Here is an updated (untested) patch.

Comment 84

[Colin Barrett [:cbarrett]]

Attachment: fix v2.0

This is an update of the previous patch, with review commends address and hakan's notes taken care of.

Popup windows on 10.4 are still not translucent. That's next.

Comment 85

[Colin Barrett [:cbarrett]]

One thing about the above patch:

When I renamed [Set|Get]WindowTranslucency to [Set|Get]HasTransparentBackground, I renamed the argument to be aTransparent, and moved local variables inside those functions to use transparent. I didn't do any more than that. I wonder if it is worth it to do more renaming -- that could be done in a separate patch or bug. What do people think?

Comment 86

[Colin Barrett [:cbarrett]]

Attachment: fix v2.1

So I wasn't able to fix the issue with popup windows (windows with eWindowType_popup) drawing without transparency on 10.4. I did get some data though:

- I tried not setting the content view of the popup to a child view, and instead set it to a custom view that just draws 50% transparent blue. Instead of drawing transparent when I loaded attachment 278621 [details] with -chrome, it would draw with a black background behind the blue, resulting in a dark blue color.

- I was able to get my custom blue view to draw correctly if I changed the features the window was created with to not be borderless but to instead have a titlebar. Worked fine then.

- I am unable to reproduce any of these symptoms in a test app.

So, if anyone can think of a reason why a borderless window would fail to draw with transparency, but marking that very same window as having a titlebar would allow it to draw correctly, feel free to chime in. It's like that window is somehow getting kicked off the double buffered path and is drawing non-retained? Changing at test app to draw non-retained did indeed reproduce the symptoms, but our windows are definitely using the buffered backing store. It's also very mysterious that this works completely fine on Leopard.

I talked with Josh and we agree that this should be checked in with the 10.4 issue, and that another bug should be created to track it. Hopefully someone will be able to figure it out eventually.

Comment 87

[Håkan Waara]

So in the end this is basically the same patch I attached on comment 69 + the minor changes done I suppose?

Wild guess: I wonder if changing deferred to YES when creating our windows make any difference. 

Also, does anyone have an Apple contact we can ask if there's any similar known bug in 10.4? Maybe they fixed it in 10.5 and just haven't back-ported the fix.

Comment 88

[Colin Barrett [:cbarrett]]

(In reply to comment #87)
> So in the end this is basically the same patch I attached on comment 69 + the
> minor changes done I suppose?

Yeah, I addressed any review comments / TODOs already posted in the bug, updated it to apply cleanly, and changed isTransparent to mIsTransparent.

> Wild guess: I wonder if changing deferred to YES when creating our windows make
> any difference. 

I don't think so, since windows are shown before they are asked to become transparent, I believe. I'll give it a try though.

> Also, does anyone have an Apple contact we can ask if there's any similar known
> bug in 10.4? Maybe they fixed it in 10.5 and just haven't back-ported the fix.

I can ping my contacts, but it's really a pretty vague issue and I am unable to reduce a testcase :(

Comment 89

[Colin Barrett [:cbarrett]]

(In reply to comment #88)
> > Wild guess: I wonder if changing deferred to YES when creating our windows make
> > any difference. 
> 
> I don't think so, since windows are shown before they are asked to become
> transparent, I believe. I'll give it a try though.

I did, and it worked! I could have sworn I tried that before. I will post a new patch shortly, I want to understand why that works.

Comment 90

[Colin Barrett [:cbarrett]]

So, the above change works because:

The defer flag avoids creating the actual window device (context the window draws to) until the window has been moved onscreen. It turns out that I was wrong about ordering. Windows are made transparent and then shown. I guess we triggered a bug somehow where after the window device was created, changes to its opaqueness weren't registered or something.

Anyway with that change popups on Tiger now work. Good idea, I don't know why I didn't try it before.

Comment 91

[Colin Barrett [:cbarrett]]

Comment on attachment 292850 [details] [diff] [review]
fix v2.1

Marking as obsolete, hwaara do you want to want to post a new patch with the defer:YES change?

Comment 92

[Håkan Waara]

Nice to hear that it worked! I had a hunch that the timing of initialization was relevant in this case.   Perhaps we should only use defer:YES for popups, just to be safe? Otherwise there might be a risk of regressions in other cases, or what do you think?

I'll post a new patch within the coming days.

Comment 93

[Colin Barrett [:cbarrett]]

(In reply to comment #92)
> Nice to hear that it worked! I had a hunch that the timing of initialization
> was relevant in this case.   Perhaps we should only use defer:YES for popups,
> just to be safe? Otherwise there might be a risk of regressions in other cases,
> or what do you think?

It should be OK -- I talked to vlad and stuart and they said that we really should not be doing drawing before a window is shown for the first time. From the documentation:

"Deferring the creation of the window improves launch time and minimizes the virtual memory load on the window server."

Sounds like it's at least worth trying for all windows if there's a performance gain. I checked the CVS blame for nsCococaWindow.mm, defer:NO wasn't changed in any specific checkin, it's just been living there since the initial rev in 2002.

Comment 94

[Håkan Waara]

Attachment: Patch v3

Here it is, in all its glory. 

Testing:

* A testbuild for the curious is available here: https://build.mozilla.org/tryserver-builds/2007-12-13_12:[e-mail]-bug_307204_test1/
* smorgan confirmed that the translucent popup testcase now works on Tiger with this build.

Latest changes: 

* Windows are now created when they are first shown. This might improve window launch time and maybe even app start time? We'll see.
* Removed an unused method and member on nsChildView; mAcceptFocusOnClick and AcceptFocusOnClick()

Comment 95

[Josh Aas]

Comment on attachment 293006 [details] [diff] [review]
Patch v3

+}
+
+// This is called by nsContainerFrame on the root widget for all window types

Two lines between methods in this file please. This happens in multiple places.

+    // XXXhakan: this should only be done on the screen showing the menubar

Please remove this comment and file a bug instead.

+    aTransparent = mIsTranslucent;

Rename mIsTranslucent to mIsTransparent.

Not done reviewing yet, more coming.

Comment 96

[Josh Aas]

Comment on attachment 293006 [details] [diff] [review]
Patch v3

+    GetHasTransparentBackground(translucent);

change variable name to transparent

Make those changes and it looks good to me. Thanks!

Comment 97

[Håkan Waara]

(In reply to comment #95)
> 
> +    aTransparent = mIsTranslucent;
> 
> Rename mIsTranslucent to mIsTransparent.

Should I do this even though mIsTranslucent is the Windows widget code?

Comment 98

[Josh Aas]

yes, it doesn't make any more sense there than it does in mac code

Comment 99

[Colin Barrett [:cbarrett]]

When I renamed SetWindowTranslucent to SetHasTransparentBackground, I probably should have changed those members.

There are a bunch more places where we use the word translucent still, I didn't change everywhere, just in a couple spots.

Comment 100

[Håkan Waara]

Attachment: Patch v4

Changes:

* Rename all variables and members to use "transparent" instead of "translucent"
* Filed bug 408476 for the positioning of new windows on multiple screens
* Fixed the other whitespace nits

Comment 101

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 293288 [details] [diff] [review]
Patch v4

You need to remove the declaration of mAcceptFocusOnClick (although I don't know why you're removing that stuff in this bug).

+  if (mIsTransparent != transparent) {
+    mIsTransparent = transparent;
+  }

Just do the assignment, this conditional test is pointless.

I think changing names from "translucent" to "transparent" was unnecessary, but I don't really care.

Comment 102

[Håkan Waara]

I'm ready to check in, but need driver's approval or blocking1.9+. Any volunteers?

Comment 103

[Håkan Waara]

That is; any volunteer drivers or module owners?

Comment 104

[Robert O'Callahan (:roc) (email my personal email if necessary)]

You have approval, we just need to land it. Do you have CVS access? If not, add the checkin-needed keyword.

Comment 105

[Håkan Waara]

Landed on trunk!

Comment 106

[Aronnax]

Hi,
i was asked for help to design the contextual dialog - for the new Mac default theme 

It is Bug 403158 - Style new bookmark contextual dialog on OS X 

To explored the possibilities have i tried some variations, but unfortunately have i not found what i need. 

Please take a look on: 
http://www.takebacktheweb.org/themes/stuff/contextual_dialog.png

1. is the intended design 
2. a try with this code in the userchrome.css 

#identity-popup-container  {
    background-color: #CDCDCD !important;
    -moz-border-radius-bottomleft: 8px !important;
    -moz-border-radius-bottomright: 8px !important;
    -moz-border-radius-topright: 8px !important;
     -moz-appearance: none !important;
}

3. a try with this code: 

#identity-popup {
  background-image: url("test.png") !important;
  -moz-border-radius-bottomleft: 8px !important;
  -moz-border-radius-bottomright: 8px !important;
  -moz-border-radius-topright: 8px !important;
  -moz-appearance: none !important;
}

#identity-popup-container  {
background: transparent !important;
}

------

With number 2. are the white borders a problem and with number 3. the missing shadow, but maybe has one of the experts more ideas?

(likely is it related with Bug 391984 – [10.5] Add roundness to context menus)

Cheers 

Comment 107

[Håkan Waara]

(In reply to comment #106)

#2 is not transparent because the container is not marked as transparent. This doesn't look like a bug.

When transparency is on, we turn shadows off explicitly. This is because the shadow for an animated transparent window (e.g. the moving octopus example PNG in this bug) would not redraw properly. This was discussed earlier in this bug.

If we want, we could add a new window chrome flag to turn shadows on, so users who really want it still can do it.

Comment 108

[Aronnax]

Thanks, 
it would be great to have another transparent window variation with shadows.
To be exact, a variation which makes #2 (only round corners) possible without the white background.

Only a container should have then the shadows and not the elements inside it. 
It could be used for many, many other theme elements in the Mac default  - in particular as well Bug 391984 - or in other words, to spend time on this would be for sure not wasted.

Cheers 

Comment 109

[Håkan Waara]

I filed bug 415443 where we can continue to discuss the shadows situation.

Comment 110

[Ovidiu Guse]

Hi,

I'm digging on the transparency issue for some time now without any luck. 
I'm opening a transparent window containing some textbox elements inside. 

The problem I have with it is that the textbox caret is not visible in this particular case. If I remove the transparency, then it will show correctly.

Can you please suggest a possible solution to this? If needed I can send a sample demo file outlining the problem.

Thank you,
Ovi.

Comment 111

[Håkan Waara]

(In reply to comment #110)

Send me the demo and I'll look at it. If you consider it a bug, please file a new bug for the issue and add it as blocking this.

It's better if new bugs are filed than added as comments here.

Comment 112

[Ovidiu Guse]

Hi,

Thank you for your quick reply. I don't know how to attach a file in here, it seems I don;t have any link for it. Thus, I will paste the XUL code inside this message.

To open the window I use this code:

window.openDialog("chrome://myextension/content/test-dlg.xul", "Test dlg", "centerscreen,titlebar=no,resizable=yes,modal=no,alwaysRaised=yes", params).focus();

It shows the window, as semimodal over the browser.
The XUL code for the test window is this (test-dlg.xul):

<?xml version="1.0"?>
<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>

<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" 
        id="groupyDlg" 
        hidechrome="true" 
        style="background-color: transparent; color:white; font-weight:bold;"
        onload="init();">
  <vbox flex="1">
    <vbox>
      <hbox>
        <titlebar style="background-color:black;">
          <spacer width="5"/>
          <label value="Sample Title"/>
          <spacer width="5"/>
        </titlebar>
      </hbox>
      <spacer height="3"/>
      <hbox flex="1">
        <spacer width="5"/>
        <label value="Name:" style="color:black; font-weight:normal;"/>
        <spacer width="3"/>
        <textbox width="150"/>
        <spacer width="5"/>
      </hbox>
      <spacer height="3"/>
    </vbox>
  </vbox>
</window>


Thank you very much for your support.
Ovi

Comment 113

[Ovidiu Guse]

I just noticed, if I'm using the standard window.open(...) instead of the window.openDialog(...), it will show just a bordered non-transparent, white background window, without the default title bar, with the caret working. 
I'm not sure in this case how to made it transparent and borderless.

Thank you again for your assistance.

Comment 114

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

Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9) Gecko/2008061004 Firefox/3.0 ID:2008061004