587909 - Improve the visual style of location bar results

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect)

Description

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

This bug covers improving the visual style of location bar results on all platforms.  The UX team will provide more detailed mockups, but in general changes are likely to include:

-removing the lines separating results in favor of whitespace
-using black/grey text to show highlighting instead of bold

-actual menu back on OS X

Comment 1

[Ashley Bischoff (blog at handcoding.com)]

(In reply to comment #0)
> This bug covers improving the visual style of location bar results on all
> platforms. The UX team will provide more detailed mockups, but in general
> changes are likely to include:

Alex: Do you think we may be able to use a vertical rhythm for the redesigned location bar results?


For anyone who may be unfamiliar with the technique, this excerpt from A List Apart goes over the basic idea:

"The main principle of the baseline grid is that the bottom of every line of text (the baseline) falls on a vertical grid set in even increments all the way down the page. Imagine those old Big Chief ruled writing pads they gave you in grade school to practice penmanship and you've got the basic idea. The magical end result is that all the text on your page lines up across all the columns, creating a harmonious vertical rhythm. [...] "

-- "Setting Type on the Web to a Baseline Grid" (http://www.alistapart.com/articles/settingtypeontheweb)

That ALA article covers the subject pretty well, but in case it helps, I did also come across this post with links to some additional resources: http://www.clagnut.com/blog/1942/

Comment 2

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

Quick update: we'll hopefully be taking this bug on after the feature freeze and during the polish phase as we finish up Firefox 4.

Comment 3

[Alex Limi (:limi) — Firefox UX Team]

Additionally: Shift-Delete isn't discoverable enough, we should have a close button that shows up on hover to allow you to delete an item. We could even offer to forget everything about that site, or just the selected URL.

Comment 4

[Frank Yan (:fryn)]

Do we have a mockup or a concrete list of changes for this? That would be a helpful guide for potential patch makers.

Comment 5

[Stephen Horlander [:shorlander]]

(In reply to comment #4)
> Do we have a mockup or a concrete list of changes for this? That would be a
> helpful guide for potential patch makers.

Not yet. This is something I am working on.

Comment 6

[(no longer active)]

(In reply to comment #3)
> Additionally: Shift-Delete isn't discoverable enough, we should have a close
> button that shows up on hover to allow you to delete an item. We could even
> offer to forget everything about that site, or just the selected URL.

Do we want to get the same UI for other autocomplete lists as well?

Comment 7

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

Just clearing the single UI turns this into a non-stop game of whack-a-mole.  For instance, what if the user viewed 700 different pages on the same sensitive site?  So I think we probably want this close button to act on the domain, perhaps even with a confirmation prompt/panel stating how many history entries are going to be removed (same prompt appears in the library window with "forget about this site" right now).

Comment 8

[Stephen Horlander [:shorlander]]

Attachment: LocationBar Result Improvements - i01

(In reply to comment #4)
> Do we have a mockup or a concrete list of changes for this? That would be a
> helpful guide for potential patch makers.

As noted in the project page(https://wiki.mozilla.org/Improve_display_of_location_bar_results) we have a few things we would like to change:

General
* 32x32px icons
* No use of bold, since it shifts the content
* More whitespace
* No separator lines
* Ability to delete entry on hover
* Remove scrollbar

OS X
* Use "native" menu style

Linux
* Switch to using a different system color besides Menu for the background color (e.g. Window)
  - Many current Linux themes use a dark Menu color which feels pretty heavy for these results

We have brainstormed several ideas for making the results more easy to scan. This attachment is the first iteration. The results here are:

* Lighter title text, dark string matching text
* Lighter URL text, dark string matching text
* Subtle background box/highlight

Comment 9

[Peter Henkel [:Terepin]]

Solid blue hover looks ugly in Vista/7. We should use Aero hover.

Comment 10

[Marco Bonardo [:mak]]

> * 32x32px icons

this will probably require changes to the favicon service, iirc we scale down icons to 16x16 in most cases.

Comment 11

[d]

We may also want to investigate how many sites that actually supply favicons larger than 16x16 today. My impression is that it's quite rare, but there's no reason that couldn't change once we release the new style, of course.

Comment 12

[:Margaret Leibovic]

(In reply to Magne Andersson from comment #11)
> We may also want to investigate how many sites that actually supply favicons
> larger than 16x16 today. My impression is that it's quite rare, but there's
> no reason that couldn't change once we release the new style, of course.

I wrote a blog post about this: http://blog.margaretleibovic.com/post/8099813509/larger-site-icons. Some of the sites that have started supplying larger icons are doing it inside .ico files, which will require a change to our image decoder code if we want to be able to read these (bug 419588). However, there are other sites that just supply a single larger icon that we currently scale down, as Marco noted in comment 10.

Marco, is there a bug anywhere about changing the favicon service to handle larger icons? If not, I think it would be worth filing one, since has come up before.

Comment 13

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

>but there's no reason that couldn't change once we release the new style, of
>course.

yeah, chicken and egg problem.  It's way past time for sites to start exposing high resolution icons but they are only go to do it once they are used (look at how many sites exposed iPhone specific icons).  Ideally we'll also start to see usage of the HTML5 spec for multiple icons sizes, so people don't feel like they need to generate an .ico file, which is admittedly pretty difficult to do.

Comment 14

[Stephen Horlander [:shorlander]]

(In reply to Peter Henkel [:Terepin] from comment #9)
> Solid blue hover looks ugly in Vista/7. We should use Aero hover.

I agree. I will update the design to use that style instead.

Comment 15

[Peter Henkel [:Terepin]]

Just FYI, I'm using this style for months and I'm very satisfied with it. Maybe it can be helpful in some way to owner of this bug: http://userstyles.org/styles/19308/awesomebar-popup

Comment 16

[Stephen Horlander [:shorlander]]

Attachment: LocationBar Result Improvements - i02

Updated Windows 7 Design

Comment 17

[Stephen Horlander [:shorlander]]

(In reply to Peter Henkel [:Terepin] from comment #15)
> Just FYI, I'm using this style for months and I'm very satisfied with it.
> Maybe it can be helpful in some way to owner of this bug:
> http://userstyles.org/styles/19308/awesomebar-popup

I have been using this, good stuff. Thanks!

Comment 18

[chaosfreedom1220]

hope to see this soon, even though I guess it will be FF11 nightly instead of FF10?

Comment 19

[frogstar101]

I'm the author of that userstyle, glad you chaps like it. The mockups are looking great too, hope it become implemented soon.
On a side note, on the topic of discoverability of function; here's an old bug that I think needs to be resolved if Firefox is to have a native feel:
https://bugzilla.mozilla.org/show_bug.cgi?id=631444

Comment 20

[Tommy XP]

I find that having the title and url difficult to search.
Usually I just look at url, then look at title to make sure or the other way around.

Having urls and title in different columns would make it easier to scan. This is what I noticed when I used safari for a bit. Each results in one line and with url and title in a column.

Comment 21

[Paul Rouget [:paul]]

What is the current status of this bug? Is anyone working on it? Can it be a good first bug or not really?

Comment 22

[Peter Henkel [:Terepin]]

Well, Stylish style already exists, so it only needs changes. At least I think so.

Comment 23

[Dão Gottwald [:dao]]

(In reply to Paul Rouget [:paul] from comment #21)
> Can it be a good first bug or not really?

No, more like good twentieth bug. Getting the styling right for all OSes isn't trivial and there seem to be open questions regarding the favicon size.

Comment 24

[Peter Henkel [:Terepin]]

Can be current user style used as a base for Windows or you'll have to start from scratch?

Comment 25

[:Margaret Leibovic]

(In reply to Dão Gottwald [:dao] from comment #23)
> (In reply to Paul Rouget [:paul] from comment #21)
> > Can it be a good first bug or not really?
> 
> No, more like good twentieth bug. Getting the styling right for all OSes
> isn't trivial and there seem to be open questions regarding the favicon size.

Yes, the mockups include use 32x32 favicons, so we'd need to do something about getting those. FWIW, bug 517525 added a pref (places.favicons.optimizeToDimension) to let us store larger icons, so this might not be too hard.

Comment 26

[Robert Claypool]

>For instance, what if the user viewed 700 different pages on the same sensitive site?
Wait... just how many entries does the address bar keep? I can't think of any site that I would have viewed 700 pages of, not to mention at one go, I can think of some times when I'd like to prune back a site, like YouTube to keep more used links around but not have links that I won't be using anymore.
> Additionally: Shift-Delete isn't discoverable enough,
Why is it Shift+Del instead of just Del. I can understand an initial desire to avoid a conflict between deleting text in the address bar with deleting an entry, but once I'm scrolling through the entries, I'm no longer messing with the address bar text. 

I just checked and Del does behave just as I described, so maybe you just aren't aware of how it actually functions?

I could use a right-click menu with "copy", "delete" and "Open in new tab" options. The copy option would copy the URL to the clipboard. I have a vague notion of how this would be useful.

Comment 27

[Vova Olar]

HI. I have 2 questions related to this bugs. Can anyone help me to clarify them?
Is there filled bugs for:
1. to don't close results after middle click (or CTRL-Enter)
2. keyboard navigation in results (how to open in new tab for example?) I think bug 237027 should be taken into account here)

Comment 28

[u428464]

Is there any plans about Tags in the new design of the location bar ?

Comment 29

[Jonathan Haas]

(In reply to Stephen Horlander from comment #16)
> Created attachment 554216 [details]
> LocationBar Result Improvements - i02
> 
> Updated Windows 7 Design

Is it intended to remove the dropdown arrow? I use it regularly to open my recently visited sites.

Comment 30

[(mostly gone) XtC4UaLL [:xtc4uall]]

(In reply to Jonathan Haas from comment #29)
> Is it intended to remove the dropdown arrow? I use it regularly to open my
> recently visited sites.

See Bug 712602 :-/.

Comment 31

[Rakshith]

So is there any scope for pre-loading sites/instant/search results in Firefox like chrome? Because that would be awesome if integrated along with the overhauled suggest drop down.

Comment 32

[Marco Bonardo [:mak]]

(In reply to Rakshith from comment #31)
> So is there any scope for pre-loading sites/instant/search results in
> Firefox like chrome?

not the scope of this bug.

Comment 33

[Rakshith]

true story

Comment 34

[u428464]

I think that this bug needs to be redefined with the future merge of the search and the location bar. It needs more than just graphical changes. The bug isn't planned for Firefox 13 anymore.

Comment 35

[Itiel]

Oh man! I was counting on it :(
So this bug is planned for what version of firefox?

Comment 37

[Andrew Hurle [:ahurle]]

Attachment: v1 - Style changes for Windows 7

A first try for the CSS/XUL changes to match the mockup for Windows 7.  Feel free to give feedback.  I haven't considered other operating systems yet.  I'll worry about proper favicon resolution, scrollbar removal, and a functional delete button in other bugs.

This patch has a few problems:

- The gray stripe on the left is made with a hardcoded gradient background on .autocomplete-richlistbox.  This is necessary for the [selected] background of .autocomplete-richlistitem to appear above the gray stripe.  The gradient does not automatically expand with .ac-site-icon, which is ugly.  There are other ways to accomplish this hover effect with the gray stripe as a background for .ac-site-icon, but I think they would result in significantly uglier CSS.

- Lack of proper system colors, so this doesn't look very good on different Windows themes.  Not sure which ones to use, or if it's even possible to use them .

- The .ac-emphasize-text spans are taller than shown in the mockup, and line-height can't make them any smaller.  I assume this is to accommodate larger characters with diacritics, umlauts, etc.  This spreads the title and URL text farther apart - we may want additional margin between results to compensate.  I might be able to use some gross hack to squish the lines closer together, but that may cause certain characters to overlap.

Comment 38

[Andrew Hurle [:ahurle]]

Attachment: v1 Screenshot on Windows 7

Comment 39

[Andrew Hurle [:ahurle]]

(In reply to Stephen Horlander from comment #16)
> Created attachment 554216 [details]
> LocationBar Result Improvements - i02
> 
> Updated Windows 7 Design

After talking with Jared, we agreed that the results box shouldn't be transparent.  It's a high cost in performance to achieve a very subtle effect.  I can't find a quick way to make panels transparent anyway, so it looks like it would take more time to achieve than one might expect.

There are a few lingering questions not answered by the mockup:

- As noted in comment 28, where are bookmark tags going?  In the screenshot, you can see that I've just left them where they were previously.

- What are Switch to Tab results going to look like?

- What are keyword search results going to look like?


Also, could you please provide the icons, color values, and opacity values used in the mockup so I can use them in the CSS?  Might be easiest to attach a PSD for now.

Comment 40

[Matthew N. [:MattN]]

Comment on attachment 628490 [details] [diff] [review]
v1 - Style changes for Windows 7

Review of attachment 628490 [details] [diff] [review]:
-----------------------------------------------------------------

Some notes from my quick glance:

::: browser/themes/winstripe/browser.css
@@ +1490,4 @@
>  }
>  
>  .ac-extra > .ac-comment {
> +  font-size: 0.8;

Nit: Units please.

::: toolkit/content/widgets/autocomplete.xml
@@ +1230,4 @@
>          </xul:hbox>
> +      </xul:vbox>
> +      <xul:image anonid="type-image" class="ac-type-icon"/>
> +      <xul:button anonid="delete-button" class="ac-delete-button"/>

It would be best to keep the delete button changes (including styles) for bug 675818

Comment 41

[bogas04]

Great work ! Looks nice :)

On a side note , why are we using upscaled icons ? When many sites with different factions would have such distorted effect in awesome bar it would just cause eye sore. 

We can , umm , implement the concept of  1st iteration of new tab page , where a small icon was used , surrounded by its color ?

http://people.mozilla.com/~faaborg/files/20110610-homeTabNewTab/newTab-i1.png


Thanks again for posting tha screenshot :)

Comment 42

[Andrew Hurle [:ahurle]]

(In reply to bogas04 from comment #41)
> Great work ! Looks nice :)
> 
> On a side note , why are we using upscaled icons ? When many sites with
> different factions would have such distorted effect in awesome bar it would
> just cause eye sore.

Thanks.

The blown up favicons are definitely just a placeholder for now - getting them in their proper resolution and handling smaller icons will be taken care of in a different bug.

Comment 43

[u428464]

Very nice work. I think Stephen Horlander may have the answers you need and maybe some newer or more complete mockups.

Comment 44

[Dão Gottwald [:dao]]

Unfortunately, given the lack of high-quality favicons, I think we need new mockups with 16x16 favicons. Since we didn't end up removing the icon in the location bar, the wider icons column in the popup also looks unbalanced.

Comment 45

[Andrew Hurle [:ahurle]]

Since we're no longer going to use bold for .ac-emphasize-text, I think we can tear out everything associated with .ac-emphasize-alt as introduced for bug 407861.

Comment 46

[Andrew Hurle [:ahurle]]

Jared Wein spoke with Horlander at some point, who said that we should just use the dominant color background as described in comment 41 and bug 634139 to accommodate the smaller icons.

Comment 47

[u428464]

Bug 690191 could also be part of the redesign.

Comment 48

[Dão Gottwald [:dao]]

(In reply to Andrew Hurle [:ahurle] from comment #46)
> Jared Wein spoke with Horlander at some point, who said that we should just
> use the dominant color background as described in comment 41 and bug 634139
> to accommodate the smaller icons.

I have seen no mockups for this and I'm not sure it's going to improve usability. Implementing this is also going to be another roadblock for the other improvements this bug was aiming for. Generally, I don't think it's wise to keep widening the scope here. The alignment issue with larger icons vs. the identity block icon mentioned in comment 44 also remains.

Comment 49

[Andrew Hurle [:ahurle]]

(In reply to Dão Gottwald [:dao] from comment #48)
> (In reply to Andrew Hurle [:ahurle] from comment #46)
> > Jared Wein spoke with Horlander at some point, who said that we should just
> > use the dominant color background as described in comment 41 and bug 634139
> > to accommodate the smaller icons.
> 
> I have seen no mockups for this and I'm not sure it's going to improve
> usability. Implementing this is also going to be another roadblock for the
> other improvements this bug was aiming for. Generally, I don't think it's
> wise to keep widening the scope here. The alignment issue with larger icons
> vs. the identity block icon mentioned in comment 44 also remains.

I think it would be agreeable to keep the favicons in their old 16x16 positions for this patch to avoid the alignment and scope issues, but at least move forward with the other improvements.  Then we'll be able to land those improvements quickly (pending Stephen's answers to my questions in comment 39) and tackle the issue of differently sized icons at a later point.

Comment 50

[Andrew Hurle [:ahurle]]

Attachment: v2 - Favicons in old 16x16 positions, no delete button, cleanup

This patch has the favicons in the same size and position as the current Nightly.

Comment 51

[Andrew Hurle [:ahurle]]

Attachment: v2 Screenshot on Windows 7

Comment 52

[Darren Kalck [:D-Kalck]]

Could it be pushed to UX ?

Comment 53

[Andrew Hurle [:ahurle]]

Attachment: v3 - Changes necessary for all 4 platforms

In order to emphasize matched text while still using system colors, I used "text-shadow: 0 0 CurrentColor" to add a slight bold effect without shifting content.  This is different than the mockup, which looks like it uses a lighter color for non-emphasized text.  I'm not aware of any way to say "use -moz-nativehyperlinktext, but slightly lighter".  I suppose what we do depends on how strict we want to be about using that system color.

On OSX, I can't figure out how to add that rounded corner to the results panel.  That style only seems to apply to menupopup elements (as in #BMB_bookmarksPopup in browser.xul).  The panel doesn't respond to border-radius.

I don't think we have a trivial way to tweak the panel drop shadow.  Ubuntu is missing one completely, Windows 7 and XP are different than the mockup.  Not sure if the change of shadow in the mockup was intentional or not.  -moz-window-shadow might have been promising, but it only works on OSX.

Comment 54

[Andrew Hurle [:ahurle]]

Attachment: v3 Screenshots on all platforms

Comment 55

[Dão Gottwald [:dao]]

(In reply to Andrew Hurle [:ahurle] from comment #50)
> Created attachment 630389 [details] [diff] [review]
> v2 - Favicons in old 16x16 positions, no delete button, cleanup
> 
> This patch has the favicons in the same size and position as the current
> Nightly.

Yet it makes the popup's icon row wider, making it different from how the icon and text are set up in the location bar. Why?

Comment 56

[u428464]

Have you got the answers you need about tags, keyword search and switch to tab ? And the new icons ? 

In another thread there was also mention of a magnifying glass for search from the location bar (ala Chrome), but it was blocked by another bug.

Comment 57

[u428464]

According to the mockup the location bar popup should have rounded corners.

Comment 58

[Itiel]

Wow.. very nice Andrew.
Since it has been decided not to set 32x32 favicons, why won't you at least vertical-center the favicon to each block in the location bar?
I think it would seem prettier.

Comment 59

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 632050 [details] [diff] [review]
v3 - Changes necessary for all 4 platforms

Review of attachment 632050 [details] [diff] [review]:
-----------------------------------------------------------------

I agree with Dao that the misalignment of our text looks less than good. I think we can only get away with the misalignment if the icon area is much larger. Since we're sticking with 16x16 icons for this first phase, we should remove the secondary background color of the icon area on Aero and keep everything aligned to the same grid.

When/if we decide to go with the larger icons, then we should relook at breaking the current alignment.

Overall this patch looks very good. Nice work :)

::: toolkit/themes/gnomestripe/global/autocomplete.css
@@ +164,5 @@
>  html|span.ac-emphasize-text {
> +  box-shadow: inset 0 0 1px 1px rgba(0,0,0,0.1);
> +  background-color: rgba(0,0,0,0.05);
> +  border-radius: 2px;
> +  text-shadow: 0 0 CurrentColor; /*faux bold effect*/

From MDN documentation: "If the <color> value is unspecified, then Gecko uses the value of the element's color property." [https://developer.mozilla.org/en/CSS/text-shadow#Browser_compatibility]. If you still want to explicitly set the color here, you should use |currentColor| as that is the casing that the spec uses.

I can only see this faux bold effect working on some colors but not all. I expect it would only work on colors that have an alpha value of < 1 or potentially on the edges of anti-aliased text. Another (more intuitive) technique would be to vary the opacity on text, but we would have to measure the perf hit for that.

::: toolkit/themes/winstripe/global/autocomplete.css
@@ +118,5 @@
> +/*
> +-moz-appearance: menuitem is almost right, but the hover effect is not
> +transparent and is lighter than desired.
> +*/
> +.autocomplete-richlistitem[selected="true"] {

For this, you should make sure that this doesn't apply in modes like high contrast. Can you add :-moz-windows-default-theme to this selector so it only applies when the default theme is being used?  [https://developer.mozilla.org/en/CSS/Media_queries#-moz-windows-default-theme]

Comment 60

[Andrew Hurle [:ahurle]]

(In reply to Dão Gottwald [:dao] from comment #55)
> (In reply to Andrew Hurle [:ahurle] from comment #50)
> > Created attachment 630389 [details] [diff] [review]
> > v2 - Favicons in old 16x16 positions, no delete button, cleanup
> > 
> > This patch has the favicons in the same size and position as the current
> > Nightly.
> 
> Yet it makes the popup's icon row wider, making it different from how the
> icon and text are set up in the location bar. Why?

On Windows 7, the icon looks awkward to me if it isn't centered within its gray column.  Bringing the result title text really tight next to the column's right border also looks bad to me and goes against the stated goal of "more whitespace".

However, if we get rid of the gray background as Jared suggests, those are no longer issues and I can line up the text.

(In reply to Guillaume C. [:ge3k0s] from comment #56)
> Have you got the answers you need about tags, keyword search and switch to
> tab ? And the new icons ? 

I pinged Stephen and he said he will get to this when he can - he has a backlog of things to look at.

(In reply to Guillaume C. [:ge3k0s] from comment #57)
> According to the mockup the location bar popup should have rounded corners.

Yeah, I mentioned that I'm not sure how to do that on OSX since the panel doesn't react to changes to border-radius.  I don't think that this problem and drop shadows is worth blocking the landing of this patch, unless someone knows of a quick way to do them.

(In reply to Jared Wein [:jaws] from comment #59)
> I can only see this faux bold effect working on some colors but not all. I
> expect it would only work ... on the edges of anti-aliased text.

I think that's why it works now as seen in the screenshot.  It's very subtle.

> Another (more intuitive)
> technique would be to vary the opacity on text, but we would have to measure
> the perf hit for that.

The emphasized spans are within the Title/URL container.  I would have to alter the opacity on the container to make non-emphasized text lighter, but that would also lower the opacity of the emphasized text.

I guess I could wrap non-emphasized text segments in their own spans to alter their opacity specifically.  Looks like a pretty straightforward change to autocomplete-richlistitem._setUpDescription.

(In reply to ETL from comment #58)
> Since it has been decided not to set 32x32 favicons, why won't you at least
> vertical-center the favicon to each block in the location bar?
> I think it would seem prettier.

Until we have further input from UX, I'm just sticking with the previous layout which had them on the same line as the result title.

Comment 61

[Andrew Hurle [:ahurle]]

Attachment: v4 - Support for non-default Windows themes, align titles with location bar

On Windows 7, using a non-default theme will now make the hover effect fall back to Highlight/HighlightText colors.  Got rid of the gray background in Windows 7 so I could align everything properly.  Removed unnecessary CurrentColor in text-shadow.

Comment 62

[Andrew Hurle [:ahurle]]

Attachment: v4 Screenshots for all platforms

I should also mention that the v4 patch fixes a test that was failing on Windows.  The test was synthesizing a click in the very top left corner of one of the autocomplete results.  This is dead space in Windows 7 now since the results have a border-radius.  I made the test click in the center instead, as it probably should have in the first place.

https://tbpl.mozilla.org/?tree=Try&rev=a4ff1ee8afe2

The rest of the failures in mochitest-a11y went away after fixing the first one and testing locally.

Comment 63

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 633757 [details] [diff] [review]
v4 - Support for non-default Windows themes, align titles with location bar

Review of attachment 633757 [details] [diff] [review]:
-----------------------------------------------------------------

This looks fine to me, but I'll wait to hear from Stephen on the ui-review before.

In the screenshots, there is a one or two pixel overlap between the text highlight background-color and border in some of the themes. I know that you've tried your hardest to reduce the height of that background-color/border and couldn't find a way to do it, but it would be nice for this situation to fix that. How bad would it look if you put one or two pixels between the top text and bottom text?

::: toolkit/themes/pinstripe/global/autocomplete.css
@@ +97,2 @@
>    color: HighlightText;
> +  background-image: -moz-linear-gradient(to bottom, rgba(255,255,255,0.3), rgba(255,255,255,0))

|to bottom, | is the default here, so we can just remove that part of the property value.

Comment 64

[Andrew Hurle [:ahurle]]

Attachment: v5 - Consistent 1px margin between lines

This makes Windows 7 and Ubuntu have a 1px space between lines like XP and OSX as Jared suggested.

Comment 65

[Jared Wein [:jaws] (please needinfo? me)]

I'll wait for Stephen's review before proceeding here.

Comment 66

[Dão Gottwald [:dao]]

Comment on attachment 634187 [details] [diff] [review]
v5 - Consistent 1px margin between lines

>--- a/toolkit/themes/pinstripe/global/autocomplete.css
>+++ b/toolkit/themes/pinstripe/global/autocomplete.css

> .autocomplete-richlistitem[selected="true"] {
>-  background-color: Highlight;
>+  background-color: rgb(0,85,253);

Why this change? This probably breaks OS X Graphite support.

>--- a/toolkit/themes/winstripe/global/autocomplete.css
>+++ b/toolkit/themes/winstripe/global/autocomplete.css

>+%ifdef WINSTRIPE_AERO
>+@media (-moz-windows-default-theme) {
...
> }
> 
>+/* Apply basic colors to XP and non-default Vista+ themes */
>+@media not all and (-moz-windows-default-theme) {
>+%endif
>+  .autocomplete-richlistitem[selected="true"] {
>+    background-color: Highlight;
>+    color: HighlightText;
>+  }
>+%ifdef WINSTRIPE_AERO
> }
>+%endif

Move the second block before the first one and remove "@media not all and (-moz-windows-default-theme) {".

> html|span.ac-emphasize-text {
>+  box-shadow: inset 0 0 1px 1px rgba(0,0,0,0.1);
>+  background-color: rgba(0,0,0,0.05);
>+  border-radius: 2px;
>+  text-shadow: 0 0 currentColor;
> }
> 
>+.ac-url-text > html|span.ac-emphasize-text,
>+.ac-action-text > html|span.ac-emphasize-text {
>+  box-shadow: inset 0 0 1px 1px rgba(79,135,173,0.2);
>+  background-color: rgba(79,135,173,0.1);
>+}
>+
>+%ifndef WINSTRIPE_AERO
>+html|span.ac-emphasize-text {
>+  box-shadow: inset 0 0 1px 1px rgba(208,208,208,0.5);
>+  background-color: rgba(208,208,208,0.3);
>+}
>+.ac-url-text > html|span.ac-emphasize-text,
>+.ac-action-text > html|span.ac-emphasize-text {
>+  box-shadow: inset 0 0 1px 1px rgba(202,214,201,0.3);
>+  background-color: rgba(202,214,201,0.2);
>+}
>+%endif

I'm not sure what you're doing here. I suspect these colors should be OS-theme specific rather than OS specific (WINSTRIPE_AERO == Vista or later).

Comment 67

[Andrew Hurle [:ahurle]]

Attachment: v6 - Use "Highlight" for selected background on OSX, clearer theme-specific emphasis on Windows

(In reply to Dão Gottwald [:dao] from comment #66)
> Why this change? This probably breaks OS X Graphite support.

You're right, it breaks user-specified highlight colors entirely.  The mockup had a different highlight color, which is why I originally changed it.

> I'm not sure what you're doing here. I suspect these colors should be
> OS-theme specific rather than OS specific (WINSTRIPE_AERO == Vista or later).

Yeah, that part wasn't very... correct.  I think this patch will be clearer.  All themes now start with a whiteish color as the emphasis background.  When on the default theme, Aero will override that with its own title/URL colors, while XP will override the URL color.  This also has the benefit of making the background actually visible on non-default themes.  I'll include a screenshot of a high contrast theme in my next batch since shorlander will probably want to see it.

Thanks for your feedback.

Comment 68

[Andrew Hurle [:ahurle]]

Attachment: v6 Screenshots on all platforms + high contrast Windows 7

Comment 69

[Jared Wein [:jaws] (please needinfo? me)]

Andrew, this looks very good. Can you make it such that only matched tags are shown?

Comment 70

[Matthew N. [:MattN]]

(In reply to Jared Wein [:jaws] from comment #69)
> Andrew, this looks very good. Can you make it such that only matched tags
> are shown?

This is the first I heard of that change and I think it may be confusing to a user if they don't see the other tags they've assigned to a page. That would also add more overhead on every keystroke since we'd have to re-filter the tag list.

Comment 71

[Jared Wein [:jaws] (please needinfo? me)]

Fair enough. I'll move that request to a follow-up bug for discussion there.

Comment 72

[Matthew N. [:MattN]]

Stephen, as we discussed on IRC we'll align the bookmark icon to the top as it is now and land these improvements for now until we figure out the 32px icon box design. ui-r? with the alignment change?

Comment 73

[Stephen Horlander [:shorlander]]

Comment on attachment 634747 [details]
v6 Screenshots on all platforms + high contrast Windows 7

(In reply to Matthew N. [:MattN] from comment #72)
> Stephen, as we discussed on IRC we'll align the bookmark icon to the top as
> it is now and land these improvements for now until we figure out the 32px
> icon box design. ui-r? with the alignment change?

Yes, thank you! :)

Comment 74

[Andrew Hurle [:ahurle]]

Attachment: v7 - Bookmark icon now in its old position on the top of results

This also fixes a bug I noticed on OSX and XP where switch to tab results are a slightly different height - especially apparent when you press shift to override the switch to tab results.

Since bookmark icons are in their old place, all those XUL changes are unnecessary and gone now.

Comment 75

[Dão Gottwald [:dao]]

Comment on attachment 637029 [details] [diff] [review]
v7 - Bookmark icon now in its old position on the top of results

wrong patch

Comment 76

[Andrew Hurle [:ahurle]]

Attachment: v7 - Bookmark icon now actually in its old position on the top of results

Disregard that other patch that is named with the same first two characters in my patch queue :)

Comment 77

[Dão Gottwald [:dao]]

Comment on attachment 637030 [details] [diff] [review]
v7 - Bookmark icon now actually in its old position on the top of results

>--- a/toolkit/themes/gnomestripe/global/autocomplete.css
>+++ b/toolkit/themes/gnomestripe/global/autocomplete.css

>+.ac-url-text > html|span.ac-emphasize-text,
>+.ac-action-text > html|span.ac-emphasize-text {
>+  box-shadow: none;
>+  background-color: rgba(221,72,20,0.1);
>+}

This is a Ubuntu-default-theme-specific color that isn't going to fit other themes.

Note that this needs review from a toolkit peer...

Comment 78

[u428464]

What about further visual changes ? This patch only changes text styling and on hover effect. Plans about new icons, switch to tab, tags, etc. ?

Comment 79

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 637030 [details] [diff] [review]
v7 - Bookmark icon now actually in its old position on the top of results

I'll still provide feedback on this patch if I have some, but Dao should give the final r+.

Comment 80

[Matthew N. [:MattN]]

(In reply to Guillaume C. [:ge3k0s] from comment #78)
> What about further visual changes ? This patch only changes text styling and
> on hover effect. Plans about new icons, switch to tab, tags, etc. ?

See the dependencies and blocked bugs.  All the items on the list at https://wiki.mozilla.org/Improve_display_of_location_bar_results#6._User_experience_design will be completed.

Comment 81

[u428464]

Thanks for the answer, but I wasn't talking about this. Several visual changes have been mentioned above and I would like to know what are plans about them (e.g. new star icon as seen on mockups, switch to tab, tags, etc.). These questions interest me.

(In reply to Andrew Hurle [:ahurle] from comment #39)
> There are a few lingering questions not answered by the mockup:
> 
> - As noted in comment 28, where are bookmark tags going?  In the screenshot,
> you can see that I've just left them where they were previously.
> 
> - What are Switch to Tab results going to look like?
> 
> - What are keyword search results going to look like?

Comment 82

[Andrew Hurle [:ahurle]]

Attachment: v8 - Use gray emphasis on Ubuntu and Aero

(In reply to Dão Gottwald [:dao] from comment #77)
> This is a Ubuntu-default-theme-specific color that isn't going to fit other
> themes.

Aero also suffers from this problem, since -moz-nativehyperlinktext can change and I'm just using blueish as the background color there.  I suppose the only reasonable[1] solution without fixing bug 538727 is to use the neutral gray like in the title emphasis, as shown in this patch.

I don't think it's necessary to change this in XP or OSX though.  We currently use #008800 for .ac-url-text in XP (not something I introduced), so we can keep the green emphasis there.  -moz-nativehyperlinktext is hardcoded on OSX[2], so we can keep using blueish there.

[1] Unreasonable solution: beckground-image: -moz-linear-gradient(to bottom, currentColor -700%, rgba(255, 255, 255, 0) 500%)

[2] https://mxr.mozilla.org/mozilla-central/source/widget/cocoa/nsLookAndFeel.mm#263

Comment 83

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 637299 [details] [diff] [review]
v8 - Use gray emphasis on Ubuntu and Aero

Review of attachment 637299 [details] [diff] [review]:
-----------------------------------------------------------------

The hover state for the switch-to-tab icon needs changed on Aero/AeroBasic - previously it was light icon on dark background, now it's light icon on light background. So it should be dark icon on light background.

The neutral grey is an acceptable solution, given the limitations, IMO.

Another followup bug would be to reduce the line between the urlbar and the autocomplete popup. It's 2px (bottom border of urlbar + top border of popup), which feels more noticeable with these changes.

Comment 84

[Dão Gottwald [:dao]]

Comment on attachment 637299 [details] [diff] [review]
v8 - Use gray emphasis on Ubuntu and Aero

> richlistitem[type~="action"][actiontype="switchtab"] > .ac-url-box > .ac-action-icon {
>   list-style-image: url("chrome://browser/skin/actionicon-tab.png");
>   -moz-image-region: rect(0, 16px, 16px, 0);
>   padding: 0 3px;
>+  /* Need this negative margin so the icon doesn't stretch its container */
>+  margin-top: -1px;
>+  margin-bottom: -1px;
> }

actionicon-tab.png contains transparent pixels at the top and at the bottom. Seems like we should just remove these.

Comment 85

[Andrew Hurle [:ahurle]]

Attachment: v9 - Dark tab icon on Aero, crop out transparent pixels in all actionicon-tab.png images

The switch-to-tab icon on aero on hover is now the same dark icon used in the non-hover state.  Non-default themes will still change to the lighter icon on hover.

I cropped the transparent pixels out in gnomestripe as well even though I didn't use the negative margin hack there, just for consistency.  I used "pngcrush -rem alla -brute" on the images - I think that's what I'm supposed to do...

It would be nice to get this landed in time for Firefox 16 along with the solution to bug 583683.

Comment 86

[Matthew N. [:MattN]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/141fd2c4581b

Comment 87

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/141fd2c4581b

Comment 88

[...]

Sorry but I can't help but notice on Aero the matched terms should be yellow, like the matched terms shown in Windows Explorer. Black and grey don't mix well as they're slightly harder to differentiate.
There should also be a hover state.
The styling also isn't applied to all richlists, such as the exceptions dialogs in the preferences window.

Also noticed in the past few years that the dropdown menus in the preferences also looks out of place. The dropdown box shouldn't turn blue after you click out of them. The dropdown results hovered text should be white with a blue background.

I should be filing new bugs but I have no idea how to categorise them, sorry.

Comment 89

[bogas04]

I second that , matched terms should have yellow highlight , to make it more native for Aero.

How it looks like in Windows: http://i.imgur.com/Iw8mD.jpg

Comment 90

[John Doe]

That would be a welcome change, but i would also like to point out another issue.
I think that the removal of the lines between the suggestions is a bad idea.
It's now much harder to see and find what you are looking for.
It also looks and feels really odd.

Comment 91

[John Drinkwater (:beta)]

This change also adjusts font sizes for the URL shown, was that intended? On my system the URL is much smaller than the title.

Comment 92

[Andrew Hurle [:ahurle]]

(In reply to John Drinkwater (:beta) from comment #91)
> This change also adjusts font sizes for the URL shown, was that intended? On
> my system the URL is much smaller than the title.

Yes, there were some intentional changes to font sizes.  If it looks different from the attached screenshot, feel free to file a bug for it with a screenshot of your own, but note that certain OS settings can change your font sizes.

Comment 93

[John Drinkwater (:beta)]

(In reply to Andrew Hurle [:ahurle] from comment #92)
> (In reply to John Drinkwater (:beta) from comment #91)
> > This change also adjusts font sizes for the URL shown, was that intended? On
> > my system the URL is much smaller than the title.
> 
> Yes, there were some intentional changes to font sizes.  If it looks
> different from the attached screenshot, feel free to file a bug for it with
> a screenshot of your own, but note that certain OS settings can change your
> font sizes.

Understandable, its not crazily important to me, but filed Bug 779489 anyhow.