Improve the visual style of location bar results

RESOLVED FIXED in Firefox 16

Status

()

Firefox
Location Bar
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: faaborg, Assigned: ahurle)

Tracking

(Depends on: 3 bugs, Blocks: 2 bugs)

unspecified
Firefox 16
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments, 14 obsolete attachments)

989.38 KB, image/jpeg
Details
266.30 KB, image/png
shorlander
: ui-review+
Details
17.77 KB, patch
Unfocused
: review+
Details | Diff | Splinter Review
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
(Reporter)

Updated

7 years ago
Blocks: 579521
(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/
(Reporter)

Comment 2

7 years ago
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.
Depends on: 420237
Whiteboard: [target-betaN]
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

6 years ago
Do we have a mockup or a concrete list of changes for this? That would be a helpful guide for potential patch makers.
(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.
(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?
(Reporter)

Comment 7

6 years ago
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).

Updated

6 years ago
Depends on: 675818
Created attachment 551065 [details]
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
Solid blue hover looks ugly in Vista/7. We should use Aero hover.
> * 32x32px icons

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

Comment 11

6 years ago
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

6 years ago
(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.
>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.
(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.
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
Created attachment 554216 [details]
LocationBar Result Improvements - i02

Updated Windows 7 Design
Attachment #551065 - Attachment is obsolete: true
(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

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

Comment 19

6 years ago
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

6 years ago
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

6 years ago
What is the current status of this bug? Is anyone working on it? Can it be a good first bug or not really?
Well, Stylish style already exists, so it only needs changes. At least I think so.
(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.
Can be current user style used as a base for Windows or you'll have to start from scratch?

Comment 25

6 years ago
(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

6 years ago
>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

5 years ago
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)
Is there any plans about Tags in the new design of the location bar ?

Comment 29

5 years ago
(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.
(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

5 years ago
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.
(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

5 years ago
true story
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

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

Updated

5 years ago
Duplicate of this bug: 646008
(Assignee)

Updated

5 years ago
Assignee: nobody → fracture91
(Assignee)

Comment 37

5 years ago
Created attachment 628490 [details] [diff] [review]
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.
(Assignee)

Comment 38

5 years ago
Created attachment 628496 [details]
v1 Screenshot on Windows 7
(Assignee)

Comment 39

5 years ago
(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.
Status: NEW → ASSIGNED
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

5 years ago
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 :)
(Assignee)

Comment 42

5 years ago
(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.
Very nice work. I think Stephen Horlander may have the answers you need and maybe some newer or more complete mockups.
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.
(Assignee)

Updated

5 years ago
Depends on: 583683
(Assignee)

Comment 45

5 years ago
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.
(Assignee)

Comment 46

5 years ago
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.
Depends on: 634139
Bug 690191 could also be part of the redesign.
(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.
(Assignee)

Comment 49

5 years ago
(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.
Target Milestone: --- → Firefox 15
(Assignee)

Updated

5 years ago
Target Milestone: Firefox 15 → ---
(Assignee)

Updated

5 years ago
Attachment #628490 - Flags: feedback?(dao)
(Assignee)

Comment 50

5 years ago
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.
Attachment #628490 - Attachment is obsolete: true
Attachment #628490 - Flags: feedback?(dao)
Attachment #630389 - Flags: review?(dao)
(Assignee)

Updated

5 years ago
Attachment #630389 - Flags: review?(dao) → feedback?(dao)
(Assignee)

Comment 51

5 years ago
Created attachment 630391 [details]
v2 Screenshot on Windows 7
Attachment #628496 - Attachment is obsolete: true
Could it be pushed to UX ?
(Assignee)

Comment 53

5 years ago
Created attachment 632050 [details] [diff] [review]
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.
Attachment #630389 - Attachment is obsolete: true
Attachment #630389 - Flags: feedback?(dao)
Attachment #632050 - Flags: feedback?(jaws)
(Assignee)

Comment 54

5 years ago
Created attachment 632052 [details]
v3 Screenshots on all platforms
Attachment #630391 - Attachment is obsolete: true
(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?
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.
According to the mockup the location bar popup should have rounded corners.

Comment 58

5 years ago
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 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]
Attachment #632050 - Flags: feedback?(jaws) → feedback+
(Assignee)

Comment 60

5 years ago
(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.
(Assignee)

Comment 61

5 years ago
Created attachment 633757 [details] [diff] [review]
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.
Attachment #632050 - Attachment is obsolete: true
(Assignee)

Comment 62

5 years ago
Created attachment 633758 [details]
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.
Attachment #632052 - Attachment is obsolete: true
Attachment #633757 - Flags: review?(jaws)
Attachment #633758 - Flags: ui-review?(shorlander)
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.
(Assignee)

Comment 64

5 years ago
Created attachment 634187 [details] [diff] [review]
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.
Attachment #633757 - Attachment is obsolete: true
Attachment #633757 - Flags: review?(jaws)
Attachment #634187 - Flags: ui-review?(shorlander)
Attachment #634187 - Flags: review?(jaws)
I'll wait for Stephen's review before proceeding here.
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).
(Assignee)

Comment 67

5 years ago
Created attachment 634634 [details] [diff] [review]
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.
Attachment #634187 - Attachment is obsolete: true
Attachment #634187 - Flags: ui-review?(shorlander)
Attachment #634187 - Flags: review?(jaws)
(Assignee)

Comment 68

5 years ago
Created attachment 634747 [details]
v6 Screenshots on all platforms + high contrast Windows 7
Attachment #633758 - Attachment is obsolete: true
Attachment #633758 - Flags: ui-review?(shorlander)
Attachment #634747 - Flags: ui-review?(shorlander)
Andrew, this looks very good. Can you make it such that only matched tags are shown?
(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.
Fair enough. I'll move that request to a follow-up bug for discussion there.
Blocks: 766476
(Assignee)

Updated

5 years ago
Blocks: 768703
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 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! :)
Attachment #634747 - Flags: ui-review?(shorlander) → ui-review+
(Assignee)

Comment 74

5 years ago
Created attachment 637029 [details] [diff] [review]
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.
Attachment #634634 - Attachment is obsolete: true
Attachment #637029 - Flags: review?(jaws)
Comment on attachment 637029 [details] [diff] [review]
v7 - Bookmark icon now in its old position on the top of results

wrong patch
Attachment #637029 - Attachment is obsolete: true
Attachment #637029 - Flags: review?(jaws)
(Assignee)

Comment 76

5 years ago
Created attachment 637030 [details] [diff] [review]
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 :)
Attachment #637030 - Flags: review?(jaws)
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...
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 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+.
Attachment #637030 - Flags: review?(jaws) → review?(dao)
(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.
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?
(Assignee)

Comment 82

5 years ago
Created attachment 637299 [details] [diff] [review]
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
Attachment #637030 - Attachment is obsolete: true
Attachment #637030 - Flags: review?(dao)
Attachment #637299 - Flags: review?(dao)
(Assignee)

Updated

5 years ago
Attachment #637299 - Flags: review?(dao) → review?(jaws)
Attachment #637299 - Flags: review?(dao)
Attachment #637299 - Flags: review?(jaws) → review?(bmcbride)
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.
Attachment #637299 - Flags: review?(bmcbride) → review-
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.
Attachment #637299 - Flags: review?(dao)
(Assignee)

Comment 85

5 years ago
Created attachment 641721 [details] [diff] [review]
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.
Attachment #637299 - Attachment is obsolete: true
Attachment #641721 - Flags: review?(bmcbride)
Attachment #641721 - Flags: review?(bmcbride) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/141fd2c4581b
Flags: in-testsuite-
https://hg.mozilla.org/mozilla-central/rev/141fd2c4581b
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16

Comment 88

5 years ago
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

5 years ago
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
Depends on: 774118

Comment 90

5 years ago
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.

Updated

5 years ago
Whiteboard: [target-betaN]

Updated

5 years ago
Depends on: 774219
(Assignee)

Updated

5 years ago
Blocks: 774495
(Assignee)

Updated

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

Updated

5 years ago
Blocks: 686442
(Assignee)

Comment 92

5 years ago
(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.

Updated

5 years ago
Blocks: 406254
(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.

Updated

5 years ago
Depends on: 779489

Updated

4 years ago
Duplicate of this bug: 420237
You need to log in before you can comment on or make changes to this bug.