Closed
Bug 588764
Opened 14 years ago
Closed 14 years ago
Content area needs a grey border around it
Categories
(Firefox :: Theme, defect)
Tracking
()
RESOLVED
FIXED
Firefox 4.0b8
Tracking | Status | |
---|---|---|
blocking2.0 | --- | betaN+ |
People
(Reporter: josh.tumath+bugzilla, Assigned: dao)
References
(Blocks 1 open bug, )
Details
Attachments
(4 files, 6 obsolete files)
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b5pre) Gecko/20100819 Minefield/4.0b5pre
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b5pre) Gecko/20100819 Minefield/4.0b5pre
Due to the landing of bug 554982, the content area and status bar of browser.xul now have no border. The mockups have a 1px semi-transparent border around them. Also, correct me if I'm wrong, but they have a box-shadow as well.
Reproducible: Always
Reporter | ||
Comment 1•14 years ago
|
||
I'll request for this to be a blocker for 2.0, because it might look strange for user's without this border.
Comment 2•14 years ago
|
||
Reporter | ||
Comment 3•14 years ago
|
||
(In reply to comment #2)
I don't understand. Are you saying that this should depend on your bug, or that this is a duplicate?
Summary: Content needs a grey border around it → Content area needs a grey border around it
Comment 4•14 years ago
|
||
The problem is I don't know. I reported the same issue there, because it is the same border.
Reporter | ||
Comment 5•14 years ago
|
||
When I read your bug, I assumed that this would be unrelated, because that relates to the toolbars, where as this relates to the actual content area. :) We'll just have to see whether the devs would rather handle them separately or in the same patch.
Comment 7•14 years ago
|
||
Updated•14 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 8•14 years ago
|
||
Shorlander: I'm guessing you're supporting this as a theme blocker?
I think patch should be:
#main-window[sizemode="normal"] #browser {
border-left:1px solid ThreeDShadow;
border-right:1px solid ThreeDShadow;
}
#main-window[sizemode="normal"] #browser-bottombox {
border-left:1px solid ThreeDShadow;
border-right:1px solid ThreeDShadow;
border-bottom:1px solid ThreeDShadow;
}
This adds a border to the window with the same color as the toolbar border when in windowed mode. It doesn't seem possible to add a shadow that expands into the border area, which comment 7 seems to suggest.
I don't know how to create a patch, but it seems that these lines should be added to themes/winstripe/browser/browser-aero.css ?
Comment 10•14 years ago
|
||
This shadow is also missing from toolbars background with tabs on bottom.
Comment 11•14 years ago
|
||
(In reply to comment #8)
> Shorlander: I'm guessing you're supporting this as a theme blocker?
Yes.
Reporter | ||
Updated•14 years ago
|
Summary: Content area needs a grey border around it → Content area needs a grey border and shadow around it
Comment 12•14 years ago
|
||
With Photoshop put the frame of Windows Explorer on top Minefield.
Clearly keeping the proportions and color pixels.
Comment 14•14 years ago
|
||
A possible solution to this bug should be:
#main-window[sizemode="normal"] #browser, #browser-bottombox {
border-left:1px solid ThreeDShadow !important;
border-right:1px solid ThreeDShadow !important;
}
#main-window[sizemode="normal"] #browser-bottombox {
border-bottom:1px solid ThreeDShadow !important;
}
#main-window[sizemode="normal"] #tab-view {
border: 1px solid ThreeDShadow !important;
}
#main-window[sizemode="maximized"] #tab-view {
border-top: 1px solid ThreeDShadow !important;
}
Comment 15•14 years ago
|
||
I don't see any difference.
Comment 16•14 years ago
|
||
(In reply to comment #15)
> I don't see any difference.
before there wasn't border in panorama view
Comment 17•14 years ago
|
||
Before there wasn't any border at all. It is quiet light-colored and that's why it is not very visible. The mockups show a more dark-colored line and a shadow which looks much better I think.
Comment 18•14 years ago
|
||
replacing ThreedShadow with ThreeDDarkShadow the border become more dark
Comment 19•14 years ago
|
||
This is how is Panorama suppose to look:
http://www.mozilla.sk/wp-content/uploads/2010/05/Firefox-4-Mockup-i06-Win7-Aero-InContentUI-TabCandy-Glass.png
Comment 20•14 years ago
|
||
"suppose"... Are you sure that will be done?
there is some bug for that?
Comment 21•14 years ago
|
||
I dunno.
Comment 22•14 years ago
|
||
This draw a border around toolbar when tabs are on bottom:
#main-window[sizemode="normal"] #navigator-toolbox:not([tabsontop="true"]) > #nav-bar + #customToolbars + #PersonalToolbar[collapsed="false"] +
#TabsToolbar:last-child:not(:-moz-lwtheme) {
border-left:1px solid ThreeDShadow !important;
border-right:1px solid ThreeDShadow !important;
}
#main-window[sizemode="normal"] [tabsontop="false"] #PersonalToolbar {
border-left: 1px solid ThreeDShadow !important;
border-right: 1px solid ThreeDShadow !important;
}
[tabsontop="false"] #PersonalToolbar {
border-top: 1px solid ThreeDShadow !important;
}
Assignee | ||
Comment 23•14 years ago
|
||
This adds an opaque border. A transparent one would regress bug 590468, so we don't want that.
Comment 9 is right regarding the shadow. Obviously it would regress bug 590468 as well.
Assignee | ||
Comment 24•14 years ago
|
||
realized that the border shouldn't be there when the window is maximized
Attachment #476793 -
Attachment is obsolete: true
Attachment #476796 -
Flags: review?(gavin.sharp)
Attachment #476793 -
Flags: review?(gavin.sharp)
Assignee | ||
Comment 25•14 years ago
|
||
Actually I think even an opaque border might regress bug 590468. We'll see if talos complains.
Comment 26•14 years ago
|
||
I think you're unlikely to see any change in talos numbers. Tp4 runs without browser chrome, and bug 590468 had no noticeable effect on the Tp4 numbers (that I remember).
Comment 27•14 years ago
|
||
I can take a look at how your patch changes the layer tree next time I boot into Windows.
I'm sure this will regress bug 590468.
If we really have to do this, I suggest we hardcode the border drawing into nsWindow painting code when there is a glass background.
Assignee | ||
Comment 29•14 years ago
|
||
We're currently using -moz-win-borderless-glass (bug 554982).
Assignee | ||
Updated•14 years ago
|
Assignee: dao → nobody
Status: ASSIGNED → NEW
Component: Theme → Widget: Win32
Product: Firefox → Core
QA Contact: theme → win32
Assignee | ||
Updated•14 years ago
|
Attachment #476796 -
Flags: review?(gavin.sharp)
Comment 30•14 years ago
|
||
If it isn't possible to get a proper translucent shadow around the content and the chrome it would be best to just use the default Windows 7 etch/border around the content; then use a proper shadow around the toolbars and other UI.
Would the design in this mockup be possible?
Comment 31•14 years ago
|
||
Timothy said he was going to do some stuff. He should do that stuff and then reassign to Dão!
Assignee: nobody → tnikkel
Updated•14 years ago
|
Assignee: nobody → dao
Assignee | ||
Comment 33•14 years ago
|
||
I'm not working on this since it's a widget bug now.
Assignee: dao → nobody
Reporter | ||
Comment 34•14 years ago
|
||
I thought it was now a theme bug? If we're following Stephen's new mockups, then we're basically just undoing bug 554982 and giving it some polish, aren't we?
Comment 35•14 years ago
|
||
(In reply to comment #34)
> If we're following Stephen's new mockups...
Which new mockups?
Reporter | ||
Comment 36•14 years ago
|
||
Updated•14 years ago
|
Component: Widget: Win32 → Theme
Product: Core → Firefox
QA Contact: win32 → theme
Comment 37•14 years ago
|
||
Fixing Bug 590945 should make the possibility of having an alternatively themed glass window border (one with a shadow in it) real, should it not?
Reporter | ||
Updated•14 years ago
|
Summary: Content area needs a grey border and shadow around it → Content area (and addons bar) needs a glass border and around it
Assignee | ||
Comment 38•14 years ago
|
||
(In reply to comment #30)
> Created attachment 479464 [details]
> Design with default Windows etch/border
>
> If it isn't possible to get a proper translucent shadow around the content and
> the chrome it would be best to just use the default Windows 7 etch/border
> around the content; then use a proper shadow around the toolbars and other UI.
>
> Would the design in this mockup be possible?
The etch is outside of the client area, so the toolbar border can't be drawn there.
Summary: Content area (and addons bar) needs a glass border and around it → Content area needs a grey border and shadow around it
Component: Theme → Widget
Product: Firefox → Core
QA Contact: theme → general
I suggest we use a new Windows theme value to express the border look we want, and draw it in the theme engine. This will prevent large layers from needing to be created.
Jim, I know you're kinda swamped, but can you take this?
Assignee: nobody → jmathies
felipe, maybe you can take this?
Comment 41•14 years ago
|
||
(In reply to comment #39)
> I suggest we use a new Windows theme value to express the border look we want,
> and draw it in the theme engine. This will prevent large layers from needing to
> be created.
I agree, this seems like the best approach.
Comment 42•14 years ago
|
||
(In reply to comment #39)
> I suggest we use a new Windows theme value to express the border look we want,
> and draw it in the theme engine. This will prevent large layers from needing to
> be created.
>
> Jim, I know you're kinda swamped, but can you take this?
Hmm, maybe. I'm not sure how we would target just the content area. Looking through theme constants we have NS_THEME_WINDOW, NS_THEME_WIN_BORDERLESS_GLASS, and NS_THEME_WIN_GLASS. Would 1) adding padding to one of these plus 2)rendering a border in DrawWidgetBackground suffice?
(I think adding padding might actually shrink the drawing surface so I'm not sure this would work, have to test it.)
Actually, just adding a new theme value and painting it normally in DrawWidgetBackground would regress bug 590468. We need another approach.
One possibility would be to put one or more colored rectangles under or around the content area, using XUL elements with background-color and the "layer" attribute we added in bug 590468. Those should be converted to ColorLayers, which don't take up much memory, draw fast, and shouldn't cause bug 590468 to regress --- although I'd like to see a layer tree dump with the resulting patch, to confirm (set gDumpPaintList=1 in a debug build in the debugger).
If colored rectangles are too clumsy, then three elements, one for each piece of the border, each with a "layer" attribute, should work fine too.
Or we could just go crazy and implement supermagical optimizations for almost-entirely-transparent ThebesLayers, but I really don't want to do that for Firefox 4.
Comment 47•14 years ago
|
||
Why the current window style with -moz-win-glass doesn't regress bug 590468? Maybe we could add a margin to the browser <hbox> and draw the border as part of DrawWidgetBackground of the window.
But I don't know how we could pass the content-area dimensions info so we don't draw this border over the toolbars/addon-bar area (i.e. only to the sides of the content area).
Er yeah, I meant to mention that. I realized this morning that bug 601603 probably *did* just regress bug 590468. Before that bug, we pretended we didn't support -moz-win-glass for drawing, so nothing was drawn. I'll follow up in bug 601603.
Comment 49•14 years ago
|
||
For clarity the theme/appearance design preferences for this are:
1) Seamless border/shadow around content and toolbars (attachment 467570 [details])
2) Use default Aero border around content and overlap the top 2px (attachment 479464 [details])
3) Some other type of border if either of those two aren't possible
Comment 50•14 years ago
|
||
(In reply to comment #49)
> 2) Use default Aero border around content and overlap the top 2px (attachment
> 479464 [details])
Then why removing them in the first place?
Updated•14 years ago
|
Assignee: jmathies → nobody
Component: Widget → Theme
Product: Core → Firefox
QA Contact: general → theme
Comment 51•14 years ago
|
||
(In reply to comment #44)
> One possibility would be to put one or more colored rectangles under or around
> the content area, using XUL elements with background-color and the "layer"
> attribute we added in bug 590468. Those should be converted to ColorLayers,
> which don't take up much memory, draw fast, and shouldn't cause bug 590468 to
> regress --- although I'd like to see a layer tree dump with the resulting
> patch, to confirm (set gDumpPaintList=1 in a debug build in the debugger).
Switched this back to theme assuming we're going to try what roc suggested. I can put this together unless someone else takes it before I get back here.
Comment 52•14 years ago
|
||
I sent a patch to tryserver (http://hg.mozilla.org/try/rev/97829721007b) to get talos numbers using roc's suggestion, as I'm always scared of browser.xul changes.
http://bit.ly/aEUMQm
It seems it didn't cause any problems. I took a look at the layer tree and IIRC it is what is expected, but I'll confirm this soon.
Roc, I have a question: this was done using a solid background-color for the "border" elements, as these become ColorLayers which you mentioned are fast. Will we be able to do a more elaborate look here ("grey border and shadow")? Likely a rgba color will be needed at minimum, and maybe a border or shadow style. Unless we go for a simple solid border look.
It's actually fine to do whatever you want in those elements, as long as you keep them small.
Comment 54•14 years ago
|
||
Attached is the resulting layer tree with this approach implemented.
Note that the layer for the bottom bar will always be present (even if the add-on bar is not visible) because we also need a border on the bottom
Yeah, that looks good. Thanks!
Comment 56•14 years ago
|
||
Posting my WIP patch that implements a simple solid border and that I have used to test the resulting layer tree.
Dao, could you pick this up from here to tweak the appearance as you're proficient with how the CSS shadows on the toolbars are implemented?
Assignee | ||
Comment 57•14 years ago
|
||
Is there a way to reduce the window border? Any border added in the client area currently cuts away from the content area and visually makes the window border fatter.
Comment 58•14 years ago
|
||
It's possible to change the chromemargin left/right values from -1 (applies the default value) to something else. The default is 8 so a value of 6 or 7 probably gets the intended look.
The problem is that this overrides the system settings if the user chose a different setting.
Assignee | ||
Comment 59•14 years ago
|
||
(In reply to comment #58)
> It's possible to change the chromemargin left/right values from -1 (applies the
> default value) to something else. The default is 8 so a value of 6 or 7
> probably gets the intended look.
>
> The problem is that this overrides the system settings if the user chose a
> different setting.
Rather than setting absolute numbers, could the native margins be reduced by one or two pixels for -moz-win-borderless-glass?
Comment 60•14 years ago
|
||
The new Addon manager magically creates a good looking border around the content, why?
Assignee | ||
Comment 61•14 years ago
|
||
Assignee: nobody → dao
Attachment #471123 -
Attachment is obsolete: true
Attachment #475057 -
Attachment is obsolete: true
Attachment #476796 -
Attachment is obsolete: true
Attachment #479464 -
Attachment is obsolete: true
Attachment #485474 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #493505 -
Flags: review?(gavin.sharp)
Assignee | ||
Updated•14 years ago
|
Attachment #493505 -
Flags: review?(dolske)
Comment 62•14 years ago
|
||
Comment on attachment 493505 [details] [diff] [review]
patch
Out of curiosity, what's |layer="true"| do?
But patch looks fine, checked it on my Win7 box.
Attachment #493505 -
Flags: review?(gavin.sharp)
Attachment #493505 -
Flags: review?(dolske)
Attachment #493505 -
Flags: review+
Assignee | ||
Comment 63•14 years ago
|
||
Assignee | ||
Comment 64•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b8
Comment 65•14 years ago
|
||
With this patch the border's color of the toolbar and the content area is not the same because in the toolbar border code there is "background-clip: padding-box;"
Comment 66•14 years ago
|
||
i think this has given the border above the address and below the bookmarks bar the glass effect.
Comment 67•14 years ago
|
||
Replacing "background-clip:padding-box;" with "background-clip: border-box;" like this :
#navigator-toolbox,
#navigator-toolbox > toolbar {
border-color: @glassToolbarBorderColor@ !important;
background-clip: border-box !important;
}
then the border of the toolbar and the content area have the same color
Comment 68•14 years ago
|
||
Isn't this border way too light? Looks really bad in my eyes. Looks hardly better than before. :(
Comment 69•14 years ago
|
||
This border is the wrong color. Not only does it abruptly change from the navigation bar above it, but on a white background its just a different enough color to create a blur effect.
If its going to be white, it needs to be completely white (to at least avoid that blur effect on white pages).
But it should really be much darker to match the mockups
Comment 70•14 years ago
|
||
Comment 71•14 years ago
|
||
re-opening because this isn't right yet.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 72•14 years ago
|
||
We're not going to implement the shadow for reasons discussed in this bug. As for the border itself, file new bugs as needed.
Status: REOPENED → RESOLVED
Closed: 14 years ago → 14 years ago
Resolution: --- → FIXED
Summary: Content area needs a grey border and shadow around it → Content area needs a grey border around it
Comment 73•14 years ago
|
||
Note that both colors are wrong. The border around the top area is way to blueish (is there a separate bug for that?) and around the content it's to bright, while he border between the content and the toolbars needs to be brighter according to the mockups.
Additionally the area for the bottom-left and bottom-right window resizer seems to be now smaller than those of native windows applications, but I'm not sure this is related to this bug.
Assignee | ||
Comment 74•14 years ago
|
||
(In reply to comment #73)
> Note that both colors are wrong. The border around the top area is way to
> blueish (is there a separate bug for that?)
bug 617506 more or less
Comment 75•14 years ago
|
||
I've filed Bug 618399 on the basic "it's too light" problem
Comment 76•14 years ago
|
||
(In reply to comment #73)
> Additionally the area for the bottom-left and bottom-right window resizer seems
> to be now smaller than those of native windows applications, but I'm not sure
> this is related to this bug.
Filed bug 618420 on this.
Updated•14 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•