1104221 - Search drop down shows the icon of the previous default engine if the new default doesn't have an icon

Status: VERIFIED FIXED

(Firefox :: Search, defect)

Description

[Florian Quèze [:florian]]

[Tracking Requested - why for this release]:

The fix from bug 1103119 missed an edge case, as I pointed out in bug 1103119 comment 3:

> Comment on attachment 8526953 [details] [diff] [review]
> patch
> 
> Review of attachment 8526953 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/base/content/urlbarBindings.xml
> @@ +957,3 @@
> >            uri = uri.spec;
> > +          this.setAttribute("src", PlacesUtils.getImageURLForResolution(window, uri));
> > +        }
> 
> You need to handle the "else" case here. Either show a placeholder icon, or
> just remove the "src" attribute.
> 
> Your patch currently shows the icon of the previously-default provider after
> the user changed the default provider to a provider without icon.
> 
> Currently if you don't set the 'src' attribute, we fallback to the icon
> specified in some old CSS, which is inconsistent.
> 
> On Mac/Linux you'll get:
> http://mxr.mozilla.org/mozilla-beta/source/toolkit/themes/osx/mozapps/places/
> defaultFavicon.png
> On Windows you'll get the first icon of this image:
> http://mxr.mozilla.org/mozilla-beta/source/toolkit/themes/windows/global/
> icons/folder-item.png

Comment 1

[Florian Quèze [:florian]]

Attachment: Fix

I was tempted to also change the placeholders, but having 2 glass icons in the same searchbox would look odd on the new UI, so I decided to keep the patch very low risk and only change what I really wanted to get fixed for 34.

Comment 2

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8527855 [details] [diff] [review]
Fix

Beta+

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/7b937b21452d
https://hg.mozilla.org/releases/mozilla-release/rev/7b937b21452d

Comment 4

[Petruta Horea [:phorea], Desktop QA]

Verified as fixed using Firefox 34.0 RC build (20141124205320) under Win 7 64-bit, Ubuntu 14.04 32-bit and Mac OSX 10.9.5.

Comment 5

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 8527855 [details] [diff] [review]
Fix

Approving for Aurora since this has already landed on Beta, and it's a holiday so this is the last aurora approval round before the weekend (and merge-day).

Comment 6

[Florian Quèze [:florian]]

https://hg.mozilla.org/integration/fx-team/rev/e597f8f270c6

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e597f8f270c6

Comment 8

[Marco Mucci [:MarcoM]]

Hi Florian, can you assign a point value.

Comment 9

[Florian Quèze [:florian]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9664ebc4bf26

Comment 10

[Vasilica Tamas, QA [:vtamas]]

I was able to reproduce this issue on Firefox 34 Beta 11 (20141120192249) using Windows 7 x64.

Verified fixed on Firefox 35 Beta 1 (20141201162954) and Latest Firefox 36.0a2 (2014-12-02) using Windows 7 x64, Ubuntu 12.04 x32 and Mac OSX 10.9.5.

Comment 11

[Vasilica Tamas, QA [:vtamas]]

I was unable to reproduce this issue on Latest Firefox 37.0a1 (2014-12-02) using Windows 7 x64, Ubuntu 12.04 x32 and Mac OSX 10.9.5.