Closed Bug 1518031 Opened 2 years ago Closed 2 years ago

Add to addressbar in the Page Actions menu gives a blank icon when you pin "Add Search Engine"

Categories

(Firefox :: Toolbars and Customization, defect, P2)

defect

Tracking

()

RESOLVED FIXED
Firefox 66
Tracking Status
firefox-esr60 --- unaffected
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- fixed

People

(Reporter: asa, Assigned: adw)

References

Details

(Keywords: regression)

Attachments

(1 file)

Tested on latest x86 Nightly build on Windows 10

If you try to pin "Add Search Engine" to the addressbar from the Page Actions menu, you get a blank icon. It functions but it's blank. 

Steps to reproduce
1. Visit ecosia.com or any other page with a search engine you haven't installed (Bugzilla works too)
2. Click the Page Actions menu
3. Right click on "Add Search Engine" and select "Add to Addressbar"

Results: It gets added to the addressbar with no icon

Expected: We get some kind of icon in the addressbar.
WFM on Mac (2019-01-06) - I get the favicon of the Ecosia page.

It works when I do it on the page having the search engine, if then I remove the added search engine and reload the page with the engine, I indeed get an empty space, as reported. Switching tab makes the icon appear. Something fishy.
Drew may know more about how this icon is set, maybe it's a timing problem, maybe the icon src is added when the button is not yet visible and we don't paint it.

Flags: needinfo?(adw)
Priority: -- → P2

The icon is set here: https://dxr.mozilla.org/mozilla-central/rev/c2593a3058afdfeaac5c990e18794ee8257afe99/browser/base/content/browser-pageActions.js#1124

It looks like this.engines[0].icon is null when _updateTitleAndIcon is called via updateOpenSearchBadge, at least sometimes. The stack I'm seeing when it's null is:

_updateTitleAndIcon@chrome://browser/content/browser-pageActions.js:1139:11
updateEngines@chrome://browser/content/browser-pageActions.js:1128:7
updateOpenSearchBadge@chrome://browser/content/browser.js:3966:5
addEngine@chrome://browser/content/browser.js:3956:9
addSearch@chrome://browser/content/browser.js:3745:5
receiveMessage@chrome://browser/content/browser.js:3675:9
MessageListener.receiveMessageinit@chrome://browser/content/browser.js:3650:5
onLoad@chrome://browser/content/browser.js:1325:5
EventHandlerNonNull
@chrome://browser/content/browser.xul:108:19

It's easy to reproduce: Visit a site that offers an engine, add the "Add Search Engine" action to the urlbar, and reload the page. That triggers the stack above.

It's possible this has always been a problem, but it might be a regression, I don't know.

It looks like the icon comes from browser.mIconURL: https://dxr.mozilla.org/mozilla-central/rev/c2593a3058afdfeaac5c990e18794ee8257afe99/browser/base/content/browser.js#3977 So we'd need to somehow wait until that becomes non-null and then set the action's icon.

This isn't a problem for the search bar because the icon is only shown once the user opens the search bar menu, and by that time the icon is likely non-null.

Moving this to Toolbars, where page action bugs live.

Component: Address Bar → Toolbars and Customization
Flags: needinfo?(adw)
We can just add `onLinkIconAvailable` to `TabsProgressListener` in browser.js and update the search badge.

This is likely another regression from bug 1453751. Previously the url for the favicon was sent to the browser very early in page load. Now we wait until we've actually loaded the icon's content which is probably after any search engines are detected.

Blocks: 1453751
Assignee: nobody → adw
Status: NEW → ASSIGNED
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8dbf2f4f7c53
Update "Add Search Engine" page action image when search engine icon becomes available. r=mossop
Keywords: regression
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66
You need to log in before you can comment on or make changes to this bug.