John Ford [:jhford] CET/CEST Berlin Time Reporter
	Description • 13 years ago

The left side of the awesomebar is visually inconsistent with the right side of the awesomebar because of a 1px white border around the domain name button.  This is on Linux, Mac and Windows.  On some platforms, the white border doesn't look equally sized, which makes it look like the button isn't aligned in the text box.

The picture that I have attached to this bug shows an example of this border.

	Jennifer Morrow [:Boriss] (UX)
	Comment 1 • 13 years ago

The problem (as I view it) is the 1-pixel thick "gutter" that currently surrounds the site identity button.  John is right: this is inconsistent with the right side of the URL bar.  However, it's a problem even beyond the inconsistency.  The 1-pixel gutter is visually awkward, creating a small white track that traps the eye and makes the identity button look as though it's rendering too small.  Also, because of the small shadow on the top of the URL bar, the gutter itself seems to thin at the top of the identity button and widen on the left and bottom.  We need to expand the identity button 2px on the top, left, and bottom to remove this gutter and bring it into visual consistency with the right side of the URL bar.

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

This doesn't block release without an agreed-upon UX design fix. Renominate once that's worked through, if you'd like.

	Jennifer Morrow [:Boriss] (UX)
	Comment 3 • 13 years ago

UX talked about this bug on 2/14/2011.  Some discussion is still needed to decide what, if any, action will be taken in Firefox 4.

As Faaborg correctly pointed out, there's a lot we'd ideally like to do to improve the URL bar, and a lot of it will have to wait for future versions of Firefox.

However, I think we'd all agree that the narrow gutter that currently surrounds the site identity button is a problem.

We could extend the site identity button to eliminate this gutter.  That would mean that if a site asked for a permission, such as a password or geolocation, the site identity button would still slide to the right and the permission icon would appear on the left.  I think that  extending the site identity button to the bottom and top of the URL bar, even in this permission state, is a huge improvement visually over what we have now.  I propose that we extend this button for Firefox 4 and develop a more complete solution for future versions.

Thoughts, UX?

	Stephen Horlander [:shorlander]
	Comment 4 • 13 years ago

(In reply to comment #3)
> We could extend the site identity button to eliminate this gutter.  That would
> mean that if a site asked for a permission, such as a password or geolocation,
> the site identity button would still slide to the right and the permission icon
> would appear on the left.  I think that  extending the site identity button to
> the bottom and top of the URL bar, even in this permission state, is a huge
> improvement visually over what we have now.  I propose that we extend this
> button for Firefox 4 and develop a more complete solution for future versions.
> 
> Thoughts, UX?

I agree that extending the site identity button to eliminate the gutter would be the best short-term fix.

In addition to that I think using a flatter more subtle style would give the left and right side more visual consistency. Using a lighter color scheme would also be more friendly to a wider range of favicons.

	Jennifer Morrow [:Boriss] (UX)
	Comment 5 • 13 years ago

(In reply to comment #4)
> Created attachment 513594 [details]
> Extended Site Identity Button
> 
> (In reply to comment #3)
> > We could extend the site identity button to eliminate this gutter.  That would
> > mean that if a site asked for a permission, such as a password or geolocation,
> > the site identity button would still slide to the right and the permission icon
> > would appear on the left.  I think that  extending the site identity button to
> > the bottom and top of the URL bar, even in this permission state, is a huge
> > improvement visually over what we have now.  I propose that we extend this
> > button for Firefox 4 and develop a more complete solution for future versions.
> > 
> > Thoughts, UX?
> 
> I agree that extending the site identity button to eliminate the gutter would
> be the best short-term fix.
> 
> In addition to that I think using a flatter more subtle style would give the
> left and right side more visual consistency. Using a lighter color scheme would
> also be more friendly to a wider range of favicons.

That attached screenshot is such an improvement, Shorlander!  Wow!  It also  puts us in a good position to make future improvements to the URL bar and site identity button.  Shall we patch it?

	:Margaret Leibovic Assignee
	Comment 6 • 13 years ago

I'm working on this! Patch almost done for winstripe.

	:Margaret Leibovic Assignee
	Comment 7 • 13 years ago

I just realized that this arrow approach is not completely correct because the urlbar is not white when it is not hovered/focused, so we would want the left side of the arrow to reflect that as well. I'm not sure of the best way to do that off the top of my head.

Also, I'm not sure what we should do about the hover/active styles for the identity box. It feels like keeping the current gradient styles makes the box look dirty for the verified states. Maybe we should just lighten it up?

	:Margaret Leibovic Assignee
	Comment 8 • 13 years ago

Oops, forgot to add arrow image.

	Jennifer Morrow [:Boriss] (UX)
	Comment 9 • 13 years ago

(In reply to comment #8)
> Created attachment 514942 [details] [diff] [review]
> wip winstripe patch
> 
> Oops, forgot to add arrow image.

This looks fantastic, with dialogs left and without.  As Margaret pointed out to me, for now she's using a graphic for the arrow that isn't transparent, so there's a slight mismatch on the arrow when the left icons are hovered over between the white background of the arrow and the slightly blue mouseover color.  It's very slight, though, and a partially transparent image should take care of it.

	Frank Yan (:fryn)
	Comment 10 • 13 years ago

(In reply to comment #9)
> a partially transparent image should take care of it.

Actually, we'd have to add a transparent-to-white left-to-right gradient to the left of the arrow image, since with the current technique, the arrow needs to be opaque to cover the identity box.

A more robust solution would be to apply an svg clip (or mask) to the identity box, but that would require more complex code.

(In reply to comment #8)
> Created attachment 514942 [details] [diff] [review]
> wip winstripe patch

> #identity-box {
...
> -  border-radius: 2px;
> +  background-color: #ededed;
> +  border-top-left-radius: 2px;
> +  border-bottom-left-radius: 2px;

>  #notification-popup-box {
...
> +  background: url("chrome://browser/skin/urlbar-arrow.png") no-repeat right;

This will break in RTL.

	Frank Yan (:fryn)
	Comment 11 • 13 years ago

Comment on attachment 514942 [details] [diff] [review]
wip winstripe patch

I just realized that the background of the notification popup region can just be white. The current behavior of that region changing color (on Windows) when the url bar gets focused is odd, since conceptually it's not part of the editable input box. By making the notification popup region opaque, we eliminate this odd behavior and the transparent/white problem.

Overall, the appearance is already spectacular. :)

	:Margaret Leibovic Assignee
	Comment 12 • 13 years ago

Comment on attachment 514942 [details] [diff] [review]
wip winstripe patch

I have a new patch in the works that uses an SVG mask instead of an image. So far, it seems like a more elegant approach.

	:Margaret Leibovic Assignee
	Comment 13 • 13 years ago

I decided to use SVG to make a background image for #notification-popup-box. I have a different version of the patch that applies an SVG mask to #identity-box, but that isn't very helpful because it doesn't give us a border along the edge of the arrow. 

I added a 4px gradient to the edge of the arrow on Windows, and it makes the background color for the urlbar pretty much impossible to notice, event at 8x zoom.

I like using SVG for the triangle because it will scale well for different font sizes, and it looks sharper than the image I used in my first patch. However, I'll run this through try, and if the SVG causes performance regressions, it will be easy to swap in an image (a better quality one than the one I was using before).

I also still need the correct color values from Stephen.

	Frank Yan (:fryn)
	Comment 14 • 13 years ago

Comment on attachment 515801 [details] [diff] [review]
patch

I tested it on Windows and OS X, in both LTR and RTL mode, and it looks good.
The only exception is that the popup-notification-icons need opaque backgrounds so that the non-white urlbar doesn't show through.
A skim of the code yielded no obvious issues. :)
I wonder if we could pull a |-moz-transform: scaleX(-1)| trick on the #notification-popup-box for the arrow instead of using two separate images. I think that would be a lot easier.

	Frank Yan (:fryn)
	Comment 15 • 13 years ago

Using |-moz-transform: scaleX(-1)| also means that we'd use -left and -right instead of -start- and -end.

And btw, it looks awesome!

	John Ford [:jhford] CET/CEST Berlin Time Reporter
	Comment 16 • 13 years ago

This looks awesome!

	Frank Yan (:fryn)
	Comment 17 • 13 years ago

Margaret, it looks like the vertical alignment of the text in the identity-box doesn't match that of the location bar and search bar's text. On Windows and OS X, it's higher, and on Gnome, it's lower, but we already have this problem, so it's not a huge problem, if it remains that way.

	Alex Limi (:limi) — Firefox UX Team
	Comment 18 • 13 years ago

Renaming the bug, since it needs a more descriptive title (especially during these times of rapid triage and approvals).

	Markus Stange [:mstange]
	Comment 19 • 13 years ago

Comment on attachment 515801 [details] [diff] [review]
patch

>diff --git a/browser/themes/pinstripe/browser/browser.css b/browser/themes/pinstripe/browser/browser.css
>--- a/browser/themes/pinstripe/browser/browser.css
>+++ b/browser/themes/pinstripe/browser/browser.css
>@@ -774,17 +774,16 @@ toolbar[mode="icons"] #zoom-in-button {
> /* ::::: nav-bar-inner ::::: */
> 
> #urlbar,
> .searchbar-textbox {
>   font: icon;
>   width: 7em;
>   min-width: 7em;
>   -moz-appearance: none;
>-  box-shadow: 0 1px rgba(255, 255, 255, 0.2), inset 0 1px #d6d6d6;

Why this change? I think at least the outer shadow should stay.

	Peter Henkel [:Terepin]
	Comment 20 • 13 years ago

This is intended for Firefox 5?

I must say it looks awesome!

	Peter Henkel [:Terepin]
	Comment 21 • 13 years ago

One question: What's the point of keeping favicon in both places? I thought we are trying to get rid of duplication.

	:Margaret Leibovic Assignee
	Comment 22 • 13 years ago

(In reply to comment #19)
> Comment on attachment 515801 [details] [diff] [review]
> patch
> 
> >diff --git a/browser/themes/pinstripe/browser/browser.css b/browser/themes/pinstripe/browser/browser.css
> >--- a/browser/themes/pinstripe/browser/browser.css
> >+++ b/browser/themes/pinstripe/browser/browser.css
> >@@ -774,17 +774,16 @@ toolbar[mode="icons"] #zoom-in-button {
> > /* ::::: nav-bar-inner ::::: */
> > 
> > #urlbar,
> > .searchbar-textbox {
> >   font: icon;
> >   width: 7em;
> >   min-width: 7em;
> >   -moz-appearance: none;
> >-  box-shadow: 0 1px rgba(255, 255, 255, 0.2), inset 0 1px #d6d6d6;
> 
> Why this change? I think at least the outer shadow should stay.

I was trying to match Stephen's mock-up, but I guess I didn't think about the differences we want between OSX and Windows. The inner shadow interacts badly with the button, but I agree that the outer shadow could stay.

	u310142
	Comment 23 • 13 years ago

I find the SSL and EV states hard to see in that mockup. The SSL block background almost matches the nav toolbar background and the EV background is almost white.

The font color in those states also doesn't seem enough to help notice them.

	Frank Yan (:fryn)
	Comment 24 • 13 years ago

Margaret, by adding this to your patch, the favicon will always be the drag image (when dragging the identity block), instead of the entire identity block or various parts thereof.

Feel free to obsolete this upon incorporation. :)

	:Margaret Leibovic Assignee
	Comment 25 • 13 years ago

I updated my patch to address comments. The one problem I'm seeing is that the popup notification panel arrow points to the wrong place in RTL mode because of the -moz-transform style on #notification-popup-box. It seems that the panel positioning is being computed before the transformation is applied, and because there is unsymmetrical padding, the panel becomes misaligned.

I feel like this is something that can be fixed in a separate bug and should not keep us from using this approach, but I could be wrong about that.
	
Dão, I don't want to ask for review until we sort out the RTL issue, but I would appreciate any comments you might have for me so far!

	:Margaret Leibovic Assignee
	Comment 26 • 13 years ago

Neil, I think applying -moz-transform: scaleX(-1); to the arrow panel's parent node is causing the arrow to point to the wrong place (see above comment). Do you know if there is a way to work around this issue, or if is this a bug you could fix?

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

Comment on attachment 519803 [details] [diff] [review]
updated patch

CSS transforms don't affect layout, that's expected at this stage. You shouldn't use them here anyway, as we don't want to mirror all icons for RTL.

	:Margaret Leibovic Assignee
	Comment 28 • 13 years ago

(In reply to comment #27)
> CSS transforms don't affect layout, that's expected at this stage. You
> shouldn't use them here anyway, as we don't want to mirror all icons for RTL.

For the icons, Frank suggested that we could flip the icons back with another scaleX(-1). However, I don't see a solution to the layout issue if that's expected behavior. I was trying to avoid using two images, but do you think we should just do that? Or is there something else we should do?

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

> I was trying to avoid using two images, but do you think we
> should just do that?

yes

	:Margaret Leibovic Assignee
	Comment 30 • 13 years ago

I still need to get better image files from Stepehen to replace my placeholder images.

Also, we need to think of how to best fix the background color problem on Windows that was brought up in comment 7 and comment 11. Stephen said he wanted to think about if there's a better way to do it than adding a transparent gradient like I talked about in comment 13.

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

(In reply to comment #30)
> Created attachment 520071 [details] [diff] [review]
> patch using -moz-image-rect for different arrow images

I don't see the point of -moz-image-rect here, these can be just two images.

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

Comment on attachment 520314 [details] [diff] [review]
patch with two images

>--- a/browser/base/content/browser.js
>+++ b/browser/base/content/browser.js
>@@ -7800,16 +7800,17 @@ var gIdentityHandler = {
>     var htmlString = "<a href=\"" + value + "\">" + value + "</a>";
> 
>     var dt = event.dataTransfer;
>     dt.setData("text/x-moz-url", urlString);
>     dt.setData("text/uri-list", value);
>     dt.setData("text/plain", value);
>     dt.setData("text/html", htmlString);
>     dt.addElement(event.currentTarget);
>+    dt.setDragImage(gProxyFavIcon, 0, 0);

It makes no sense to call both addElement and setDragImage, and with different nodes.

I tested this patch only on Windows.

* It looks like the identity block doesn't cleanly connect with the nav bar's corners, making them look slightly washed-out.

* I agree with comment 23, the color differences are too faint to adequately communicate what the identity button was originally supposed to communicate. If we're okay with that, we might as well stop changing the colors altogether. I suspect we're not okay with it, though.

* The non-ssl styling feels a bit heaver than without this patch due to the extra border. I'm not sure that's a good tradeoff.

* The ssl styling feels in a way heaver than without this patch due to the bold text. The mockup tricked by making the text not only bold but also smaller (but too small to meet a11y requirements, imho).

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

> I tested this patch only on Windows.

XP without cleartype, btw, which might be relevant for the perception of the bold text.

	Frank Yan (:fryn)
	Comment 35 • 13 years ago

(In reply to comment #33)

> >     dt.addElement(event.currentTarget);
> >+    dt.setDragImage(gProxyFavIcon, 0, 0);
> 
> It makes no sense to call both addElement and setDragImage, and with different
> nodes.

Sometimes, it does make sense, since addElement establishes the node on which events will be fired, and you might want a different drag image. I wasn't sure if we were listening for any other events that might depend on event.currentTarget, but it looks like we're not, so in this case, we can just use:

> >-    dt.addElement(event.currentTarget);
> >+    dt.addElement(gProxyFavIcon);

I only took a quick glance on possible dependencies, so please test this change before committing, Margaret. :)

	:Margaret Leibovic Assignee
	Comment 36 • 13 years ago

(In reply to comment #35)

> > >-    dt.addElement(event.currentTarget);
> > >+    dt.addElement(gProxyFavIcon);
> 
> I only took a quick glance on possible dependencies, so please test this change
> before committing, Margaret. :)

This looks pretty strange if you initiate the drag by clicking on the text part of the identity box, since it keeps the favicon the same distance from the cursor. I think we need to go with the first approach if we want to do this. However, if this is a contentious change, perhaps we should move it to a different bug so as not to hold up the style changes in this bug.

	Frank Yan (:fryn)
	Comment 37 • 13 years ago

(In reply to comment #36)

Ugh, I see why that happens.

Let's go with:

-    dt.addElement(event.currentTarget);
+    dt.setDragImage(gProxyFavIcon, 0, 0);

I don't think we need to defer this to another bug. It's not contentious from a UI perspective (I've also gotten UX team verbal approval), and we have a solution (the above two lines).

	:Margaret Leibovic Assignee
	Comment 38 • 13 years ago

I increased the border-radius on Windows and Linux to improve the appearance of the corners, and Stephen came up with improved color/text styles. He also made much nicer arrow images!

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

Comment on attachment 523183 [details] [diff] [review]
patch

>--- a/browser/themes/winstripe/browser/browser.css
>+++ b/browser/themes/winstripe/browser/browser.css

> #identity-box {
>+  margin: -2px;
>+  -moz-margin-end: 0;
> }

Instead of this, can you remove the textbox's padding?

> #identity-box.verifiedDomain,
> #identity-box.verifiedIdentity {
>+  -moz-padding-end: 2px;
> }

This is wrong, try setting browser.identity.ssl_domain_display to 0. The spacing needs to be adjusted here:

> #identity-icon-labels {
>   -moz-margin-start: 1px;
>   -moz-margin-end: 3px;
>-  -moz-transform: translate(0, -1px);
> }

I still haven't tested Linux/Mac.

	:Margaret Leibovic Assignee
	Comment 40 • 13 years ago

(In reply to comment #39)
> Comment on attachment 523183 [details] [diff] [review]
> patch
> 
> >--- a/browser/themes/winstripe/browser/browser.css
> >+++ b/browser/themes/winstripe/browser/browser.css
> 
> > #identity-box {
> >+  margin: -2px;
> >+  -moz-margin-end: 0;
> > }
> 
> Instead of this, can you remove the textbox's padding?

I based this off the way the combined stop/go/reload button was styled. To get rid of the padding on the textbox and keep the location bar looking right, I would need to also need to modify that button's styles.

	:Margaret Leibovic Assignee
	Comment 41 • 13 years ago

On Windows, I removed the #urlbar padding, so I could also get rid of the negative margin on #urlbar > toolbarbutton, since it's not needed anymore. Setting padding on #identity-box to maintain the height of the identity box also took care of the spacing issue from the second review comment. I also had to tweak the #notification-popup-box spacing to cope with getting rid of the #urlbar padding.

On Linux, because we don't style the textbox ourselves (bug 595973), I couldn't get rid of the padding, so I kept the negative margins there. Talking with Stephen, we decided that there should be a consistent negative margin style between the identity box and the stop/go/reload button, so we decided change the negative margin on the button to also be -1px, since that seems to create the right spacing for most themes (right now the stop/go/reload button overlaps the urlbar border on a bunch of themes).

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

Comment on attachment 523404 [details] [diff] [review]
patch v2

The :-moz-focusring styling needs an update, at least on Windows and Linux. The color could probably be just black all the time...

	:Margaret Leibovic Assignee
	Comment 43 • 13 years ago

I changed the -moz-focusring style for Windows so that it is always black. Also, after talking with Stephen I decided to remove the outline-offset. It looks like I didn't need to make any changes to the Linux styles.

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

Comment on attachment 524500 [details] [diff] [review]
patch v3

Removing the offset is a problem, as this paints the outline on the textbox border, where it likely won't be visible (this applies to Linux as well, since the button now touches the border).

	:Margaret Leibovic Assignee
	Comment 45 • 13 years ago

Added outline-offset to -moz-focusring style for Windows and Linux.

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

Comment on attachment 524638 [details] [diff] [review]
patch v4

>--- a/browser/base/content/browser.js
>+++ b/browser/base/content/browser.js
>@@ -7787,17 +7787,17 @@ var gIdentityHandler = {
>     var urlString = value + "\n" + content.document.title;
>     var htmlString = "<a href=\"" + value + "\">" + value + "</a>";
> 
>     var dt = event.dataTransfer;
>     dt.setData("text/x-moz-url", urlString);
>     dt.setData("text/uri-list", value);
>     dt.setData("text/plain", value);
>     dt.setData("text/html", htmlString);
>-    dt.addElement(event.currentTarget);
>+    dt.setDragImage(gProxyFavIcon, 0, 0);

This is kind of broken, the favicon is mostly covered by the cursor over here.

>--- a/browser/themes/gnomestripe/browser/browser.css
>+++ b/browser/themes/gnomestripe/browser/browser.css

> #identity-box:-moz-focusring {
>   outline: 1px dotted -moz-DialogText;
>+  outline-offset: -3px;
> }

>--- a/browser/themes/winstripe/browser/browser.css
>+++ b/browser/themes/winstripe/browser/browser.css

> #identity-box:-moz-focusring {
>   outline: 1px dotted -moz-DialogText;
>   outline-offset: -3px;
> }

The background colors are hardcoded, so the outline color should be 'black'.

	:Margaret Leibovic Assignee
	Comment 47 • 13 years ago

(In reply to comment #46)
> Comment on attachment 524638 [details] [diff] [review]
> patch v4
> 
> >--- a/browser/base/content/browser.js
> >+++ b/browser/base/content/browser.js
> >@@ -7787,17 +7787,17 @@ var gIdentityHandler = {
> >     var urlString = value + "\n" + content.document.title;
> >     var htmlString = "<a href=\"" + value + "\">" + value + "</a>";
> > 
> >     var dt = event.dataTransfer;
> >     dt.setData("text/x-moz-url", urlString);
> >     dt.setData("text/uri-list", value);
> >     dt.setData("text/plain", value);
> >     dt.setData("text/html", htmlString);
> >-    dt.addElement(event.currentTarget);
> >+    dt.setDragImage(gProxyFavIcon, 0, 0);
> 
> This is kind of broken, the favicon is mostly covered by the cursor over here.

Using dt.setDragImage(gProxyFavIcon, 10, 10); seems to do a good job placing the tip on the cursor in the middle of the icon. Does that sound good? (I only tested on Mac)

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

(In reply to comment #47)
> Using dt.setDragImage(gProxyFavIcon, 10, 10); seems to do a good job placing
> the tip on the cursor in the middle of the icon. Does that sound good? (I only
> tested on Mac)

Better, but whether that thing under the mouse can be recognized as the favicon still seems to depend on the icon and cursor being used. How does 16 instead of 10 work for you?

	Frank Yan (:fryn)
	Comment 49 • 13 years ago

(In reply to comment #47)

FWIW, I concur with 16, since it's the width/length of the favicon and for the same reason that Dao gave.

	:Margaret Leibovic Assignee
	Comment 50 • 13 years ago

I tried using 16, but it made the icon float away from the cursor, which looked a little strange to me. However, testing with more favicons, I think 16 is better because you can definitely see the favicon more clearly.

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

Comment on attachment 524638 [details] [diff] [review]
patch v4

> /* Notification icon box */
> #notification-popup-box {
>-  margin: 0 3px;
>+  -moz-margin-end: -8px;
>+  -moz-margin-start: 5px;
>+  -moz-padding-end: 10px;
>+  background-repeat: no-repeat;
>+  background-size: auto 100%;
>+  position: relative;
>+}
>+
>+#notification-popup-box:-moz-locale-dir(ltr) {
>+  background-image: url("chrome://browser/skin/urlbar-arrow.png");
>+  background-position: right;
>+}
>+
>+#notification-popup-box:-moz-locale-dir(rtl) {
>+  background-image: url("chrome://browser/skin/urlbar-arrow-rtl.png");
>+  background-position: left;
>+}

This is broken when using a lightweight theme or dark OS theme.

	:Margaret Leibovic Assignee
	Comment 52 • 13 years ago

To fix the theme issue, I decided to make the #notification-popup-box background always be white, since we're already using hard-coded colors for the identity block background. Now that the identity box touches the top and bottom borders of the urlbar, the notification popup box no longer looks like part of the same input box, so it does not necessarily need the same background color. Frank also points out that we don't invert the colors of the notification icons in different themes, so this change actually makes those more consistently visible.

To get this to work I had to change my implementation of the arrow, but it works out nicely because using ::after lets us use -moz-transform: scaleX(-1) on the pseudo-element so we only need one image.

I tested this in RTL mode on Windows/OSX/Linux and with different OS themes on Windows/Linux, so hopefully it's almost ready to go!

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

Comment on attachment 525208 [details] [diff] [review]
patch v5

>+#notification-popup-box::after {
>+  content: "";
>+  display: -moz-box;
>+  -moz-margin-end: -8px;
>+  width: 10px;
>+  height: 22px;
>+  background-image: url("chrome://browser/skin/urlbar-arrow.png");
>+  background-position: right;
>+  background-repeat: no-repeat;
>+}

This doesn't stretch anymore when increasing the font size. The previous patch used background-size: auto 100% for this.

	Frank Yan (:fryn)
	Comment 54 • 13 years ago

Margaret asked me for ideas on how to fix the scaling issue, since I was the one who suggested using background-size: auto 100% and later ::after.

Since ::after doesn't seem to behave nicely with height and -moz-box-align: stretch, I just went with -moz-border-image instead.

Leaving Margaret as the assignee. :)

	Siddhartha Dugar [:sdrocking]
	Comment 55 • 13 years ago

The tryserver build for Bug 455694 probably contains a patch from this bug. I'd like the identity block to be darkly colored on secure sites.

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

Comment on attachment 525823 [details] [diff] [review]
patch v6

>--- a/browser/themes/winstripe/browser/browser.css
>+++ b/browser/themes/winstripe/browser/browser.css

>+.searchbar-textbox {
>+  padding: 2px;
>+}
>+
> #urlbar,
> .searchbar-textbox {
>   -moz-appearance: none;
>   margin: 1px 3px;
>-  padding: 2px;
>   background-clip: padding-box;
>   border: 1px solid ThreeDDarkShadow;
>   border-radius: 3.5px;
> }

This lets the url and search bars consume different heights with increased font sizes. You need to either remove the padding from the search bar or undo it for the url bar.

	:Margaret Leibovic Assignee
	Comment 57 • 13 years ago

I decided to leave the padding as-is and use negative margins for the identity-box and notification-popup-box like we do on the stop/go/reload button.

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

Comment on attachment 527036 [details] [diff] [review]
patch v7

>--- a/browser/themes/gnomestripe/browser/browser.css
>+++ b/browser/themes/gnomestripe/browser/browser.css

>+#notification-popup-box:not([hidden]) + #identity-box {
>+  -moz-padding-start: 10px;

The previous patch had 6px here, please explain this change.

	:Margaret Leibovic Assignee
	Comment 59 • 13 years ago

(In reply to comment #58)
> Comment on attachment 527036 [details] [diff] [review]
> patch v7
> 
> >--- a/browser/themes/gnomestripe/browser/browser.css
> >+++ b/browser/themes/gnomestripe/browser/browser.css
> 
> >+#notification-popup-box:not([hidden]) + #identity-box {
> >+  -moz-padding-start: 10px;
> 
> The previous patch had 6px here, please explain this change.

When testing, the arrow jutted right against the favicon, so I increased the padding to make it look better. I'm not sure why winstripe/gnomestripe had 6px, because pinstripe had 10px. It was likely left over from previous iterations of different approaches.

	:Margaret Leibovic Assignee
	Comment 60 • 13 years ago

Thanks for all the review work, Dão!

http://hg.mozilla.org/mozilla-central/rev/aebc3d41381a

	Chris B.
	Comment 61 • 13 years ago

(In reply to comment #60)
> Thanks for all the review work, Dão!
> 
> http://hg.mozilla.org/mozilla-central/rev/aebc3d41381a

Is there something not set right with this patch?

This bug landed in today's nightly (4/20/11).  HOWEVER, attachment 513594 [details] "Extended Site Identity Button" shows the text in the Identity Info box for "SSL State and Notification" to be in BOLD, but the current nightly has just regular unformatted text.

	:Margaret Leibovic Assignee
	Comment 62 • 13 years ago

(In reply to comment #61)
> Is there something not set right with this patch?
> 
> This bug landed in today's nightly (4/20/11).  HOWEVER, attachment 513594 [details]
> "Extended Site Identity Button" shows the text in the Identity Info box for
> "SSL State and Notification" to be in BOLD, but the current nightly has just
> regular unformatted text.

That is expected behavior. We iterated on the design after a first pass at implementing those mock-ups.

	David Hsu
	Comment 63 • 13 years ago

And the "pressed" state for indicators? Were they for another bug? Or discarded too?

	:Margaret Leibovic Assignee
	Comment 64 • 13 years ago

(In reply to comment #63)
> And the "pressed" state for indicators? Were they for another bug? Or discarded
> too?

There is a pressed state. If you're not seeing that you should file a new bug :)

	Frank Yan (:fryn)
	Comment 65 • 13 years ago

(In reply to comment #64)
> (In reply to comment #63)
> > And the "pressed" state for indicators? Were they for another bug? Or discarded
> > too?
> 
> There is a pressed state. If you're not seeing that you should file a new bug
> :)

I don't see it for _indicators_ either, and I don't recall there being code or any intention for it.