Closed Bug 307204 Opened 19 years ago Closed 17 years ago

Support for transparent windows on Mac OS X

Categories

(Core :: Widget: Cocoa, defect)

Product:
Core
Component:
Widget: Cocoa
Platform:
All
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.9beta3

People

(Reporter: aaiyer, Assigned: hwaara)

References

Details

(Keywords: platform-parity)

Attachments

(5 files, 9 obsolete files)

I have a XUL octopus living on my desktop!
56.62 KB, image/png
Details
Happy transparent and animated octopuses living side by side
150.55 KB, image/png
Details
Screenshot of titlebarless transparent window
37.90 KB, image/png
Details
test for transparent popups
656 bytes, application/vnd.mozilla.xul+xml
Details
Patch v4
46.37 KB, patch
jaas
: review+
roc
: superreview+
damons
: approval1.9+
Details | Diff | Splinter Review
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:
Assignee: nobody → joshmoz
Component: XP Toolkit/Widgets: XUL → Widget: Mac
QA Contact: xptoolkit.xul → mac
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 ,
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.
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.
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.
confirming Mac needs this fix
Status: UNCONFIRMED → NEW
Ever confirmed: true
Component: Widget: Mac → Widget: Cocoa
QA Contact: mac → cocoa
we probably want this for platform parity
Flags: blocking1.9?
Flags: blocking1.9? → blocking1.9-
Whiteboard: [good first bug] → [good first bug][wanted-1.9]
It's a single method call to set the opacity of a window in Cocoa.
(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.
(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)
You (we?) really want this now that the better autoscroll implementation from SeaMonkey was backported to toolkit.
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-.
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
Not sure 1.9.1 scope has been fully decided, but IMO this would definitely be in scope for 1.9.1
Keywords: pp
We could take this for 1.9 if someone can find a contributor to implement it. It's not hard.
This is also required by bug 303110, which is a really-really-want for Firefox 3, and potentially a blocker.
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
Assignee: joshmoz → hwaara
Status: NEW → ASSIGNED
Attached patch Patch v0.7 (proof of concept) (obsolete) — — Splinter Review
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?
> 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...
(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.
(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"?
(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.
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. :-/
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?
Attached patch Patch v1.0 (obsolete) — — Splinter Review
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?
Attachment #277042 - Attachment is obsolete: true
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 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.
Attachment #277094 - Attachment is obsolete: true
Attached patch Patch v1.1 (obsolete) — — Splinter Review
Changes: * Always mark our ChildViews as opaque unless our window is explicitly told to be translucent, for performance.
Attachment #277122 - Flags: review?(joshmoz)
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!!!
So, I was kinda hoping this would replace the popups-specific hack with a one-liner style rule in Pinstripe ;)
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] ?
Since you're touching nsIWidget.h, maybe use this opportunity to rename GetWindowTranslucency?
(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.
(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?
> 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.
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.
Blocks: 391984
No longer blocks: 391984
> > 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.
This patch doesn't seem to work for transparent popups though, only toplevel windows. Transparent popups are desired for a firefox UI notification feature.
(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.
(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.
(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...
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
(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...
>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.
OK, I will try to find some time for this this week.
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?
General comment: are there security concerns with allowing windows with an alpha value of 0?
Is the functionality limited to chrome popups on other platforms?
Only chrome windows can have a transparent background. This is enforced by gecko.
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...
I mean a Gecko view, i.e., non-null GetClientData.
Attached patch WIP patch (obsolete) — — Splinter Review
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.
Attachment #277122 - Attachment is obsolete: true
Attachment #277122 - Flags: review?(joshmoz)
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?
(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.
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.
So I applied the WIP patch and nothing happened. The testcase Nail attached looked the same. Post an updated/complete patch?
(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...
According to your earlier comments, translucent popups were all black? Perhaps I'm misunderstanding them.
Hi Håkan, Your last patch seems to work just fine. We're going to use it as-is for Songbird 0.3. Thanks!
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.
Flags: blocking1.9- → blocking1.9?
Once again UI people are requesting major platform features very late in the game. This poor planning can't continue.
Håkan - can you update your patch so that it applies cleanly?
New patch coming up today or tomorrow, thank you for the patience
Attached patch Latest patch (-up8) (obsolete) — — Splinter Review
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
Attachment #282093 - Attachment is obsolete: true
So, to sum up: Please apply and test the patch, and we might get this in soonish.
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.
(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?
So I had a friend test this on Tiger: where translucent popups won't work. On Leopard it's working perfectly.
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.
Flags: blocking1.9? → blocking1.9+
Priority: -- → P3
Target Milestone: --- → mozilla1.9 M10
Flags: blocking1.9+ → blocking1.9-
Are we still depending on this for firefox 3 front end features?
(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.
(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.
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.
That is what I did - I had talked to mconnor he told me to wait for the decision before marking blocking or not.
OK, I'll push on mconnor :)
This doesn't apply cleanly.
Attached patch Update patch (-up8) 2007-12-04 (obsolete) — — Splinter Review
Here is an updated (untested) patch.
Attached patch fix v2.0 (obsolete) — — Splinter Review
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.
Attachment #286713 - Attachment is obsolete: true
Attachment #291432 - Attachment is obsolete: true
Attachment #291958 - Flags: review?(joshmoz)
Attachment #291958 - Flags: review?(joshmoz)
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?
Attached patch fix v2.1 (obsolete) — — Splinter Review
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.
Assignee: hwaara → cbarrett
Attachment #291958 - Attachment is obsolete: true
Attachment #292850 - Flags: review?(joshmoz)
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.
(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 :(
(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.
Assignee: cbarrett → hwaara
Status: ASSIGNED → NEW
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 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?
Attachment #292850 - Attachment is obsolete: true
Attachment #292850 - Flags: review?(joshmoz)
Priority: P3 → --
Target Milestone: mozilla1.9 M10 → mozilla1.9 M11
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.
(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.
Attached patch Patch v3 (obsolete) — — Splinter Review
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:37-hwaara@gmail.com-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()
Attachment #293006 - Flags: review?(joshmoz)
Summary: Support for translucent windows in Mac → Support for transparent windows on Mac OS X
Whiteboard: [good first bug][wanted-1.9] → [wanted-1.9]
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 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!
Attachment #293006 - Flags: review?(joshmoz) → review-
(In reply to comment #95) > > + aTransparent = mIsTranslucent; > > Rename mIsTranslucent to mIsTransparent. Should I do this even though mIsTranslucent is the Windows widget code?
yes, it doesn't make any more sense there than it does in mac code
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.
Attached patch Patch v4 — — Splinter Review
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
Attachment #293006 - Attachment is obsolete: true
Attachment #293288 - Flags: review?(joshmoz)
Attachment #293288 - Flags: superreview?(roc)
Attachment #293288 - Flags: review?(joshmoz)
Attachment #293288 - Flags: review+
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.
Attachment #293288 - Flags: superreview?(roc) → superreview+
I'm ready to check in, but need driver's approval or blocking1.9+. Any volunteers?
That is; any volunteer drivers or module owners?
Attachment #293288 - Flags: approval1.9+
You have approval, we just need to land it. Do you have CVS access? If not, add the checkin-needed keyword.
Landed on trunk!
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Blocks: 409478
Blocks: 395980
Flags: wanted1.9+
Whiteboard: [wanted-1.9]
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
(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.
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
I filed bug 415443 where we can continue to discuss the shadows situation.
Depends on: 415934
No longer depends on: 415934
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.
(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.
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
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.
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
Status: RESOLVED → VERIFIED
Hardware: Macintosh → All
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: