Alex Faaborg [:faaborg] (Firefox UX) Reporter
	Description • 17 years ago

The new theme for windows was designed to mirror curves found in the keyhole form of the navigation controls throughout the navigation toolbar.  This helps the keyhole form feel like it fits into the design as a whole, and creates a sleeker and more aerodynamic appearance throughout the browser.

Proto is already updated with this change, and since the keyhole shaped navigation controls are about to land for Windows, we need to update the site button and search button as part of the visual refresh.

Since this bug touches the site button, we are also wrapping in visual changes for the EV state.

Dao: beltzner said on IRC that you might potentially have cycles to work on this.

	Alex Faaborg [:faaborg] (Firefox UX) Reporter
	Comment 1 • 17 years ago

Notes from the mockup:

-The intersecting circle on XP should be 42x42 given a text field that is 22 pixels tall.

-The intersecting circle on Vista should be 28x28 given a text field that is 22 pixels tall.

	Alex Faaborg [:faaborg] (Firefox UX) Reporter
	Comment 2 • 17 years ago

This mockup covers visual changes to the site button for different identity states.

Notes:

-These styles emphasize that the site button is chrome and not content.

-I just did a quick gradient map on the Vista forward button, this is only an example of how we might style the EV site button with a matching texture.

On XP the navigation controls are already green, so we will definitely want to match the existing controls.

	Dão Gottwald [:dao] Assignee
	Comment 3 • 17 years ago

Could I please get a mockup of how this is expected to work with a non-default font size?

	Alex Faaborg [:faaborg] (Firefox UX) Reporter
	Comment 4 • 17 years ago

>Could I please get a mockup of how this is expected to work with a non-default
>font size?

Sure thing, I've been giving it some thought and there are a few different ways we could handle it.  Overall it will probably come down to what is the easiest to implement though, since this is a boundary case.

	Dão Gottwald [:dao] Assignee
	Comment 5 • 17 years ago

(In reply to comment #4)
> >Could I please get a mockup of how this is expected to work with a non-default
> >font size?
> 
> Sure thing, I've been giving it some thought and there are a few different ways
> we could handle it.  Overall it will probably come down to what is the easiest
> to implement though, since this is a boundary case.

Problem is, even though I could imagine a few possible styles, I don't see a way to implement any with the tools that we have: CSS, images and theme-specific XBL. The latter should be avoided for performance reasons.

Btw, I think we've been using the dropmarker for menus only. And strictly speaking the identity panel isn't a menu.

	Michaël Arnauts
	Comment 6 • 17 years ago

Yet, the drop-down arrow makes it clear that it can be clicked. If I hadn't read about the identity panel, it would never click it I guess.

	Dão Gottwald [:dao] Assignee
	Comment 7 • 17 years ago

We'd be fixing the current asymmetry (A) by creating a new one (B). Alex says that's not a showstopper, Safari is already doing something similar. I agree, asymmetry between the Location bar and the Search bar isn't a showstopper. However, compared to A it's more visible. So if B is considered minor, that challenges the reasoning behind this proposal.

	Alex Faaborg [:faaborg] (Firefox UX) Reporter
	Comment 8 • 17 years ago

Dão, I agree that the asymmetry created with B isn't ideal.  What do you think of potentially only curving the site button?

	Dão Gottwald [:dao] Assignee
	Comment 9 • 17 years ago

(A) is less of a problem (at least less in-your-face than B), because the size and appearance of the back/forward item and the site button are different, and because there are two icons in between. So I don't think that's worth it.

But sure, one curve would be better than two :)

	Mike Beltzner [:beltzner, not reading bugmail]
	Comment 10 • 17 years ago

At the very least, the site button needs to not look as ... awkward and puffy? ... as it does now. That's probably achievable by not using toolbox styling.

	Alex Faaborg [:faaborg] (Firefox UX) Reporter
	Comment 11 • 17 years ago

After thinking about this more, I'm concerned about how using images for the site button would mesh with the user switching OS themes.  Out of curiosity, how did we get the tab strip to correctly match to the OS theme in Firefox 2?

	Darren Kalck [:D-Kalck]
	Comment 12 • 17 years ago

It seems to be a color blending with the theme background color.
At the top, it's Luna, bellow is Knorr.

	Dão Gottwald [:dao] Assignee
	Comment 13 • 17 years ago

We've been using semi-transparent images and -moz-dialog as the background color. For Firefox 3, we also use ThreeDShadow for some of the tabs' borders.

	Alex Faaborg [:faaborg] (Firefox UX) Reporter
	Comment 14 • 17 years ago

-I think we should exactly match the texture of the tabs, but have the glass reflection centered vertically, like the forward button

-I agree with Dao that the curve looks bad up against the right side of the location bar, the search button needs to be square

-For consistency I think the site button should get a drop marker (it isn't a menu, but it does contain widgets that you can select to take you to other places, and hitting it produces a panel)

-We should use the same ratio as the forward button for the curve, Vista uses an intersecting circle that is 28 pixels tall, XP has one that is 42 pixels tall.

	Alex Faaborg [:faaborg] (Firefox UX) Reporter
	Comment 15 • 17 years ago

Since we are proposing that we implement the buttons with images, they should just stretch vertically when the font changes size in the location bar.

	Dão Gottwald [:dao] Assignee
	Comment 16 • 17 years ago

What's the base background color, what's the border color (opaque or transparent, necessarily not a system color)?
Are there hover and active states?
What about RTL?

	Mike Beltzner [:beltzner, not reading bugmail]
	Comment 17 • 17 years ago

Comment on attachment 299530 [details]
Site button identity states (Normal, SSL, EV)

Crap, I should have plussed this a while back.

It's an important thing for us to get right in terms of being able to echo the etch and keyhole design.

I think in the SSL case, we're going to want the background to be blue.

Dao: can we assign this to you? Will it take long to work on?

	Dão Gottwald [:dao] Assignee
	Comment 18 • 17 years ago

I've already been assigned to this bug, but IMHO the specification isn't workable yet.

	Mike Beltzner [:beltzner, not reading bugmail]
	Comment 19 • 17 years ago

(In reply to comment #16)
> What's the base background color, what's the border color (opaque or
> transparent, necessarily not a system color)?

We're getting Vlad to see if he can help us get a system colour, but for now let's just use ButtonFace (see http://iansgraham.com/books/xhtml1/appd/update-23feb2000.html to see what that looks like)

> Are there hover and active states?

On XP I think there probably should be, yes. On Vista I believe just active. Alex might have more thoughts here.

> What about RTL?
 
Yup, we'll need an RTL version, too.

(In reply to comment #18)
> I've already been assigned to this bug, but IMHO the specification isn't
> workable yet.

So, to be clear, what you need to continue are:

 - background colour
 - border colour
 - hover/active state descriptions

	Dão Gottwald [:dao] Assignee
	Comment 20 • 17 years ago

Our RTL interface isn't just mirrored, because the location bar is always LTR (except for the dropmarker).

	Dão Gottwald [:dao] Assignee
	Comment 21 • 17 years ago

(In reply to comment #19)
> for now let's just use ButtonFace

I see a fundamental problem here: Along the curve, we want transparency on the left side and some native color on the right side. That doesn't seem doable with CSS and PNG images. So for the curve we need to use SVG (implies a Ts/Txul hit) or a non-native background color (and border color too, as already mentioned).

	Alex Faaborg [:faaborg] (Firefox UX) Reporter
	Comment 22 • 17 years ago

>I've already been assigned to this bug, but IMHO the specification isn't
>workable yet.

Sorry, I thought matching the appearance of the tab background image would be good enough but as you pointed out that doesn't account for the border, and we can't draw the border with css because of the curve.

>but for now let's just use ButtonFace

The problem with buttonface this is that it gives us a grey shade on a light purple toolbar on Vista.  Ideally we would use a transparent image and the base color would be the toolbar itself, similar to how the tab strip lets the background color shine through.

I honestly don't know what we should do about the borders.

Could we pick a shade of grey for the edges that when placed over the toolbar looks ok?  Note how the proto end cap buttons don't fade exactly into the line color of the text field, but it still works reasonably well.

	Dão Gottwald [:dao] Assignee
	Comment 23 • 17 years ago

I found a way to use CSS colors for the background and the border.

	Dão Gottwald [:dao] Assignee
	Comment 24 • 17 years ago

- same radius for XP and Vista
- no special image for the shading, just reusing the image of the active tab
- no hover state, no active state

All that could be tweaked with little risk after beta 4. But basically, this patch should work.

I couldn't resist cleaning up more of larry's css code.

	Darren Kalck [:D-Kalck]
	Comment 26 • 17 years ago

It looks nice, but the drop-marker is missing.

	Dão Gottwald [:dao] Assignee
	Comment 27 • 17 years ago

using a 21px radius for XP, the Vista code is commented out.

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 30 • 17 years ago

Comment on attachment 306350 [details] [diff] [review]
updated to trunk

>Index: browser/themes/gnomestripe/browser/browser.css

>+#identity-popup-content-host ,
> #identity-popup-content-owner {
>-   font-weight: bold;
>-   max-width: 300px;
>-   margin-bottom: 0 !important;
>+  font-size: 140%;
>+  font-weight: bold;
>+  max-width: 300px;
> }

This makes the larger font size apply to |.verifiedDomain #identity-popup-content-owner| when it didn't used to. Not sure that's desired. Same comment applies to the other files as well.

>-.verifiedDomain > #identity-popup-container > #identity-popup-icon {
>-    -moz-image-region: rect(64px, 64px, 128px, 0px);
>+#identity-popup.verifiedDomain > #identity-popup-container > #identity-popup-icon {
>+  -moz-image-region: rect(64px, 64px, 128px, 0px);

I'm not sure this extra specificity actually helps. I know very little about CSS selector matching beyond what's in http://developer.mozilla.org/en/docs/Writing_Efficient_CSS, but http://developer.mozilla.org/en/docs/Writing_Efficient_CSS#Don.27t_qualify_ID-categorized_rules_with_tag_names_or_classes makes me think this might not actually be better than what was there before. Unless there's some reason the extra specificity is needed?

I just skimmed the new styling rules and didn't review them in depth, because you're much better at determining the best way to style something than I am, and it looks like you've covered all the bases with testing this.

	AndrewM
	Comment 31 • 17 years ago

I noticed that the patch adds lines with -moz-pre-wrap, would it make sense to use pre-wrap instead?

I say this because I saw that in bug 418543, roc says that -moz-pre-wrap is deprecated and its use should be phased out. (There is a patch waiting for approval that converts all instances -moz-pre-wrap to pre-wrap.)

	Dão Gottwald [:dao] Assignee
	Comment 32 • 17 years ago

(In reply to comment #30)
> This makes the larger font size apply to |.verifiedDomain
> #identity-popup-content-owner| when it didn't used to. Not sure that's desired.

It's not desired, since the owner label says "(no information provided)" in that case. I'll fix that.

> >-.verifiedDomain > #identity-popup-container > #identity-popup-icon {
> >-    -moz-image-region: rect(64px, 64px, 128px, 0px);
> >+#identity-popup.verifiedDomain > #identity-popup-container > #identity-popup-icon {
> >+  -moz-image-region: rect(64px, 64px, 128px, 0px);
> 
> I'm not sure this extra specificity actually helps. I know very little about
> CSS selector matching beyond what's in
> http://developer.mozilla.org/en/docs/Writing_Efficient_CSS, but
> http://developer.mozilla.org/en/docs/Writing_Efficient_CSS#Don.27t_qualify_ID-categorized_rules_with_tag_names_or_classes
> makes me think this might not actually be better than what was there before.
> Unless there's some reason the extra specificity is needed?

.genericClass > #concreteElement isn't in the spirit of "Writing Efficient CSS" (which states: "all of our classes will be unique"). I don't think my change makes it significantly more or less efficient, but I do think it makes these rules more meaningful.

(In reply to comment #31)
> I noticed that the patch adds lines with -moz-pre-wrap, would it make sense to
> use pre-wrap instead?

Yes, thanks.

	Mike Beltzner [:beltzner, not reading bugmail]
	Comment 33 • 17 years ago

Comment on attachment 305264 [details]
screenshots

Is RTL broken here?

	Dão Gottwald [:dao] Assignee
	Comment 34 • 17 years ago

(In reply to comment #33)
> (From update of attachment 305264 [details])
> Is RTL broken here?

No -- see comment 20.

	Reed Loden [:reed]
	Comment 37 • 17 years ago

Checking in browser/themes/gnomestripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/gnomestripe/browser/browser.css,v  <--  browser.css
new revision: 1.198; previous revision: 1.197
done
Checking in browser/themes/pinstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/pinstripe/browser/browser.css,v  <--  browser.css
new revision: 1.131; previous revision: 1.130
done
Checking in browser/themes/winstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/browser.css,v  <--  browser.css
new revision: 1.184; previous revision: 1.183
done
Checking in browser/themes/winstripe/browser/searchbar.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/searchbar.css,v  <--  searchbar.css
new revision: 1.24; previous revision: 1.23
done
Checking in browser/base/content/browser.css;
/cvsroot/mozilla/browser/base/content/browser.css,v  <--  browser.css
new revision: 1.59; previous revision: 1.58
done

	Darren Kalck [:D-Kalck]
	Comment 38 • 17 years ago

According to https://bugzilla.mozilla.org/attachment.cgi?id=299530
The dropdown icon is missing and the domain of the ssl connection is also missing.

	Johnathan Nightingale [:johnath]
	Comment 39 • 17 years ago

(In reply to comment #38)
> According to https://bugzilla.mozilla.org/attachment.cgi?id=299530
> The dropdown icon is missing and the domain of the ssl connection is also
> missing.

Bug 414627 deals with domain display on SSL.

	Darren Kalck [:D-Kalck]
	Comment 40 • 17 years ago

Should I file a new bug for missing dropdown ?

	Victor Bielawski
	Comment 41 • 17 years ago

On Windows Classic, the site button is not connected smoothly to the location bar.
Screenshot: http://img407.imageshack.us/img407/3456/identityglitchrd0.png

	Darren Kalck [:D-Kalck]
	Comment 42 • 17 years ago

The current one looks better IMHO.

	Dão Gottwald [:dao] Assignee
	Comment 43 • 17 years ago

(In reply to comment #40)
> Should I file a new bug for missing dropdown ?

Yes.

	Darren Kalck [:D-Kalck]
	Comment 44 • 17 years ago

Filed bug 420958

	Alex Faaborg [:faaborg] (Firefox UX) Reporter
	Comment 45 • 17 years ago

>using a 21px radius for XP, the Vista code is commented out.

/*margin: -4px 0;
+  padding: 3px 2px 3px 4px;
+  background-position: 0 3px;
+  -moz-border-radius-topleft: 14px;
+  -moz-border-radius-bottomleft: 14px;*/


Can we get these lines added to browser-aero.css or however we plan on using chrome overrides for vista specific themeing.

	Dão Gottwald [:dao] Assignee
	Comment 46 • 17 years ago

(In reply to comment #45)
> Can we get these lines added to browser-aero.css or however we plan on using
> chrome overrides for vista specific themeing.

Someone needs to fork browser.css. I can't do that.