Closed Bug 634065 Opened 14 years ago Closed 14 years ago

Implement design for identity block and persistent indicators

Categories

(Firefox :: Theme, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 6
Tracking Status
blocking2.0 --- -

People

(Reporter: jhford, Assigned: Margaret)

References

(Depends on 1 open bug)

Details

Attachments

(4 files, 13 obsolete files)

4.57 KB, image/png
Details
514.66 KB, image/jpeg
Details
5.68 KB, image/png
Details
24.21 KB, patch
dao
: review+
Details | Diff | Splinter Review
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.
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.
blocking2.0: --- → ?
This doesn't block release without an agreed-upon UX design fix. Renominate once that's worked through, if you'd like.
blocking2.0: ? → -
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?
(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.
(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?
I'm working on this! Patch almost done for winstripe.
Assignee: nobody → margaret.leibovic
Attached patch wip winstripe patch (obsolete) — Splinter Review
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?
Attachment #514941 - Flags: feedback?(shorlander)
Attachment #514941 - Flags: feedback?(fryn)
Attached patch wip winstripe patch (obsolete) — Splinter Review
Oops, forgot to add arrow image.
Attachment #514941 - Attachment is obsolete: true
Attachment #514942 - Flags: feedback?(shorlander)
Attachment #514942 - Flags: feedback?(fryn)
Attachment #514941 - Flags: feedback?(shorlander)
Attachment #514941 - Flags: feedback?(fryn)
(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.
(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.
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. :)
Attachment #514942 - Flags: feedback?(fryn)
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.
Attachment #514942 - Attachment is obsolete: true
Attachment #514942 - Flags: feedback?(shorlander)
Attached patch patch (obsolete) — Splinter Review
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.
Attachment #515801 - Flags: feedback?(fryn)
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.
Attachment #515801 - Flags: feedback?(fryn) → feedback+
Using |-moz-transform: scaleX(-1)| also means that we'd use -left and -right instead of -start- and -end. And btw, it looks awesome!
This looks awesome!
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.
Renaming the bug, since it needs a more descriptive title (especially during these times of rapid triage and approvals).
Summary: left side of awesomebar is visually inconsistent with right side → Implement design for identity block and persistent indicators
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.
This is intended for Firefox 5? I must say it looks awesome!
One question: What's the point of keeping favicon in both places? I thought we are trying to get rid of duplication.
(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.
Blocks: 579521
Blocks: 639155
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.
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. :)
Status: NEW → ASSIGNED
Version: unspecified → Trunk
Depends on: 641892
Attached patch updated patch (obsolete) — Splinter Review
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!
Attachment #515801 - Attachment is obsolete: true
Attachment #518998 - Attachment is obsolete: true
Attachment #519803 - Flags: feedback?(dao)
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?
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.
Attachment #519803 - Flags: feedback?(dao) → feedback-
(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?
> I was trying to avoid using two images, but do you think we > should just do that? yes
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.
Attachment #519803 - Attachment is obsolete: true
Attachment #520071 - Flags: feedback?(dao)
(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.
Attached patch patch with two images (obsolete) — Splinter Review
Attachment #520071 - Attachment is obsolete: true
Attachment #520314 - Flags: feedback?(dao)
Attachment #520071 - Flags: feedback?(dao)
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).
Attachment #520314 - Flags: feedback?(dao) → feedback-
> I tested this patch only on Windows. XP without cleartype, btw, which might be relevant for the perception of the bold text.
(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. :)
(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.
(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).
Attached patch patch (obsolete) — Splinter Review
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!
Attachment #520314 - Attachment is obsolete: true
Attachment #523183 - Flags: review?(dao)
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.
Attachment #523183 - Flags: review?(dao) → review-
(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.
Attached patch patch v2 (obsolete) — Splinter Review
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).
Attachment #523183 - Attachment is obsolete: true
Attachment #523404 - Flags: review?(dao)
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...
Attachment #523404 - Flags: review?(dao) → review-
Attached patch patch v3 (obsolete) — Splinter Review
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.
Attachment #523404 - Attachment is obsolete: true
Attachment #524500 - Flags: review?(dao)
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).
Attachment #524500 - Flags: review?(dao) → review-
Attached patch patch v4 (obsolete) — Splinter Review
Added outline-offset to -moz-focusring style for Windows and Linux.
Attachment #524500 - Attachment is obsolete: true
Attachment #524638 - Flags: review?(dao)
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'.
(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)
(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?
(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.
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.
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.
Attachment #524638 - Flags: review?(dao) → review-
Attached patch patch v5 (obsolete) — Splinter Review
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!
Attachment #524638 - Attachment is obsolete: true
Attachment #525208 - Flags: review?(dao)
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.
Attachment #525208 - Flags: review?(dao) → review-
Attached patch patch v6 (obsolete) — Splinter Review
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. :)
Attachment #525208 - Attachment is obsolete: true
Attachment #525823 - Flags: review?(dao)
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.
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.
Attachment #525823 - Flags: review?(dao) → review-
Attached patch patch v7Splinter Review
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.
Attachment #525823 - Attachment is obsolete: true
Attachment #527036 - Flags: review?(dao)
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.
(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.
Attachment #527036 - Flags: review?(dao) → review+
Thanks for all the review work, Dão! http://hg.mozilla.org/mozilla-central/rev/aebc3d41381a
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 6
Depends on: 651326
(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.
(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.
And the "pressed" state for indicators? Were they for another bug? Or discarded too?
(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 :)
(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.
Depends on: 656889
No longer depends on: 656889
Depends on: 680811
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: