Closed Bug 1104221 Opened 10 years ago Closed 10 years ago

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

Categories

(Firefox :: Search, defect)

34 Branch
defect
Not set
normal
Points:
1

Tracking

()

VERIFIED FIXED
Firefox 36
Iteration:
37.1
Tracking Status
firefox33 --- unaffected
firefox34 + verified
firefox35 + verified
firefox36 + verified
firefox37 + unaffected
firefox-esr31 --- unaffected

People

(Reporter: florian, Assigned: florian)

References

Details

Attachments

(1 file)

[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
Attached patch Fix (deleted) — Splinter Review
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.
Attachment #8527855 - Flags: review?(felipc)
Attachment #8527855 - Flags: approval-mozilla-beta?
Attachment #8527855 - Flags: review?(felipc) → review+
Attachment #8527855 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Contact: petruta.rasa
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.
Attachment #8527855 - Flags: approval-mozilla-aurora?
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).
Attachment #8527855 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 36
Hi Florian, can you assign a point value.
Iteration: --- → 37.1
Flags: qe-verify?
Flags: needinfo?(florian)
Flags: firefox-backlog+
Points: --- → 1
Flags: needinfo?(florian)
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.
Flags: qe-verify? → qe-verify+
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.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: