Closed Bug 1103119 Opened 6 years ago Closed 6 years ago

Search drop down fails to display properly if a search engine has no icon

Categories

(Firefox :: Search, defect)

x86
macOS
defect
Not set
normal
Points:
1

Tracking

()

VERIFIED FIXED
Firefox 36
Iteration:
37.1
Tracking Status
firefox34 + verified
firefox35 + verified
firefox36 + verified

People

(Reporter: mossop, Assigned: mossop)

References

Details

Attachments

(1 file)

No description provided.
Attached patch patchSplinter Review
Attachment #8526953 - Flags: review?(felipc)
Attachment #8526953 - Flags: review?(felipc) → review+
https://hg.mozilla.org/releases/mozilla-beta/rev/30ca4c851a16

[Tracking Requested - why for this release]:

This was causing some issues with tests I was writing after bug 1088660 landed.
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
Depends on: 1104221
QA Contact: petruta.rasa
Reproduced in Firefox 34 beta 11. Verified 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 on attachment 8526953 [details] [diff] [review]
patch

This landed on beta with a=gavin.
Attachment #8526953 - Flags: approval-mozilla-beta?
Attachment #8526953 - Flags: approval-mozilla-aurora?
Comment on attachment 8526953 [details] [diff] [review]
patch

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.
Attachment #8526953 - Flags: approval-mozilla-beta?
Attachment #8526953 - Flags: approval-mozilla-beta+
Attachment #8526953 - Flags: approval-mozilla-aurora?
Attachment #8526953 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/fcd31dc5cb15
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 36
Hi Dave, can you assign a point value.
Iteration: --- → 37.1
Flags: needinfo?(dtownsend+bugmail)
Flags: firefox-backlog+
Points: --- → 1
Flags: needinfo?(dtownsend+bugmail)
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.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.