Closed
Bug 634065
Opened 14 years ago
Closed 14 years ago
Implement design for identity block and persistent indicators
Categories
(Firefox :: Theme, defect)
Firefox
Theme
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)
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.
Comment 1•14 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.
Updated•14 years ago
|
blocking2.0: --- → ?
Comment 2•14 years ago
|
||
This doesn't block release without an agreed-upon UX design fix. Renominate once that's worked through, if you'd like.
blocking2.0: ? → -
Comment 3•14 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?
Comment 4•14 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.
Comment 5•14 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?
Assignee | ||
Comment 6•14 years ago
|
||
I'm working on this! Patch almost done for winstripe.
Assignee: nobody → margaret.leibovic
Assignee | ||
Comment 7•14 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?
Attachment #514941 -
Flags: feedback?(shorlander)
Attachment #514941 -
Flags: feedback?(fryn)
Assignee | ||
Comment 8•14 years ago
|
||
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)
Comment 9•14 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.
Comment 10•14 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.
Comment 11•14 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. :)
Attachment #514942 -
Flags: feedback?(fryn)
Assignee | ||
Comment 12•14 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.
Attachment #514942 -
Attachment is obsolete: true
Attachment #514942 -
Flags: feedback?(shorlander)
Assignee | ||
Comment 13•14 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.
Attachment #515801 -
Flags: feedback?(fryn)
Comment 14•14 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.
Attachment #515801 -
Flags: feedback?(fryn) → feedback+
Comment 15•14 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!
Reporter | ||
Comment 16•14 years ago
|
||
This looks awesome!
Comment 17•14 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.
Comment 18•14 years ago
|
||
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 19•14 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.
Comment 20•14 years ago
|
||
This is intended for Firefox 5?
I must say it looks awesome!
Comment 21•14 years ago
|
||
One question: What's the point of keeping favicon in both places? I thought we are trying to get rid of duplication.
Assignee | ||
Comment 22•14 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.
Comment 23•14 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.
Comment 24•14 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. :)
Updated•14 years ago
|
Status: NEW → ASSIGNED
Version: unspecified → Trunk
Assignee | ||
Comment 25•14 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!
Attachment #515801 -
Attachment is obsolete: true
Attachment #518998 -
Attachment is obsolete: true
Attachment #519803 -
Flags: feedback?(dao)
Assignee | ||
Comment 26•14 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?
Comment 27•14 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.
Attachment #519803 -
Flags: feedback?(dao) → feedback-
Assignee | ||
Comment 28•14 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?
Comment 29•14 years ago
|
||
> I was trying to avoid using two images, but do you think we
> should just do that?
yes
Assignee | ||
Comment 30•14 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.
Attachment #519803 -
Attachment is obsolete: true
Attachment #520071 -
Flags: feedback?(dao)
Comment 31•14 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.
Assignee | ||
Comment 32•14 years ago
|
||
Attachment #520071 -
Attachment is obsolete: true
Attachment #520314 -
Flags: feedback?(dao)
Attachment #520071 -
Flags: feedback?(dao)
Comment 33•14 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).
Attachment #520314 -
Flags: feedback?(dao) → feedback-
Comment 34•14 years ago
|
||
> I tested this patch only on Windows.
XP without cleartype, btw, which might be relevant for the perception of the bold text.
Comment 35•14 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. :)
Assignee | ||
Comment 36•14 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.
Comment 37•14 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).
Assignee | ||
Comment 38•14 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!
Attachment #520314 -
Attachment is obsolete: true
Attachment #523183 -
Flags: review?(dao)
Comment 39•14 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.
Attachment #523183 -
Flags: review?(dao) → review-
Assignee | ||
Comment 40•14 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.
Assignee | ||
Comment 41•14 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).
Attachment #523183 -
Attachment is obsolete: true
Attachment #523404 -
Flags: review?(dao)
Comment 42•14 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...
Attachment #523404 -
Flags: review?(dao) → review-
Assignee | ||
Comment 43•14 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.
Attachment #523404 -
Attachment is obsolete: true
Attachment #524500 -
Flags: review?(dao)
Comment 44•14 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).
Attachment #524500 -
Flags: review?(dao) → review-
Assignee | ||
Comment 45•14 years ago
|
||
Added outline-offset to -moz-focusring style for Windows and Linux.
Attachment #524500 -
Attachment is obsolete: true
Attachment #524638 -
Flags: review?(dao)
Comment 46•14 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'.
Assignee | ||
Comment 47•14 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)
Comment 48•14 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?
Comment 49•14 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.
Assignee | ||
Comment 50•14 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.
Comment 51•14 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.
Attachment #524638 -
Flags: review?(dao) → review-
Assignee | ||
Comment 52•14 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!
Attachment #524638 -
Attachment is obsolete: true
Attachment #525208 -
Flags: review?(dao)
Comment 53•14 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.
Attachment #525208 -
Flags: review?(dao) → review-
Comment 54•14 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. :)
Attachment #525208 -
Attachment is obsolete: true
Attachment #525823 -
Flags: review?(dao)
Comment 55•14 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.
Comment 56•14 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.
Attachment #525823 -
Flags: review?(dao) → review-
Assignee | ||
Comment 57•14 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.
Attachment #525823 -
Attachment is obsolete: true
Attachment #527036 -
Flags: review?(dao)
Comment 58•14 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.
Assignee | ||
Comment 59•14 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.
Updated•14 years ago
|
Attachment #527036 -
Flags: review?(dao) → review+
Assignee | ||
Comment 60•14 years ago
|
||
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
Comment 61•14 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.
Assignee | ||
Comment 62•14 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.
Comment 63•14 years ago
|
||
And the "pressed" state for indicators? Were they for another bug? Or discarded too?
Assignee | ||
Comment 64•14 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 :)
Comment 65•14 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.
You need to log in
before you can comment on or make changes to this bug.
Description
•