Original titlebar work was done in bug 575870. Fixing this involves removing the rest of the window chrome rendered by windows, and replacing it with xul styled appropriately using the new moz-appearance styles implemented in bug 574454.

What's the size and scope of this work? It's marked as BetaN+ but hasn't been touched in a month, so that scares me a little bit here. Somehow I doubt that this is all gonna be Jim, too. Decisions to be made: - do we need to consider the case where the menuBar is shown instead of the Firefox Button - who's gonna work on this Jim: can you type a few notes about what needs to be done here?

- building the window frame in browser.xul using xul markup. - applying the appropriate styling so the frame is painted correctly. - support for mouse window resizing. (the frame has to work just like a normal window frame.) - fixing any bugs related to glass clipping (similar to the left/right corner problems we had with the titlebar and personas)

Why is this blocking at all?

(In reply to comment #4) > Why is this blocking at all? We need this to make XP look nice and it is also required for making personas fill the entire window if I understand things correctly.

(In reply to comment #6) > (In reply to comment #4) > > Why is this blocking at all? > > ... and it is also required for making personas > fill the entire window if I understand things correctly. Yep. Which might look interesting with Panorama: http://www.stephenhorlander.com/images/blog-posts/incontent-ui/win7-tabcandy-glass.jpg

> Yep. Which might look interesting with Panorama: > http://www.stephenhorlander.com/images/blog-posts/incontent-ui/win7-tabcandy-glass.jpg That's really not relevant. Please stop commenting on bugs unless you have valuable information to add.

(In reply to comment #5) > Created attachment 484700 [details] > Windows XP Frame This is with the menu bar hidden, which isn't the default setup. I remember having seen similar mockups with the menu bar shown, but those looked unpleasantly non-native to me... (Yes, I still use XP most of the time.)

You mean this one? http://www.mozilla.sk/wp-content/uploads/2010/05/Firefox-4-Mockup-i06-XP-LunaBlue-TabsTop-MenuBar.png

Yes.

(In reply to comment #10) > You mean this one? > http://www.mozilla.sk/wp-content/uploads/2010/05/Firefox-4-Mockup-i06-XP-LunaBlue-TabsTop-MenuBar.png I actually think that looks better than the standard XP approach, and fits better with the rest of our needs for being able to draw into the titlebar. We should continue forward with this work. The question of whether or not it blocks, I think, depends largely on how it looks now. Anyone want to furnish me with a screenshot (both Luna and Classic?)

Attachment 484700 [details] is how it looks now in Luna. I will attach a classic SS.

(In reply to comment #13) > Attachment 484700 [details] is how it looks now in Luna. I will attach a classic SS. I meant to say "the top image" :)

Does XP theming really need this? My understanding is that it was only waiting on bug 543910

Thanks, Stephen. My feeling is that fixing this bug would make us look much more polished and modern (though, as Dao points out, less XP native) on XP systems. I'm not sure I'd put it at the level of a blocker, though, since we're not break-my-eyes ugly on Luna. I think we'd probably want to only do it for Luna, though, and not for Classic, even at the cost of Personas not working properly in Classic.

(In reply to comment #16) > Created attachment 484791 [details] > Windows XP Luna and Classic Screenshot Hmm, when did you snap this Stephen? The button sizes regressed on classic. I wonder if that's fallout from bug 601603 that landed yesterday.

(In reply to comment #18) > (In reply to comment #16) > > Created attachment 484791 [details] [details] > > Windows XP Luna and Classic Screenshot > > Hmm, when did you snap this Stephen? The button sizes regressed on classic. I > wonder if that's fallout from bug 601603 that landed yesterday. I just took the screenshot from 4.0b8pre but my build was two days out-of-date.

(In reply to comment #19) > (In reply to comment #18) > > (In reply to comment #16) > > > Created attachment 484791 [details] [details] [details] > > > Windows XP Luna and Classic Screenshot > > > > Hmm, when did you snap this Stephen? The button sizes regressed on classic. I > > wonder if that's fallout from bug 601603 that landed yesterday. > > I just took the screenshot from 4.0b8pre but my build was two days out-of-date. Hmm, looks ok on win7. Did you switch themes with the window open right before taking that?

(In reply to comment #20) > Did you switch themes with the window open right before > taking that? Yes. I was using Luna, then switched to Classic.

Ok thanks, that's filed.

(In reply to comment #13) > Attachment 484700 [details] is how it looks now in Luna. I will attach a classic SS. Back to the subject at hand.. looking at this screen shot, the left and right frames look identical to the default xp frames. Couldn't we render the upper area in the titlebar and simply blend that into the default frame without having to implement this bug?

(In reply to comment #23) > (In reply to comment #13) > > Attachment 484700 [details] [details] is how it looks now in Luna. I will attach a classic SS. > > Back to the subject at hand.. looking at this screen shot, the left and right > frames look identical to the default xp frames. Couldn't we render the upper > area in the titlebar and simply blend that into the default frame without > having to implement this bug? Similarly with aero basic - https://wiki.mozilla.org/images/a/a7/Firefox-4-Mockup-i06-%28Win7%29-%28NoAero%29-%28TabsTop%29.png I don't think there's anything here holding up this work, we just need to match the left and right sides with the default frame. (For an idea of how this would look, take a look at the IE9 beta.) This bug is specific to getting full skinning of the window frame with personas, but shouldn't hold up our titlebar / tabbar theme work.

(and probably shouldn't block a beta unless we feel personas skinning of the left, right, and bottom frame is critical.)

(In reply to comment #25) > (and probably shouldn't block a beta unless we feel personas skinning of the > left, right, and bottom frame is critical.) I would argue that it is, Personas just look buggy and unfinished without it. We certainly see a fair amount of feedback to that effect. (and yes, I know that we shipped it even worse in 3.6, but that shouldn't be the benchmark ;)

This is a pretty big change to be taking post feature-freeze. It changes the XUL structure in ways that could affect extensions with overlays and will definitely affect extensions that modify window appearance using CSS. I really think it's just too late.

Keep in mind that the common persona header image is only so many pixels high. We'd have to use the solid "accent color" for the frame, which is even worse than it sounds, because authors often specify arbitrary accent colors.

(In reply to comment #28) > Keep in mind that the common persona header image is only so many pixels high. > We'd have to use the solid "accent color" for the frame, which is even worse > than it sounds, because authors often specify arbitrary accent colors. Is this really what we want in the end? The accent color sounds like temp fix for older personas. I'd like to see graphics covering the entire frame. I;d suggest we support full graphical skinning on m-c after we branch 4.0. Then work on changes to the personas site and evangelizing to get people to update their existing personas to support the new frame graphics.

comment #29 >Is this really what we want in the end? The accent color sounds like temp fix >for older personas. I'd like to see graphics covering the entire frame. Yeah, graphics should ideally cover the entire frame. We'll need to continue to work with the personas team to transition personas to the new size. (I wonder to what extent we used the current size because OS X doesn't happen to have a window frame). comment #27 >This is a pretty big change to be taking post feature-freeze. It changes the >XUL structure in ways that could affect extensions with overlays and will >definitely affect extensions that modify window appearance using CSS. I really >think it's just too late. Had those concerns been brought up earlier we likely would have tried to get this blocking an earlier beta, but in terms of the changes need for our XP theme, I believe this still needs to block betaN. If personas were the only concern, we could just push all of this work back and deal with it later, but about half of our market share is still using XP and this bug plays a major role in how we craft the main window to appear more modern.

(In reply to comment #30) > comment #29 > Had those concerns been brought up earlier we likely would have tried to get > this blocking an earlier beta, but in terms of the changes need for our XP > theme, I believe this still needs to block betaN. Why can't we blend our titlebar in with the existing window border? Looks like it would be pretty easy to do on themes like Luna and Aero Basic.

Notes from the Grand Retriage: too late, we'll design around it.

Note that bug 625307 covers implementing the visual appearance we want, with just modifying the title bar. However, when we can draw the entire window frame we might later make some tweaks to the window border. Also either way this bug is still very much wanted for personas.

(In reply to comment #33) > Note that bug 625307 covers implementing the visual appearance we want, with > just modifying the title bar. However, when we can draw the entire window > frame we might later make some tweaks to the window border. Also either way > this bug is still very much wanted for personas. This is the first thing I want to do once we get 4.0 blockers cleared.

I have just posted in the themes forum about my wish to implement this and got directed here. Just wanted to point out that Google Chrome does it and looks great! It allows designing a more modern frames that could blend if we wish to, with the OS colors. For example, to make the (I think we can agree - ugly) Luna look better, we could design a slim frame that keeps the blue color but creates a more modern look.

Bug 77790 is probably somewhat related.

This is how things are looking at this point. I went with repeat-y on the texture which actually works out quite well for the entire frame. I'd say about 80% of the personas I've tried look excellent or acceptable with the texture repeated down the side of the border. The only time you notice it is when the seam lands in the addon bar, but I think that's a worth while trade off. Designers can improve the way the texture stack over time. On aero basic, xp, and classic everything remains the same as it is now. The normal windows chrome paints over these areas so there's no need to texture them. We can spin off bugs on implementing the xp and aero seemless look and feel after I get this work completed. It looks like I may be able to land all of this work on the e10s branch so we'll have test coverage and nightly builds. I should have patches posted in a day or so.

Comment on attachment 518194 [details] screen shot Alex, requesting ui-review for a couple things - I've added slight opacity to the fx button when it's not hovered or active. Just thought it looked pretty cool. Curious what you think. Also, prelim ui-review on the way the textures are being repeated. I'll get a try build going as well for testing purposes.

I'm not an expert on this, but I was wondering... +#main-window:not([chromemargin]) #window-frame-left-top, +#main-window:not([chromemargin]) #window-frame-left, +#main-window:not([chromemargin]) #window-frame-right-top, +#main-window:not([chromemargin]) #window-frame-right, +#main-window:not([chromemargin]) #window-frame-bottom, +#main-window[sizemode="maximized"] #window-frame-left-top, +#main-window[sizemode="maximized"] #window-frame-left, +#main-window[sizemode="maximized"] #window-frame-right-top, +#main-window[sizemode="maximized"] #window-frame-right, +#main-window[sizemode="maximized"] #window-frame-bottom, +#main-window[inFullscreen] #window-frame-left-top, +#main-window[inFullscreen] #window-frame-left, +#main-window[inFullscreen] #window-frame-right-top, +#main-window[inFullscreen] #window-frame-right, +#main-window[inFullscreen] #window-frame-bottom { + display: none; +} It might be better to use the -moz-any() selector here. Correct me if I'm wrong.

Comment on attachment 518185 [details] [diff] [review] corner styles v.1 David, this adds two moz-appearance styles that represent the upper left and upper right corners of the titlebar. (bug 612026)

Comment on attachment 518194 [details] screen shot Moving review over to Stephen since this is entirely visual and he's given this more thought. For the Firefox button, I think we should consider falling back to transparent instead of orange, since we really have no idea what background the orange is going to appear on.

(In reply to comment #46) > Comment on attachment 518194 [details] > screen shot > > Moving review over to Stephen since this is entirely visual and he's given this > more thought. For the Firefox button, I think we should consider falling back > to transparent instead of orange, since we really have no idea what background > the orange is going to appear on. I'd suggest taking a try build for a spin before deciding. With .9 opacity the texture adds the persona's hue to the orange of the button, but doesn't corrupt it. IMHO it looks pretty good. I'll post a link to try builds once they finish.

A try build should be available here in about six hours or so: http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/jmathies@mozilla.com-f30a58beabea

(In reply to comment #48) > A try build should be available here in about six hours or so: > > http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/jmathies@mozilla.com-f30a58beabea I tried this under Win7. With Aero and Classic the Personas are applied to the border, but under Basic not.

(In reply to comment #49) > (In reply to comment #48) > > A try build should be available here in about six hours or so: > > > > http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/jmathies@mozilla.com-f30a58beabea > > I tried this under Win7. With Aero and Classic the Personas are applied to the > border, but under Basic not. I need to fix the border on classic, it's not supposed to show up there or on aero basic. Basically, classic, aero basic, and xp these should all look normal. (Even though we are now painting the entire frame and titlebar areas ourselves.) Aero glass desktops should have full personas skinning of all glass areas, including the window frame.

Using the Aero Peek feature by hovering over the rectangular button on the right of the task bar in Windows 7 the window of Fx is disappearing completely instead of showing the outline.

(In reply to comment #51) > Using the Aero Peek feature by hovering over the rectangular button on the > right of the task bar in Windows 7 the window of Fx is disappearing completely > instead of showing the outline. I noticed this as well.

Comment on attachment 518194 [details] screen shot This looks great! The translucency on the Firefox button is a nice touch.

(In reply to comment #51) > Using the Aero Peek feature by hovering over the rectangular button on the > right of the task bar in Windows 7 the window of Fx is disappearing completely > instead of showing the outline. Interesting. Chrome has the same problem. I don't think it should hold up the initial patch landings here. I'll file a follow up if I can't find a simple fix.

Comment on attachment 518621 [details] [diff] [review] theme cleanup v.2 Simple theme code cleanup - swapping if else with switch.

Comment on attachment 518623 [details] [diff] [review] theme work v.2 Theme code changes. There's two main chunks in here, the first uses the gfxContext to clear the four corners of the window so that they don't end up square. The second chunk is the classic rendering for the left, right, and bottom window frames plus misc. additions to the various nsITheme calls to make everything work right.

One feature I had hoped to add was a thinning of the glass chrome when the browser is in "normal" window mode (menu bar enabled). Unfortunately the Windows dwm doesn't seem to like this. I can remove the chrome completely, or expand it in, but shaving a couple pixels off the inside edge fails. In order to get this to work we'll have to remove the default glass chrome completely in this mode as well. This presents some problems with personas and the default titlebar and it's contents. I'm going to avoid the complexity of adding this here and file a follow up.

maybe with the right stylistic additions, but stand alone this does not work.

(up to date patch and switching reviewers as Vlad is not with us anymore.) > Theme code changes. There's two main chunks in here, the first uses the > gfxContext to clear the four corners of the window so that they don't end up > square. > > The second chunk is the classic rendering for the left, right, and bottom > window frames plus misc. additions to the various nsITheme calls to make > everything work right.

Comment on attachment 519169 [details] [diff] [review] theme work v.2 Since I don't have Aero Basic, I'm not sure what the issue is with the top corners. Can you not draw the corners as part of the left and right sides, hopefully this will completely avoid the problem?

(In reply to comment #64) > Comment on attachment 519169 [details] [diff] [review] > theme work v.2 > > Since I don't have Aero Basic, I'm not sure what the issue is with the top > corners. Can you not draw the corners as part of the left and right sides, > hopefully this will completely avoid the problem? Aero basic isn't the issue, it's glass. Since we don't render the main window background here, what we do is clear the contents when we have the chance. We could clip the painting of the main window in the OnPaint handler in nsWindow, or we can handle it here. (We are already handling the command buttons this way just above this code.)

+ case NS_THEME_WINDOW_FRAME_LEFT_TOP: + // Push the center area and the opposite edge off the surface we'll render + // to. This must be a large number to prevent window edge drawing anomalies + // on aero basic. + widgetRect.right += 100; + break; + case NS_THEME_WINDOW_FRAME_RIGHT_TOP: + widgetRect.left -= 100; + break; Or were you asking about this part of the patch? Since we can only ask Windows to render the titlebar in one piece (left corner plus mid section plus right corner), I tell Windows to render it in an elongated section where most of the surface is off the hdc. If I ask Window to render just the corner, it'll try to squish the entire titlebar into that little part.

(In reply to comment #66) > Or were you asking about this part of the patch? Yes, I was. > Since we can only ask Windows to render the titlebar in one piece (left corner > plus mid section plus right corner), I tell Windows to render it in an > elongated section where most of the surface is off the hdc. If I ask Window to > render just the corner, it'll try to squish the entire titlebar into that > little part. Sorry, I'm not seeing where the call to render the titlebar is in the patch.

(In reply to comment #67) > Sorry, I'm not seeing where the call to render the titlebar is in the patch. The code to render the sections is already in place. This all falls through to nsUXThemeData::drawThemeBG. For the mid section, we set things up in similar fashion: if (aWidgetType == NS_THEME_WINDOW_TITLEBAR) { // Clip out the left and right corners of the frame, all we want in // is the middle section. widgetRect.left -= GetSystemMetrics(SM_CXFRAME); widgetRect.right += GetSystemMetrics(SM_CXFRAME); } This additional code: + case NS_THEME_WINDOW_FRAME_LEFT_TOP: + // Push the center area and the opposite edge off the surface we'll render + // to. This must be a large number to prevent window edge drawing anomalies + // on aero basic. + widgetRect.right += 100; + break; + case NS_THEME_WINDOW_FRAME_RIGHT_TOP: + widgetRect.left -= 100; + break; sets up the surface we render to using nsUXThemeData::drawThemeBG for the corners. Between these three theme types we can render a complete titlebar.

Ah, so WP_CAPTION actually draws the corners too?

(In reply to comment #69) > Ah, so WP_CAPTION actually draws the corners too? Yes, windows considers it one piece. We needed it broken down to do the titlebar work in 4.0. We could overhaul all that and combine the edges with the existing titlebar style, but that would require a lot of code/css/xul rework, and we'd loose the "drawintitlebar" functionality we currently have.

Note I did not break out the lower left and right corners as individual styles. The base is just one piece.

Comment on attachment 522470 [details] [diff] [review] add clear to nsmargin v.1 simple patch that adds a utility method for clearing an nsMargin to zero.

UpdateNonClientMargins changes so that we can have these custom margins working all the time, which gives us the ability to compensate for our borderless glass adjustment.

glass calculation changes: 1) replaces MAX_HORIZONTAL_GLASS_MARGIN with GetSystemMetrics(SM_CXFRAME). Insures we fall into the margin calculations when we have an nc client area we render ourselves. 2) Set margins to the max of either the calculated opaque region or the nc client area margin, which insures the rounded corners of the window are covered by glass. Without this you get squared off corners. 3) only adjust the margin values by kGlassMarginAdjustment if we are letting windows handle the nc client area. If we aren't there no need to compensate for the border since windows doesn't draw it.

Update the hit test code to make sure we respect the width and height of the window border.

easy peasy hit test debug feature. This is invaluable in finding mathematical problems with our mouse hit test code. There's a bug filed on converting all this printf output to nspr logging, but I've never gotten around to it. So I stuck with printf on this.

Browser css changes - 1) due to XUL structure changes I replaced some child selectors with id selectors. 2) added border styling on all sides of the window frame. 3) added border radius settings for the round corners. Note radius of these is based on the margin width/height in system settings. The function updateFrameMarginSettings in browser.js sets these values dynamically based on the boxObject of the frame. updateFrameMarginSettings also sets the proper offset on the status panel, which has absolute positioning. 4) set the opacity on the fx button per the screen shots I posted. 5) handles hiding the frame when it's not needed / enabled.

Comment on attachment 522482 [details] [diff] [review] browser css v.1 (see comments 78)

Comment on attachment 522478 [details] [diff] [review] hit test debug feature DebugHitTest would probably look cleaner using a switch statement.

Comment on attachment 522482 [details] [diff] [review] browser css v.1 >+ #browser-bottombox[lwthemefooter="true"] { >+ background-image: none !important; >+ background: none !important; > } This doesn't seem right, the footer image should still be used.

Not using the footer image is a significant change and something designers would not be thrilled about. Many are already a little annoyed (or more) that the Firefox 4 UI shows so much less of the Persona by default :) Would it be possible to continue using the footer image? Having this be inconsistent across OSs seems like a flaw to me.

Attaching one example. Aesthetically it's a pretty big trade off. If we could give the footer a z level higher than the header it would meet in the frame and probably wouldn't look too bad. (Dao, any ideas?) If that's not possible I'd say just ditch the footer.

I agree with Jim. I don't think designers would be entirely upset by this change, because they will actually have more room for their designs compared to 4.0. If we must include the footer image with the add-ons bar, we could include a border around it to separate the two images and to match the width of the border around the content area, although that makes the add-ons bar somewhat spoof-able. I'd go with removing the footer image.

Looking through getpersonas, the footers tend to be unique textures, so they generally won't match well with headers even if we could get the overlapping to work. Not sure what to do here. I can build up a couple of try builds if folks want to take each design for a spin.

(In reply to comment #82) > Many are already a little annoyed (or more) that > the Firefox 4 UI shows so much less of the Persona by default :) That's likely bug 622366, and we would like to get that fixed.

(In reply to comment #83) > If we could > give the footer a z level higher than the header it would meet in the frame and > probably wouldn't look too bad. (Dao, any ideas?) I don't understand what kind of difference giving it "a z level higher than the header" would make visually, nor what exactly it means really...

Comment on attachment 522482 [details] [diff] [review] browser css v.1 This already makes it hard enough to create a decent appearance for aero glass when you're not using aero glass (i.e. if you're on XP, Win7 with aero glass unavailable, Linux or Mac). Hiding the footer image for aero glass crosses the line of cross platform compatibility.

(In reply to comment #88) > Comment on attachment 522482 [details] [diff] [review] > browser css v.1 > > This already makes it hard enough to create a decent appearance for aero glass > when you're not using aero glass (i.e. if you're on XP, Win7 with aero glass > unavailable, Linux or Mac). Hiding the footer image for aero glass crosses the > line of cross platform compatibility. Could you clarify why this was r-'d? Are you saying you want the footer visible as in the screen shot on the left here: https://bug590945.bugzilla.mozilla.org/attachment.cgi?id=522550 or did you minus the patch for some other reason? I don't understand what you mean by "create a decent appearance for aero glass when you're not using aero glass".

> I don't understand what you > mean by "create a decent appearance for aero glass when you're not using aero > glass". People on said platforms won't see anything close to attachment 522550 [details], which makes it more complicated for them to create images that work fine across the board. Hiding the footer image for aero glass makes this situation even worse (e.g. for designers who do use aero glass), and that's what my r- is for.

(In reply to comment #90) > > I don't understand what you > > mean by "create a decent appearance for aero glass when you're not using aero > > glass". > > People on said platforms won't see anything close to attachment 522550 [details], which > makes it more complicated for them to create images that work fine across the > board. > > Hiding the footer image for aero glass makes this situation even worse (e.g. > for designers who do use aero glass), and that's what my r- is for. Ok, not a big deal since it's not that visible anyway. Updated patch with the footer re-enabled coming up.

updated.

Comment on attachment 522672 [details] [diff] [review] browser css w/footer v.1 >+function updateFrameMarginSettings(chromemargin) { >+ // setup the correct border radii on the corners of the window >+ // based on the width/height of the frame. These values are >+ // dynamically set down in widget theming code. >+ let element = document.getElementById("window-frame-left-top-inner"); >+ let leftwidth = element.boxObject.width; >+ element.style.borderTopLeftRadius = leftwidth + "px"; >+ element = document.getElementById("window-frame-right-top-inner"); >+ let rightwidth = element.boxObject.width; >+ element.style.borderTopRightRadius = rightwidth + "px"; >+ element = document.getElementById("window-frame-bottom-inner"); >+ element.style.borderBottomLeftRadius = element.boxObject.height + "px"; >+ element.style.borderBottomRightRadius = element.boxObject.height + "px"; What happens when you set these radii to some large value regardless of the element dimensions? >+ // set the padding on the status panel >+ let e = document.getElementById("statusbar-display"); >+ if (!e.hasAttribute("mirror")) { >+ e.style.left = (chromemargin ? leftwidth : 0) + "px"; >+ e.style.right = "auto"; >+ } else { >+ e.style.left = "auto"; >+ e.style.right = (chromemargin ? rightwidth : 0) + "px"; >+ } I haven't tested this, but this looks wrong. The mirror attribute isn't static, it comes and goes every now and then. Also, most of the above code doesn't look rtl-ready. (=> r-) >+@media all and (-moz-windows-compositor) { >+ #appmenu-button:-moz-lwtheme { >+ opacity: .9; >+ } >+} If you add :not(:hover):not([open]) here, you can remove this: >+#appmenu-button:hover, >+#appmenu-button[open] { >+ opacity: 1; >+} >+#main-window:not([chromemargin]) #window-frame-left-top, >+#main-window:not([chromemargin]) #window-frame-left, >+#main-window:not([chromemargin]) #window-frame-right-top, >+#main-window:not([chromemargin]) #window-frame-right, >+#main-window:not([chromemargin]) #window-frame-bottom, >+#main-window[sizemode="maximized"] #window-frame-left-top, >+#main-window[sizemode="maximized"] #window-frame-left, >+#main-window[sizemode="maximized"] #window-frame-right-top, >+#main-window[sizemode="maximized"] #window-frame-right, >+#main-window[sizemode="maximized"] #window-frame-bottom, >+#main-window[inFullscreen] #window-frame-left-top, >+#main-window[inFullscreen] #window-frame-left, >+#main-window[inFullscreen] #window-frame-right-top, >+#main-window[inFullscreen] #window-frame-right, >+#main-window[inFullscreen] #window-frame-bottom { >+ display: none; >+} This should probably be in browser/base/content/browser.css. In fact, maybe the rules setting the -moz-appearance for these nodes should be in content too, to make theme authors' lives easier.

Comment on attachment 519169 [details] [diff] [review] theme work v.2 Sorry for taking so long to get back to this. >+ ctx->Arc(gfxPoint(offset,offset), offset, 180 * (M_PI / 180.0f), >+ 270 * (M_PI / 180.0f)); >+ ctx->LineTo(gfxPoint(0,0)); >+ } else { >+ ctx->MoveTo(gfxPoint(0,0)); >+ ctx->Arc(gfxPoint(0,offset), offset, 270 * (M_PI / 180.0f), 2 * M_PI); As well as 2 * M_PI, why not also 0.5 * M_PI, M_PI, 1.5 * M_PI as appropriate? Or if you prefer, 360 * (M_IP / 180.0f) instead of 2 * M_PI.

(In reply to comment #93) > Comment on attachment 522672 [details] [diff] [review] > browser css w/footer v.1 > > >+ element = document.getElementById("window-frame-bottom-inner"); > >+ element.style.borderBottomLeftRadius = element.boxObject.height + "px"; > >+ element.style.borderBottomRightRadius = element.boxObject.height + "px"; > > What happens when you set these radii to some large value regardless of the > element dimensions? They should always be based on the size of the margin to a point. I did some testing with custom border padding and noticed Windows clamps this so that the radius is always <= eight. I need to add that. > > >+ // set the padding on the status panel > >+ let e = document.getElementById("statusbar-display"); > >+ if (!e.hasAttribute("mirror")) { > >+ e.style.left = (chromemargin ? leftwidth : 0) + "px"; > >+ e.style.right = "auto"; > >+ } else { > >+ e.style.left = "auto"; > >+ e.style.right = (chromemargin ? rightwidth : 0) + "px"; > >+ } > > I haven't tested this, but this looks wrong. The mirror attribute isn't static, > it comes and goes every now and then. > > Also, most of the above code doesn't look rtl-ready. (=> r-) I'll check things with ehsan's rtl extension and test a bit more. The statusbar-display was a real pita since it's absolutely positioned. Without the proper left/right margin settings (which vary depending on Windows system settings) the panel ends up overlaping the glass window frame. > > > >+@media all and (-moz-windows-compositor) { > >+ #appmenu-button:-moz-lwtheme { > >+ opacity: .9; > >+ } > >+} > > If you add :not(:hover):not([open]) here, you can remove this: > > >+#appmenu-button:hover, > >+#appmenu-button[open] { > >+ opacity: 1; > >+} > > > >+#main-window:not([chromemargin]) #window-frame-left-top, > >+#main-window:not([chromemargin]) #window-frame-left, > >+#main-window:not([chromemargin]) #window-frame-right-top, > >+#main-window:not([chromemargin]) #window-frame-right, > >+#main-window:not([chromemargin]) #window-frame-bottom, > >+#main-window[sizemode="maximized"] #window-frame-left-top, > >+#main-window[sizemode="maximized"] #window-frame-left, > >+#main-window[sizemode="maximized"] #window-frame-right-top, > >+#main-window[sizemode="maximized"] #window-frame-right, > >+#main-window[sizemode="maximized"] #window-frame-bottom, > >+#main-window[inFullscreen] #window-frame-left-top, > >+#main-window[inFullscreen] #window-frame-left, > >+#main-window[inFullscreen] #window-frame-right-top, > >+#main-window[inFullscreen] #window-frame-right, > >+#main-window[inFullscreen] #window-frame-bottom { > >+ display: none; > >+} > > This should probably be in browser/base/content/browser.css. In fact, maybe the > rules setting the -moz-appearance for these nodes should be in content too, to > make theme authors' lives easier. will update.

Comment on attachment 518185 [details] [diff] [review] corner styles v.1 If these are corners, I'd much prefer top-left and top-right, which is much more consistent with the rest of CSS (and I think with general English usage, though not Chinese). Or are these something other than corners? r=dbaron.

(In reply to comment #96) > Comment on attachment 518185 [details] [diff] [review] > corner styles v.1 > > If these are corners, I'd much prefer top-left and top-right, which is much > more consistent with the rest of CSS (and I think with general English usage, > though not Chinese). > > Or are these something other than corners? > > r=dbaron. They are corners. The reason I went with left-top/right-top is that they are associated with the left and right sides. We could flip the side/top.. or maybe associate them with the titlebar via something like 'moz-window-titlebar-left'/'moz-window-titlebar-right'. (That seems a bit confusing to me since we also have 'moz-window-titlebar-maximized' which doesn't need corners!) So I ended up with left-top, right-top. I'm not partial to any of these so whatever you think fits best I'm fine with.

This fixes a bug that's been around since the original titlebar work landed. When a user changes certain advanced ui settings, instead of a theme changed event we get a settings changed event with wParam equal to SPI_SETNONCLIENTMETRICS. The patch detects that and refreshes all our ui settings plus it fires off a theme changed event.

Comment on attachment 523137 [details] [diff] [review] settings changed notification v.1 Can we not refactor this and the WM_THEMECHANGED code into a separate method?

rtl updates on the frame plus previous changes requested. status panel support is not in this, it also has some rtl issues. I'll post that in a followup patch.

(In reply to comment #99) > Comment on attachment 523137 [details] [diff] [review] > settings changed notification v.1 > > Can we not refactor this and the WM_THEMECHANGED code into a separate method? sure, I'll update that locally.

I went ahead and flipped left-top / right-top to top-left / top-right per dbaron's suggestion. I also updated the xul element ids to match.

(In reply to comment #74) > Created attachment 522472 [details] [diff] [review] > UpdateNonClientMargins v.1 > > UpdateNonClientMargins changes so that we can have these custom margins working > all the time, which gives us the ability to compensate for our borderless glass > adjustment. One small update to this: + mNonClientMargins.Clear(); + mNonClientOffset.Clear(); should have been: + mNonClientMargins.SizeTo(-1,-1,-1,-1); + mNonClientOffset.Clear(); this caused a mochitest chrome test failure. The initial values should be set to disable chrome margins, vs. removing them. I've fixed this in my local patch queue.

try builds with everything put together available in a few hours: http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/jmathies@mozilla.com-92796d2403f6

with the latest tryserver build there is the same problem for the borders appears in a precedent bug (I don't remember its number), the border around the content area is opaque and when the window is maximized appears a white line above the tabs toolbar

(In reply to comment #106) > Created attachment 523774 [details] > border > > with the latest tryserver build there is the same problem for the borders > appears in a precedent bug (I don't remember its number), the border around the > content area is opaque and when the window is maximized appears a white line > above the tabs toolbar Looks like bug 637018. I'll see if I can track down the cause, but I wouldn't hold up landing these patches for it.

I think the main disconnect with the footers was that the personas team was primarily on OS X, where the title bar and footer are two completley seperate elements. On every version of windows (classic, luna, aero, glass), the window frame is a single surface, with the top and bottom connected together. I propose we: -for now ignore the footer images on windows -in the future switch to a new personas format that encourages designers to draw out the entire frame, and then leverage 9 image tiling for increasing the size of the window, or cropping the sides off entirely for OS X

(In reply to comment #107) > (In reply to comment #106) > > Created attachment 523774 [details] > > border > > > > with the latest tryserver build there is the same problem for the borders > > appears in a precedent bug (I don't remember its number), the border around the > > content area is opaque and when the window is maximized appears a white line > > above the tabs toolbar > > Looks like bug 637018. Nope, this looks like you're regressing bug 629709.

(In reply to comment #109) > (In reply to comment #107) > > (In reply to comment #106) > > > Created attachment 523774 [details] > > > border > > > > > > with the latest tryserver build there is the same problem for the borders > > > appears in a precedent bug (I don't remember its number), the border around the > > > content area is opaque and when the window is maximized appears a white line > > > above the tabs toolbar > > > > Looks like bug 637018. > > Nope, this looks like you're regressing bug 629709. bug 629709 was exactly what I meant

Ignoring the footer image is a significant change that will have to be communicated loudly to the Personas designer community and will require fairly extensive changes to the GetPersonas website (and probably the Personas Plus add-on, but I'll have to double check that). If this is required by the UX team that's fine, it will just be difficult (possibly impossible) to the rest of this ready in time for the next release of Firefox so it may have to wait for a later release.

(In reply to comment #111) > Ignoring the footer image is a significant change that will have to be > communicated loudly to the Personas designer community and will require fairly > extensive changes to the GetPersonas website (and probably the Personas Plus > add-on, but I'll have to double check that). > > If this is required by the UX team that's fine, it will just be difficult > (possibly impossible) to the rest of this ready in time for the next release of > Firefox so it may have to wait for a later release. Dropping the footer entirely for all platforms isn't what we want to do. I was thinking that for (7 / Vista) + aero glass we would simply not display it. That would not require much of a change to the site (just text on the differences in behavior) and would not require changes to personas or the personas add-on. As far schedule goes, this is purely a (really really big) cosmetic win for Windows 7 / Vista glass users. We can delay it to 6 or 7 if needed.

It requires changes to the site because I will have to explain to designers that their footer images will not be used on those platforms, as well as updating the documentation and tutorials (which also require L10N changes). Based on the stats I have this will impact around 35 million users (41% of Firefox users are on Win 7/Vista, and 19% use Personas), so it would be great to get this out to them ASAP. I spoke with morgamic and apparently I will be able to get resources to make the site changes. I'll sort out what needs to be done and file those bugs as soon as I can.

We can discuss the footer choices at the personas meeting tomorrow. Generally speaking we need to move towards one big image that we can use, not just for modern windows, but contexts like Panorama (on OS X, Linux, etc.)

I'm currently working on sorting out and filing the necessary bugs to get the documentation and website updated to account for this change. Would it make sense to mark this as dependent on those when they're filed?

(In reply to comment #115) > I'm currently working on sorting out and filing the necessary bugs to get the > documentation and website updated to account for this change. Would it make > sense to mark this as dependent on those when they're filed? Not dependent but please post them here so we can track. I'm hoping to get these reviews done and land on m-c next week. (I need to post a couple patches here for the footer and a fix for the white line problem mentioned in comment 106 still but that shouldn't be a big deal.) That means we'd be landing two weeks into the 6.0 release cycle, so we'd have about 3.5 months before we release to the world. Do you think that will provide enough time for site changes Deb? If not we'll have to wait for 7.0.

Why have you added in browser/browser.css: #main-window[inFullscreen] #titlebar { display: none; } and not only changed in content/browser.css: #main-window[inFullscreen] > #titlebar, #main-window[inFullscreen] .titlebar-placeholder, #main-window:not([tabsintitlebar]) .titlebar-placeholder { display: none; } This would be easier and not leave a unused selector.

(In reply to comment #117) > Why have you added in browser/browser.css: > > #main-window[inFullscreen] #titlebar { > display: none; > } > > and not only changed in content/browser.css: > > #main-window[inFullscreen] > #titlebar, > #main-window[inFullscreen] .titlebar-placeholder, > #main-window:not([tabsintitlebar]) .titlebar-placeholder { > display: none; > } > > This would be easier and not leave a unused selector. ah, thanks. The xul structure changes broke that selector. Updated.

Dao asked me to check this. This looks comparable to fx without these patches. Roc, see anything here that looks out of the ordinary?

Added back in the bit of css that removes the footer image: + #browser-bottombox[lwthemefooter="true"] { + background-image: none !important; + background: none !important; + }

(In reply to comment #63) > Created attachment 519169 [details] [diff] [review] > theme work v.2 The Arc fill code in this regressed talos paint by about 50ms which is pretty bad. So I'm changing this up to use a poor man's arc via a set of three rectangles. Visually it looks the same (we clip the background then paint a rounded border on top) and it's fast.

(In reply to comment #121) > (In reply to comment #63) > > Created attachment 519169 [details] [diff] [review] > > theme work v.2 > > The Arc fill code in this regressed talos paint by about 50ms which is pretty > bad. So I'm changing this up to use a poor man's arc via a set of three > rectangles. Visually it looks the same (we clip the background then paint a > rounded border on top) and it's fast. Note this only applies to glass. Windows handles the clipping when rendering themed window frames.

Jim, I have tried this http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/jmathies@mozilla.com-809481a3522d/ but I noticed that when I have applied a Persona the four corner of the window are not round

And the browser-bottombox, when I have a Persona applied, still have the border around it, as opposed to how it looks in the image on the right in this screenshot: https://bug590945.bugzilla.mozilla.org/attachment.cgi?id=522550

Do we know which release this is expected to land in yet? I'm working on updating the docs and knowing whether this will be "Firefox 6+" or whatever would be useful.

Its severe bug with current patches i guess

(In reply to comment #124) > And the browser-bottombox, when I have a Persona applied, still have the border > around it, as opposed to how it looks in the image on the right in this > screenshot: https://bug590945.bugzilla.mozilla.org/attachment.cgi?id=522550 That was a test build with features disabled. I was searching for a talos paint regression.

(In reply to comment #125) > Do we know which release this is expected to land in yet? I'm working on > updating the docs and knowing whether this will be "Firefox 6+" or whatever > would be useful. The cut off for Fx 6 is mid May so at this point Fx6 still seems plausible. ping: Dao! (for reviews)

tpaint regression fix

Comment on attachment 522470 [details] [diff] [review] add clear to nsmargin v.1 this doesn't seem to apply (anymore?)

Comment on attachment 527854 [details] [diff] [review] theme work v.3 Confirmed on try server the tpaint regression is fixed. Neil, same patch except for the clearing code. It's no longer using Arc.

updated to latest mc.

updated to latest. the only change here: + #browser-bottombox[lwthemefooter="true"] { should have been: + #browser-bottombox:-moz-lwtheme {

Comment on attachment 527854 [details] [diff] [review] theme work v.3 If the only difference is some GFX stuff then maybe it would be better to get a GFX peer to review the change? It's not as if I have a system that I can actually test the change on...

Comment on attachment 527854 [details] [diff] [review] theme work v.3 Joe, this has already been reviewed except for the gfxContext code in the middle that does the corner clearing. Curious if you would be able to look that over?

I still can't get this set of patches to apply. Not sure if there's just a lot of movement in these files or if I applied them in the wrong order, although I think I tried all possible combinations...

(In reply to comment #136) > I still can't get this set of patches to apply. Not sure if there's just a lot > of movement in these files or if I applied them in the wrong order, although I > think I tried all possible combinations... Hrm, bug 614720 and it's descendants broke all the widget and theme patches. Here's a roll up to 'settings changed notification v.1'. 'browser frame v.3' & 'status panel v.1' sit on top of that.

(In reply to comment #137) > Created attachment 528632 [details] [diff] [review] > rollup to settings changes patch > > (In reply to comment #136) > > I still can't get this set of patches to apply. Not sure if there's just a lot > > of movement in these files or if I applied them in the wrong order, although I > > think I tried all possible combinations... > > Hrm, bug 614720 and it's descendants broke all the widget and theme patches. > Here's a roll up to 'settings changed notification v.1'. 'browser frame v.3' & > 'status panel v.1' sit on top of that. bug 473687 specifically landed on 4.24

Comment on attachment 527854 [details] [diff] [review] theme work v.3 Review of attachment 527854 [details] [diff] [review]: Looks good. I've suggested some minor improvements, but I think this is all ok. Note that I didn't review the Thebes code for correctness, just for API usage. ::: widget/src/windows/nsNativeThemeWin.cpp @@ +1620,5 @@ + nsUXThemeData::CheckForCompositor()) { + // Clear the corner regions underneath us where we don't want + // drawing to take place. This keeps the glass corners rounded + // by removing all content. + ctx->Save(); You can use gfxContextAutoSaveRestore here. @@ +1625,5 @@ + ctx->ResetClip(); + ctx->Translate(tr.TopLeft()); + + gfxContext::GraphicsOperator currentOp = ctx->CurrentOperator(); + ctx->SetOperator(gfxContext::OPERATOR_CLEAR); Turns out you don't actually have to save the currentOp here, since Save() and Restore() do that for you. (These two comments also apply to the if block below.) @@ +1695,5 @@ + ctx->Fill(); + ctx->Restore(); + + // Clear the right corner + ctx->Save(); Restoring, then saving again, seems a little heavyweight here. You could just save the matrix before translating, then explicitly set it back right here.

Comment on attachment 523591 [details] [diff] [review] status panel v.1 The border around the content area becomes opaque when I apply these patches.

Comment on attachment 527941 [details] [diff] [review] browser frame v.3 Err, I meant to r- this patch with the previous comment. As far as the status panel patch is concerned, it causes the status panel to be misaligned when maximizing the window.

(In reply to comment #140) > Comment on attachment 523591 [details] [diff] [review] [review] > status panel v.1 > > The border around the content area becomes opaque when I apply these patches. Hmm, I'm not able to reproduce. (I also have the patches in bug 633282 applied underneath this.) Is this the border you're referring to Dao? If so it looks like the combination of the two sets addresses the problem. (In reply to comment #141) > Comment on attachment 527941 [details] [diff] [review] [review] > browser frame v.3 > > Err, I meant to r- this patch with the previous comment. > > As far as the status panel patch is concerned, it causes the status panel to > be misaligned when maximizing the window. I can reproduce this, looks like I need to update when the size mode changes. I'll work something up.

I broke these up a little differently this time - css and js changes. The js changes include the maximized/fullscreen fix for the status panel.

(In reply to comment #125) > Do we know which release this is expected to land in yet? I'm working on > updating the docs and knowing whether this will be "Firefox 6+" or whatever > would be useful. Deb, since the merge is next tuesday and considering this will need a lot of bake time, I'm pushing this back to the 7.0 release.

https://bugzilla.mozilla.org/show_bug.cgi?id=594318 is a duplicate of this bug

I got there by maximizing and restoring the window. The border stayed like this when moving the window (also offscreen), resizing the window fixed it.

Comment on attachment 533333 [details] [diff] [review] browser css All ids and -moz-appearance values containing "left" and "right" should say "start" and "end" instead, so that they make sense for RTL. >+ #browser-bottombox:-moz-lwtheme { >+ background-image: none !important; >+ background: none !important; > } background-image is redundant >+ #main-window[sizemode="normal"] #window-frame-top-left-inner:-moz-lwtheme:-moz-locale-dir(ltr) { >+ border-left: 2px solid; >+ -moz-border-left-colors: @glassActiveBorderColor@ @glassBorderHighlightColor@; >+ } >+ >+ #main-window[sizemode="normal"] #window-frame-top-left-inner:-moz-lwtheme:-moz-locale-dir(rtl) { >+ border-right: 2px solid; >+ -moz-border-right-colors: @glassActiveBorderColor@ @glassBorderHighlightColor@; >+ } use -moz-border-start: 2px solid; here > #main-window[sizemode="normal"] #titlebar-buttonbox:-moz-lwtheme { > margin-top: -2px; > } > >+ #main-window[sizemode="normal"] #titlebar-buttonbox:-moz-lwtheme:-moz-locale-dir(ltr) { >+ margin-right: 1px; >+ } >+ >+ #main-window[sizemode="normal"] #titlebar-buttonbox:-moz-lwtheme:-moz-locale-dir(rtl) { >+ margin-left: 1px; >+ } use -moz-margin-end

Comment on attachment 533335 [details] [diff] [review] browser js >+var FrameSettings = { >+ handleEvent: function (event) { >+ if (event.type == "resize") { >+ if (event.target != window) >+ return; >+ this.update(); >+ } >+ }, >+ >+ init: function () { >+ window.addEventListener("resize", this, false); >+ }, >+ >+ uninit: function () { >+ window.removeEventListener("resize", this, false); I don't think this is needed. >+ }, >+ >+ update: function () { >+ this.updateFrameMarginSettings(this._chromeMarginsEnabled); >+ }, >+ >+ updateFrameMarginSettings: function (chromemargin) { >+ this._chromeMarginsEnabled = chromemargin; Why do you need the chromemargin argument? Can't you just check the window's chromemargin attribute? >+ let sm = document.documentElement.getAttribute("sizemode"); >+ if (sm == "maximized" || sm == "fullscreen") { >+ let sp = document.getElementById("statusbar-display"); >+ sp.style.marginLeft = "0px"; >+ sp.style.marginRight = "0px"; >+ return; >+ } >+ >+ // setup the correct border radii on the corners of the window >+ // based on the width/height of the frame. we also limit this >+ // since there's a maximum radius Windows applies. >+ let maxRadius = 6; >+ let isLTR = window.getComputedStyle(document.documentElement, null).direction == >+ "ltr" ? true : false; >+ >+ let element = document.getElementById("window-frame-top-left-inner"); >+ let leftwidth = element.boxObject.width > maxRadius ? maxRadius : element.boxObject.width; >+ if (isLTR) >+ element.style.borderTopLeftRadius = leftwidth + "px"; >+ else >+ element.style.borderTopRightRadius = leftwidth + "px"; >+ >+ element = document.getElementById("window-frame-top-right-inner"); >+ let rightwidth = element.boxObject.width > maxRadius ? maxRadius : element.boxObject.width; >+ if (isLTR) >+ element.style.borderTopRightRadius = rightwidth + "px"; >+ else >+ element.style.borderTopLeftRadius = rightwidth + "px"; >+ >+ element = document.getElementById("window-frame-bottom-inner"); >+ let height = element.boxObject.height > maxRadius ? maxRadius : element.boxObject.height; >+ element.style.borderBottomLeftRadius = height + "px"; >+ element.style.borderBottomRightRadius = height + "px"; >+ >+ // set the margin on the status panel equal to the frame >+ // width. >+ let sp = document.getElementById("statusbar-display"); >+ sp.style.marginLeft = (chromemargin ? leftwidth : 0) + "px"; >+ sp.style.marginRight = (chromemargin ? leftwidth : 0) + "px"; chromemargin ? leftwidth + "px" : ""; >+ }, You seem to be doing a lot of unnecessary work here for resize events that don't change the sizemode, i.e. when resizing a restored window.

Jimm/Dao , can you push this to UX branch for testing?

Hi Jim, would this code still allow the theme/appearance look and act more like Zune Software or iTunes, meaning no or minimal visible frameborders, custom min/max/close buttons, etc? It seems to me that we could go that direction with Personas skinning the entire window to look more modern.

@Dennis, I think we can't skin the captions with this since this code only modifies the borders and not the content atm. But it fixes all the bugs found with larger or smaller border padding.

Jim, Dao, how close are we on this. We're about to wrap up Firefox 7 development period and it would be really nice to get this (bug 618353) in before the cut-over to aurora.

(In reply to comment #155) > Jim, Dao, how close are we on this. We're about to wrap up Firefox 7 > development period and it would be really nice to get this (bug 618353) in > before the cut-over to aurora. Making the next train (July 5th) is questionable. I'm currently working on the updates now though so we shall see how it goes.

(In reply to comment #150) > Comment on attachment 533333 [details] [diff] [review] [review] > browser css > > All ids and -moz-appearance values containing "left" and "right" should say > "start" and "end" instead, so that they make sense for RTL. Should I also update the naming for window-frame-left & window-frame-right, which are already in mc?

Those aren't used anywhere yet, are they? Anyway, sounds like they should be updated.

(In reply to comment #158) > Those aren't used anywhere yet, are they? Anyway, sounds like they should be > updated. Went ahead and did that along with the other changes. I currently can't reproduce that maximized -> normal bug in the bottom border. I'll push all the patches to try to generate a release build for testing.

(In reply to comment #149) > Created attachment 537989 [details] > screenshot of broken left border > > I got there by maximizing and restoring the window. The border stayed like > this when moving the window (also offscreen), resizing the window fixed it. Dao, would you mind testing with that try build? I've been playing with restore-maximise and don't see the issue. Also, were you running a theme or persona in this screen shot? Your titlebar looks to be themed, but the glass on the side is not.

> Also, were you running a theme or persona in this screen shot? A persona. > Your titlebar looks to be themed, but the glass on the side is not. That's exactly what I was complaining about.

I didn't find the theme you were using, but I found one that has the same side effect on the left side. There's not much we can do here though, in this case the developer themed the #titlebar via css. Fixing this requires changes to the theme. I believe the add-on site has version checks, so themes not compatible would be disabled on the site. (Deb could probably confirm this.) #titlebar{ box-shadow:0 -1px rgba(178,111,0,.5) inset; border-top:2px solid; border-bottom:2px solid; -moz-border-top-colors:#B26F00 #EED4A2; -moz-border-bottom-colors:#EED4A2 #B26F00; background-image:url(chrome://global/skin/hout-hover.png) }

(In reply to comment #165) > > Also, were you running a theme or persona in this screen shot? > > A persona. > > > Your titlebar looks to be themed, but the glass on the side is not. > > That's exactly what I was complaining about. Could you point me to the persona you were running?

It's the fifth from the right on <http://design-noir.de/mozilla/themes/>. There's no way for this bug to be persona-dependent, though.

(In reply to comment #168) > It's the fifth from the right on <http://design-noir.de/mozilla/themes/>. > There's no way for this bug to be persona-dependent, though. Hmm, that's working fine here, Are you still seeing this in the try build I posted?

just tried the try build. its perfect

I tried the try build with fresh profile. After maximizing the window with maximizing button, all of the caption buttons disappear. Can i file a new bug?

Painting the browser on resizing is slow , it feels laggy and twitchy. Will it be fixed in this bug or some other work around bug would be created?

(In reply to comment #171) > I tried the try build with fresh profile. After maximizing the window with > maximizing button, all of the caption buttons disappear. Can i file a new > bug? What operating system and desktop were you testing on?

Win7 x64, default theme with Aero Glass. But i cannot reproduce the problem. :S BTW, while resizing/maximizing, all glass surfaces became black. It is annoying and very ugly. Also, i can confirm bogas' report: resizing is very laggy.

(In reply to comment #174) > Win7 x64, default theme with Aero Glass. But i cannot reproduce the problem. > :S > > BTW, while resizing/maximizing, all glass surfaces became black. We have that problem currently in the titlebar. It's unrelated to these patches. It has to do with timing in setting glass surfaces and the repaint.

(In reply to comment #175) > (In reply to comment #174) > > Win7 x64, default theme with Aero Glass. But i cannot reproduce the problem. > > :S > > > > BTW, while resizing/maximizing, all glass surfaces became black. > > We have that problem currently in the titlebar. It's unrelated to these > patches. It has to do with timing in setting glass surfaces and the repaint. Yes, you are right. Is there a bug about it? And what about laggy resizing? It is indeed came with this patch.

Win7 x64, modded theme with Aero Glass. resizing causes pc ui unresponsive lags with aero,, the problem didnt happen wearing a persona which was how i tested first time couldn't reproduce the maximizing button issue

OK, i can clearly reproduce the maximizing button issue. STR: 1. Maximize/demaximize the window with the button 2. Do the same with Aero Snap 3. Repeat the first two steps randomly until the maximizing button disappear

(In reply to comment #178) > OK, i can clearly reproduce the maximizing button issue. STR: > > 1. Maximize/demaximize the window with the button > 2. Do the same with Aero Snap > 3. Repeat the first two steps randomly until the maximizing button disappear I'm using Soft7 theme and it works for me.

When I hover a button or open the bookmarks sidepane it shows the titlebar drawn by dwm instead of firefox... Could that be fixed so the titlebar is always drawn by firefox?

Did this make the cutoff for Fx7? I'm a little confused :)

(In reply to comment #181) > Did this make the cutoff for Fx7? I'm a little confused :) Nope. It'll have to wait until Firefox 8 gets uplifted.

I think the perceived resizing performance may just be the persona on the edge tricking you into thinking things are slower. I don't see a noticeable difference between these patches and a nightly. But the resize performance of the window is made more apparent when the frame is textured with the persona. Note in testing, please check to see if you have hardware acceleration enabled through the options. Resizing with acceleration enabled is much slower than with gdi. If you see performance issues, please note whether or not acceleration is enabled and whether or not you see the same problem with gdi. Also, please do not post bug reports here about try builds if you are running a non-ms sanctioned Windows theme. Any problems with these themes will be handled in separate bugs. There are two bugs with these patch that I know of currently - 1) w/hardware acceleration enabled including d3d9/d3d10 layers, resizing the window to large dimensions can cause a redraw ('flash') problem with the window. I've been trying to track this down. 2) On XP, maximize/restore can result in content not getting positioned correctly, independent of acceleration settings. I'll post new try builds as I make progress.

Whats the status of the new patch? Waiting for it to land to UX build for testing!

(In reply to comment #184) > Whats the status of the new patch? Waiting for it to land to UX build for > testing! Bug 672885 is a hard blocker on these patches landing.

I've posted Bug 730219 that maybe is related to these changes. (see also 'Related info' there) Jim, I can't find your last try-build for this bug. Searched a lot in ftp and hg (for keyword). Thanks everybody for your work.