400682 - favicons displayed at full size in open location autocomplete and homepage (pref ui) autocomplete

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P3)

Description

[Dave Townsend [:mossop]]

1. Hide the url bar (fullscreen mode, drag it off the toolbar, whatever)
2. File - Open Location.
3. Start typing into the testbox in the dialog that appears.

The favicons in the autocomplete that pops up are displayed at full size rather than shrunk to 16x16 when appropriate. You need to have something like www.bbspot.com in your autocomplete to see it.

Comment 1

[(not reading, please use [e-mail] instead)]

confirming on Windows XP.

thanks for reporting this, Dave.

there are rules in browser.css to set the image to 16px x 16px, but perhaps they should be moved to autocomplete.css

Comment 2

[Mike Connor [:mconnor]]

bug 389273 helps here I bet.

Comment 3

[(not reading, please use [e-mail] instead)]

> bug 389273 helps here I bet.

I think this is really a case of some missing css, see comment #1.

Comment 4

[(not reading, please use [e-mail] instead)]

taking, while testing #405269, I noticed this appears in the homepage pref UI as well.

Note, mconnor is right (about dolske's fix helping), but if the favicon was less than a certain number of bytes (the threshhold before he converts and scales) but bigger than 16px x 16px, we'd still have this bug.

fix coming up.

Comment 5

[(not reading, please use [e-mail] instead)]

Attachment: screen shot of the problem #1

Comment 6

[(not reading, please use [e-mail] instead)]

Attachment: screen shot of the problem #2

Comment 7

[(not reading, please use [e-mail] instead)]

Attachment: wip patch

gavin, what do you think of this approach?

These rules are currently duplicated in browser.css, see http://lxr.mozilla.org/mozilla/source/browser/themes/winstripe/browser/browser.css#937

I could move all the url bar related css rules out of browser.css and into autocomplete.css (would url-autocomplete.css be a better name?) and leave behind the ones that deal with the search bar (suggesthint)

(note, pinstripe and gnomestripe changes are missing.)

Comment 8

[(not reading, please use [e-mail] instead)]

Attachment: better patch, per gavin over irc

note, the url bar autocomplete continues to work fine (tested by setting browser.urlbar.richResults to false)

note, switched from width to max-width, as these rules will affect the search bar autocomplete results, browser content autocomplete results, etc.

Comment 9

[(not reading, please use [e-mail] instead)]

oops, not a regression, since fx 2 didn't have favicons in these places.

Comment 10

[(not reading, please use [e-mail] instead)]

fixed.

for litmus, a good test case is http://www.nps.gov/alcatraz/, assuming that http://www.nps.gov/favicon.ico remains 32 x 32 px

Checking in toolkit/themes/pinstripe/global/autocomplete.css;
/cvsroot/mozilla/toolkit/themes/pinstripe/global/autocomplete.css,v  <--  autoco
mplete.css
new revision: 1.16; previous revision: 1.15
done
Checking in toolkit/themes/winstripe/global/autocomplete.css;
/cvsroot/mozilla/toolkit/themes/winstripe/global/autocomplete.css,v  <--  autoco
mplete.css
new revision: 1.24; previous revision: 1.23
done
Checking in browser/themes/gnomestripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/gnomestripe/browser/browser.css,v  <--  browser.
css
new revision: 1.145; previous revision: 1.144
done
Checking in browser/themes/pinstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/pinstripe/browser/browser.css,v  <--  browser.cs
s
new revision: 1.101; previous revision: 1.100
done
Checking in browser/themes/winstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/browser.css,v  <--  browser.cs
s
new revision: 1.141; previous revision: 1.140
done

Comment 11

[(not reading, please use [e-mail] instead)]

alex / kevin:  you will want to make similar changes to proto

Comment 12

[Justin Dolske [:Dolske]]

Late comments:

* Does Gnomestripe have this issue? The patch doesn't change toolkit/themes/gnomestripe/global/autocomplete.css (as with win/pingstripe), presumably because that file doesn't exist. So I guess it was already working?

* Why |max-width: 16|? Will this cause misalignments if an entry has a favicon less than 16 pixels wide?

Comment 13

[(not reading, please use [e-mail] instead)]

> * Does Gnomestripe have this issue? The patch doesn't change
> toolkit/themes/gnomestripe/global/autocomplete.css (as with win/pingstripe),
> presumably because that file doesn't exist. So I guess it was already working?

yes, gnomestripe doesn't have autocomplete.css.  it uses winstripe's autocomplete.css by default.  See http://lxr.mozilla.org/seamonkey/source/toolkit/themes/Makefile.in#49

> * Why |max-width: 16|? 

max-width, because these css rules will impact the search bar autocomplete results, the browser content autocomplete and other toolkit consumers.  if we use min-width, we'd now have an extra 16px gap.

> Will this cause misalignments if an entry has a favicon less than 16 pixels wide?

Good question!  I just tested, and we don't have this problem.  The 8x8 favicon that I created for testing purposes gets scaled up to 16 x 16 px in the various locations.

Let me figure out why.

I'll attach the test case to this bug.

Comment 14

[(not reading, please use [e-mail] instead)]

Attachment: 8x8 favicon for testing dolske's scenario

Comment 15

[(not reading, please use [e-mail] instead)]

Attachment: test case

Comment 16

[Stephen Donner [:stephend] Not actively reading bugmail]

https://litmus.mozilla.org/show_test.cgi?id=5040 (Open Location)
https://litmus.mozilla.org/show_test.cgi?id=5041 (Options / Preferences)

in-litmus+

Comment 17

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED with testcases in comment 16 on:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2008010204 Minefield/3.0b3pre

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2008010204 Minefield/3.0b3pre

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b3pre) Gecko/2008010104 Minefield/3.0b3pre