Closed Bug 423718 Opened 12 years ago Closed 12 years ago

Use native platform colors for URLs in the location bar autocomplete, make the line between rows lighter

Categories

(Firefox :: Theme, defect, P2)

defect

Tracking

()

RESOLVED FIXED
Firefox 3

People

(Reporter: faaborg, Assigned: Mardak)

References

Details

(Keywords: polish, Whiteboard: [better with bug 426732])

Attachments

(20 files, 4 obsolete files)

19.82 KB, image/png
Details
22.54 KB, image/png
Details
16.88 KB, image/png
Details
78.06 KB, image/png
Details
3.07 KB, image/gif
Details
22.65 KB, image/png
Details
6.80 KB, image/gif
Details
2.73 KB, image/gif
Details
6.31 KB, image/gif
Details
6.38 KB, image/gif
Details
25.10 KB, image/gif
Details
4.14 KB, image/gif
Details
148.96 KB, image/png
Details
87.68 KB, image/png
Details
18.18 KB, image/png
Details
18.18 KB, image/png
Details
39.05 KB, image/png
Details
21.05 KB, image/png
Details
2.04 KB, patch
beltzner
: review-
beltzner
: ui-review+
Details | Diff | Splinter Review
3.43 KB, patch
beltzner
: review+
beltzner
: superreview+
beltzner
: approval1.9+
Details | Diff | Splinter Review
Here are the colors we should use on each platform for URLs in the location bar autocomplete:

OS X: (35,77,177) or #234db1
Vista: (0,102,204) or #0066cc
XP: (0,153,0) or #009900

For the line between items,  on OS X I think the standard menu line is probably what we want to go with, at (228,228,228) or #e4e4e4

On windows we should pull a system color that we know is light in the default themes.  I think ThreeDFace works the best.

I need input on what color to use on Linux, perhaps something out of the Tango color palette for the URL (http://tango.freedesktop.org/static/cvs/tango-art-tools/palettes/Tango-Palette.png) like the darkest Chameleon, the the middle sky blue?  We could try ThreeDFace for the line to see if that looks ok.
Flags: blocking-firefox3?
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P2
Attached patch v1 (obsolete) — Splinter Review
A quick stab with the suggested colors.. hopefully I got the right jar.mn stuff. I've got a build going on tryserver.

Also, this patch is way deep in my stack, so the browser.css I copy/pasted has other stuff like keyword search styling, showing tags, and smaller title text.
Assignee: nobody → edilee
Status: NEW → ASSIGNED
Attached image screenshot of trunk mac
Attached image screenshot of v1 mac
Attached image OS X url color
This time trying to pull it off of an underline, let's try #144fae
Attached image xp url color (help)
It's really hard to capture colors of text on os x, here is the underline when using #144fae.  Anyway, I think this is close enough.
Attachment #310657 - Attachment description: xp trunk, 009900 Face, 006600 LightShadow → xp trunk 336633 Shadow, 009900 Face, 006600 LightShadow
>006600 LightShadow

This one works well, what do you think of ThreeDFace vs LightShadow?
Attached image 235,240,245,250,255
Related to bug 409974.. if we use the default background color instead of forcing our own (white?) to go with the native url colors, it could be hard to read.
Attached patch v1.1 (obsolete) — Splinter Review
os x: 144fae (attachment 310630 [details])
linux: 4e9a06 (dark chameleon)
xp: 006600 (attachment 310657 [details])
vista: 0066cc

border for all: ThreeDLightShadow
Attachment #310612 - Attachment is obsolete: true
(In reply to comment #15)
> forcing our own (white?)

see bug 417279
It's ironic that this bug is labeled "Use native platform colors", because your approach to pick rgb values is rather opposed to that. This is very bad for Windows (ignoring third-party themes and Royale, XP has three Luna color schemes and a load of Classic color schemes) and a showstopper on Linux (different distributions ship different themes). I don't know about OS X... you can probably get away with hard coding all colors; seems to be common practice for Pinstripe.
Would it be possible to make the line between rows only span about 90% of the width? like an HTML <hr> element, they don't connect to the sides. I'm guessing you would have to use something other than border-bottom for that.
Attached image not complete border
(In reply to comment #19)
> Would it be possible to make the line between rows only span about 90% of the
> width?
Easier than you think. At least depending on how short you want it.

 .autocomplete-richlistitem {
   padding: 1px 2px;
   border-bottom: 1px solid ThreeDLightShadow;
+  -moz-border-radius: 100%;
 }

Make use of the border radius which just cuts off if there's no side borders.
Hahah.. oops. That wasn't expected.
>This is very bad for Windows (ignoring third-party themes and Royale, XP has >three Luna color schemes and a load of Classic color schemes)

XP doesn't change its icon set when you change OS themes, and we are basing these colors on the palette of the XP icon set (http://people.mozilla.com/~faaborg/files/granParadisoUI/icons/rfp/xpColors.gif).  This color palette was designed to work well with the three luna themes.  So the colors are native, they just aren't extracted.
(In reply to comment #17)
> (In reply to comment #15)
> > forcing our own (white?)
> see bug 417279
So what should we do if we're to specify those particular colors on each platform?
I was going to suggest we use Window for background, WindowText for title and Browser.anchor_color for the URL (since it's kind of like an anchor) but for one I don't think we can use browser.anchor_color in css? and 2, it looks a little odd.
(In reply to comment #22)
> XP doesn't change its icon set when you change OS themes, and we are basing
> these colors on the palette of the XP icon set
> (http://people.mozilla.com/~faaborg/files/granParadisoUI/icons/rfp/xpColors.gif).
>  This color palette was designed to work well with the three luna themes.

Looks quite worthless without a word where the colors should be used. It's also clear that text isn't comparable to icons, since you can mix colors within a single icon so that it doesn't just disappear on a different background.

>  So the colors are native, they just aren't extracted.

Right, so we shouldn't use them until they are.

Btw, #009900 is not going to work in high contrast mode.
It's slightly misusing a color, but how about ActiveCaption for URL text?
All the default high contrast themes in XP deal with this nicely. I don't know what the default ActiveCaption is on vista, but the default on XP looks decent.
(In reply to comment #26)
> It's slightly misusing a color, but how about ActiveCaption for URL text?

It's actually a background color. But I agree that misusing a native color (assuming it works with the themes that ship with XP and Vista) is better than hard-coding a color.

Anyway, the safest and most logical choice is GrayText.
Depends on: 425598
Bugs 409974 and 423718 are both blocking+, yet they both want to change the color of URLs in the location bar autocomplete results.  To resolve the issue, I've filed bug 425598 – All hard coded colors need to fall back to system colors for accessibility.
Blocks: 425582
Reading this bug highlights one interesting point: you don't think about users who don't know about Microsoft guidelines for colors, and choose their own.

How can hard coding colors work with that?
>Reading this bug highlights one interesting point: you don't think about users
>who don't know about Microsoft guidelines for colors, and choose their own.

We won't be able to support detection of the OS theme with this release, but that is certainly something that we hope to do in the future, so we can also fall back to system colors to support other third party OS themes.  Only a very small fraction of users currently use third party OS themes, so not supporting them isn't a huge disaster.  Also, users with a third party OS theme can certainly switch to another Firefox theme, or even use userchrome.css hacks since the user clearly has an interest in modifying the overall appearance of their system.
(In reply to comment #30)
> >Reading this bug highlights one interesting point: you don't think about users
> >who don't know about Microsoft guidelines for colors, and choose their own.
> 
> We won't be able to support detection of the OS theme with this release, but
> that is certainly something that we hope to do in the future, so we can also
> fall back to system colors to support other third party OS themes.  Only a very
> small fraction of users currently use third party OS themes, so not supporting
> them isn't a huge disaster.  Also, users with a third party OS theme can
> certainly switch to another Firefox theme, or even use userchrome.css hacks
> since the user clearly has an interest in modifying the overall appearance of
> their system.
> 

Changing a color with the Control Panel Display window is certainly not the same as editing userChrome.css. And I wouldn't consider that as third party OS theme. It's only a color change!

And I wonder how can anyone measure how users choose their colors.
This is the
This is the UI that Windows offers to change its colors.

I think that is fairly easy for a user to change them.
Blocks: 405605
Depends on: 426660
No longer depends on: 425598
Blocks: 409974
maybe i'm missing something, but -moz-hyperlinktext seems like the appropriate color choice to me.  that or GrayText.  either of those is pretty much guaranteed to show up against Window.
-moz-hyperlinktext isn't guaranteed to show up. As long as it defaults to blue, we'd run into bug 371870.
(In reply to comment #35)
> As long as it defaults to blue

As opposed to a system color that is, which can of course be blue, too.
Once bug 409974 is resolved, you should be able to pick any color on Windows.
No longer blocks: 409974
Depends on: 409974
Attached patch v2 (obsolete) — Splinter Review
For landing after bug 409974. gnomestripe already has a color and leaving it as graytext. XP/Vista color for non-default theme is graytext.

per faaborg:

xp: #006600
vista: #0066cc
os x: #144fae

All platforms use ThreeDLightShadow for the line (gnomestripe already has it)
Attachment #310845 - Attachment is obsolete: true
Attachment #315287 - Flags: ui-review?(beltzner)
Attachment #315287 - Flags: review?(beltzner)
Keywords: polish
Whiteboard: [has patch][need review beltzner][need bug 409974]
Wouldn't COLOR_HOTLIGHT be the correct color to use for Windows (AKA -moz-nativelinktext once bug 426732 is fixed)?
(In reply to comment #39)
> Wouldn't COLOR_HOTLIGHT be the correct color to use for Windows (AKA
> -moz-nativelinktext once bug 426732 is fixed)?

Yes, we should consider 1) using it instead of GrayText and 2) dropping the windows-default-theme special case once that bug is fixed.
Depends on: 426732
Can I see a screen shot of COLOR_HOTLIGHT text on XP and Vista?  It is probably exactly what we want, but just want to make sure.
I for one am surprised the XP COLOR_HOTLIGHT isn't #0000FF
Nice. Gets my vote.
Attached patch v2.1 (obsolete) — Splinter Review
Here's a version that needs bug 426732 for -moz-nativelinktext.

Does it also work on linux?
Attachment #315782 - Flags: ui-review?(beltzner)
Attachment #315782 - Flags: review?(beltzner)
Whiteboard: [has patch][need review beltzner][need bug 409974] → [has patch][need review beltzner][can need bug 426732]
no change to the color on Luna.

One odd point of mention, the Hyperlink color is not something that can be changed by users on XP, but strangely can on Vista.

Linux support is still pending, I'm trying to find a GTK Widget that I can use to grab link-color

It does already work on OS X, but it's hard coded anyways, since we can't find any native color.
Linux support is now in on bug 426732, but my quick unscientific survey shows that the majority of the time we'll be defaulting to #0000EE. Either way the link-color support is there, and if Firefox makes use of it then it's possible we'll see the major distros specify a nice link-color in default themes at least.
Duplicate of this bug: 429541
Attached patch v2.2Splinter Review
Include gnomestripe changes.
Attachment #315287 - Attachment is obsolete: true
Attachment #315782 - Attachment is obsolete: true
Attachment #316698 - Flags: ui-review?(beltzner)
Attachment #316698 - Flags: review?(beltzner)
Attachment #315287 - Flags: ui-review?(beltzner)
Attachment #315287 - Flags: review?(beltzner)
Attachment #315782 - Flags: ui-review?(beltzner)
Attachment #315782 - Flags: review?(beltzner)
Comment on attachment 315287 [details] [diff] [review]
v2

This might not be a bad alternative if we don't get -moz-nativelinktext.
Attachment #315287 - Attachment is obsolete: false
Attachment #315287 - Flags: ui-review?
Attachment #315287 - Flags: review?
Comment on attachment 315287 [details] [diff] [review]
v2

Need to use menuText for non-default themes. Sucks, as it breaks easy scan partitioning, but it's the safest option available to us.

Otherwise it's OK, and you can carry over review.
Attachment #315287 - Flags: ui-review?
Attachment #315287 - Flags: ui-review+
Attachment #315287 - Flags: review?
Attachment #315287 - Flags: review+
Whiteboard: [has patch][need review beltzner][can need bug 426732] → [has patch][better with bug 426732]
Comment on attachment 316698 [details] [diff] [review]
v2.2

This is the better approach, but until bug 426732 lands, I can't give it review. Re-request when the dependency clears?
Attachment #316698 - Flags: ui-review?(beltzner)
Attachment #316698 - Flags: ui-review+
Attachment #316698 - Flags: review?(beltzner)
Attachment #316698 - Flags: review-
Attached patch v2.3Splinter Review
Attachment #315287 - Attachment is obsolete: true
Attachment #317145 - Flags: approval1.9?
Comment on attachment 317145 [details] [diff] [review]
v2.3

a=beltzner, hm, what other flags can I give this?
Attachment #317145 - Flags: superreview+
Attachment #317145 - Flags: review+
Attachment #317145 - Flags: approval1.9?
Attachment #317145 - Flags: approval1.9+
Whiteboard: [has patch][better with bug 426732] → [has patch][has reviews][better with bug 426732]
http://hg.mozilla.org/cvs-trunk-mirror/index.cgi/rev/61e4231f14cb
http://hg.mozilla.org/mozilla-central/index.cgi/rev/61e4231f14cb

Checking in browser/themes/pinstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/pinstripe/browser/browser.css,v  <--  browser.css
new revision: 1.148; previous revision: 1.147
done
Checking in browser/themes/winstripe/browser/browser-aero.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/browser-aero.css,v  <--  browser-aero.css
new revision: 1.4; previous revision: 1.3
done
Checking in browser/themes/winstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/browser.css,v  <--  browser.css
new revision: 1.204; previous revision: 1.203
done
Checking in toolkit/themes/pinstripe/global/autocomplete.css;
/cvsroot/mozilla/toolkit/themes/pinstripe/global/autocomplete.css,v  <--  autocomplete.css
new revision: 1.23; previous revision: 1.22
done
Checking in toolkit/themes/winstripe/global/autocomplete.css;
/cvsroot/mozilla/toolkit/themes/winstripe/global/autocomplete.css,v  <--  autocomplete.css
new revision: 1.33; previous revision: 1.32
done
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [has patch][has reviews][better with bug 426732] → [better with bug 426732]
Target Milestone: --- → Firefox 3
I was under the impression that the link color in awesomebar on linux would be changed at some point because it is so unreadable ATM (faint gray on white background with my theme). Is there some other bug open for this issue? 
(In reply to comment #55)
> I was under the impression that the link color in awesomebar on linux would be
> changed at some point because it is so unreadable ATM (faint gray on white
> background with my theme). Is there some other bug open for this issue?

Not yet, but it would depend on bug 426732.
(In reply to comment #51)
> (From update of attachment 316698 [details] [diff] [review])
> This is the better approach, but until bug 426732 lands, I can't give it
> review. Re-request when the dependency clears?

This is now in bug 437358.
You need to log in before you can comment on or make changes to this bug.