No favicon displayed for baidu.com in Search bar panel after adding the engine

RESOLVED FIXED in Firefox 48

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: adalucinet, Assigned: florian)

Tracking

({regression})

Trunk
Firefox 48
Points:
---

Firefox Tracking Flags

(firefox42 wontfix, firefox44 wontfix, firefox45 affected, firefox46 wontfix, firefox48 fixed)

Details

(Whiteboard: [fxsearch])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

3 years ago
Posted image Screenshot
Reproducible with 44.0RC build 1 (Build ID: 20160118143821), latest Aurora 45.0a2 and Nightly 46.0a1 (both from 2016-01-18)
Affected platforms: Windows 7 x64, Windows 10 x86, Mac OS X 10.9.5 and Ubuntu 14.04 x86

Steps to reproduce:
1. Launch Firefox and navigate to http://www.baidu.com/
2. Add the search engine.

Expected result: A favicon is displayed in the Search bar panel.
Actual result: No favicon, not even the default one - magnifying glass, visible in the Search bar panel.

Additional notes:
1. Also reproducible with 42.0RC - not a recent regression.
2. Screenshot attached.
This is an issue on Baidu's side
http://www.baidu.com/content-search.xml

That's one strange searchplugin, and it' missing an image. 

IMO this bug as it is is invalid, it might make sense to morph it into displaying a default image when the searchplugin fails to provide one (assuming there isn't one already).
(Reporter)

Comment 2

3 years ago
The default icon for search engines started to show up for this particular website with Nightly from 2014-11-28 - pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cef590a6f946&tochange=17de0f463944

Regression range:
Last good: 2015-01-26 - note that with this build, the default placeholder icon was set instead of the logo
First bad: 2015-01-27
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=fa91879c8428&tochange=9b6b80222e66

Maybe bug 1106043 had something to do with this behaviour?
Has Regression Range: --- → yes
Keywords: regression
Andrei - Is 43 affected as well?
Flags: needinfo?(andrei.vaida)
(In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #3)
> Andrei - Is 43 affected as well?

Yes, 43 is also affected. I can't update the flags to reflect this, as there's no "status-firefox43" available (I'm not sure why, though).
Flags: needinfo?(andrei.vaida)
Not showing the default icon in this case sounds like a bug.
Flags: needinfo?(florian)
Whiteboard: [fxsearch]
(Assignee)

Comment 6

3 years ago
Posted patch WIP (obsolete) — Splinter Review
Baidu.com has an SVG icon (<link rel="icon" sizes="any" mask href="//www.baidu.com/img/baidu.svg">). The opensearch provider offered on the page doesn't contain an icon, so we fallback to using the page's icon. And the code converting the downloaded image to a data URL has the "image/x-icon" type hardcoded.

The attached WIP is enough to make this work on baidu.com.

Not requesting review (yet) because I would want to think a bit more about whether there are edge cases, and I think this deserves tests.
Flags: needinfo?(florian)
(In reply to Andrei Vaida, QA [:avaida] from comment #4)
> Yes, 43 is also affected. I can't update the flags to reflect this, as
> there's no "status-firefox43" available (I'm not sure why, though).

We remove the status flags after a release goes EOL. We may have done that slightly before the release with 43. Thank you for checking.
(Assignee)

Comment 8

3 years ago
Posted patch Patch v2Splinter Review
Now with a test.
Attachment #8727916 - Flags: review?(adw)
(Assignee)

Updated

3 years ago
Assignee: nobody → florian
Status: NEW → ASSIGNED
(Assignee)

Updated

3 years ago
Attachment #8712143 - Attachment is obsolete: true

Updated

3 years ago
Attachment #8727916 - Flags: review?(adw) → review+
(Assignee)

Comment 10

3 years ago
https://hg.mozilla.org/integration/fx-team/rev/f7ed458ec73dc42d58fda3d69b1036738d968833
Bug 1240729 - No favicon displayed in Search bar panel after adding an engine with an SVG icon, r=adw.

Comment 11

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f7ed458ec73d
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Could you request uplift to aurora and beta for this, if you think it would not be risky? It would be nice to fix this for 46.
Flags: needinfo?(florian)
(Assignee)

Comment 13

3 years ago
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #12)
> Could you request uplift to aurora and beta for this, if you think it would
> not be risky? It would be nice to fix this for 46.

The patch isn't really risky and has automated test coverage, so it would be OK to uplift I think, but I'm not sure why you are interested in uplifting this specific bug. If you are interested in quality around search plugin icons, bug 1241080 is similar in terms of impact, and not more risky, so I would suggest uplifting both or none.
Flags: needinfo?(florian)
I have reproduced this bug with Firefox Nightly 46.0a1 (Build ID: 20160119030232) on 
Windows 8.1, 64-bit.

Verified as fixed with Firefox Developer edition 48.0a2 (Build ID: 20160506004122)
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0
QA Whiteboard: [testday-20160506]
You need to log in before you can comment on or make changes to this bug.