414183 - Style site button and search button to mirror the keyhole form on Windows

Status: RESOLVED FIXED

(Firefox :: Theme, defect, P3)

Description

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

Attachment: Proposed changes to the site button and search button

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.

Comment 1

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

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.

Comment 2

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

Attachment: Site button identity states (Normal, SSL, EV)

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.

Comment 3

[Dão Gottwald [:dao]]

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

Comment 4

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

>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.

Comment 5

[Dão Gottwald [:dao]]

(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.

Comment 6

[Michaël Arnauts]

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.

Comment 7

[Dão Gottwald [:dao]]

Attachment: problem with the proposal

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.

Comment 8

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

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

Comment 9

[Dão Gottwald [:dao]]

(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 :)

Comment 10

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

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.

Comment 11

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

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?

Comment 12

[Darren Kalck [:D-Kalck]]

Attachment: The difference between two themes

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

Comment 13

[Dão Gottwald [:dao]]

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.

Comment 14

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

Attachment: Site button and search button

-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.

Comment 15

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

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.

Comment 16

[Dão Gottwald [:dao]]

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?

Comment 17

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

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?

Comment 18

[Dão Gottwald [:dao]]

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

Comment 19

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

(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

Comment 20

[Dão Gottwald [:dao]]

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

Comment 21

[Dão Gottwald [:dao]]

(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).

Comment 22

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

>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.

Comment 23

[Dão Gottwald [:dao]]

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

Comment 24

[Dão Gottwald [:dao]]

Attachment: patch

- 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.

Comment 25

[Dão Gottwald [:dao]]

Attachment: screenshots

Comment 26

[Darren Kalck [:D-Kalck]]

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

Comment 27

[Dão Gottwald [:dao]]

Attachment: patch v2

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

Comment 28

[Dão Gottwald [:dao]]

Attachment: updated to trunk

Comment 29

[Dão Gottwald [:dao]]

Attachment: updated to trunk

Comment 30

[:Gavin Sharp [email: [e-mail]]]

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.

Comment 31

[AndrewM]

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.)

Comment 32

[Dão Gottwald [:dao]]

(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.

Comment 33

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

Comment on attachment 305264 [details]
screenshots

Is RTL broken here?

Comment 34

[Dão Gottwald [:dao]]

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

No -- see comment 20.

Comment 35

[Dão Gottwald [:dao]]

Attachment: comments addressed

Comment 36

[Dão Gottwald [:dao]]

Attachment: updated to trunk

Comment 37

[Reed Loden [:reed]]

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

Comment 38

[Darren Kalck [:D-Kalck]]

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.

Comment 39

[Johnathan Nightingale [:johnath]]

(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.

Comment 40

[Darren Kalck [:D-Kalck]]

Should I file a new bug for missing dropdown ?

Comment 41

[Victor Bielawski]

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

Comment 42

[Darren Kalck [:D-Kalck]]

The current one looks better IMHO.

Comment 43

[Dão Gottwald [:dao]]

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

Yes.

Comment 44

[Darren Kalck [:D-Kalck]]

Filed bug 420958

Comment 45

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

>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.

Comment 46

[Dão Gottwald [:dao]]

(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.