1471383 - IconURL not working with SearchEngines policy

Status: VERIFIED FIXED

(Firefox :: Enterprise Policies, defect, P1)

Description

[James Pearson]

Attachment: Console log when using CCK2

The IconURL setting doesn't appear to work with the SearchEngines Policy - using a policies.json containing something like:

{
  "policies": {
    "SearchEngines": {
      "Add": [
        {
          "IconURL": "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAB...CYII=",
          "Method": "GET",
          "Name": "Intranet Search",
          "URLTemplate": "http://intranet/search/getSearchForm?Search={searchTerms}"
        }
      ],
      "Default": "Intranet Search",
      "PreventInstalls": false
    }
  }
}

(IconURL base64 string reduced for brevity)

The new search engine is added, but uses a generic icon, not the one supplied

However, using an identical "data:..." URL in a Search Engine XML plugin with CCK2, and the supplied icon is used

Attached are two console logs with prefs:

 browser.search.log = true
 browser.policies.loglevel = debug

policies.log when using the above policies.json

cck2.log when using CCK2 with:

  "searchplugins": {
    "Intranet Search": "resource://mpc/searchplugins/intranet.xml",
  },
  "defaultSearchEngine": "Intranet Search",

The policies.log is missing the line like:

_setIcon: Setting icon url "data:image/png;base64,iVBORw0KG..." for engine "Intranet Search".

The _setIcon: log output comes from 'setIcon: function
SRCH_ENG_setIcon()' in toolkit/components/search/nsSearchService.js -
the code does:

_setIcon: function SRCH_ENG_setIcon(aIconURL, aIsPreferred, aWidth,
aHeight) {
     var uri = makeURI(aIconURL);

     // Ignore bad URIs
     if (!uri)
       return;

     LOG("_setIcon: Setting icon url \"" + limitURILength(uri.spec) +
"\" for engine \""
        + this.name + "\".");

So I'm guessing that it is failing at makeURI() ???

Comment 1

[James Pearson]

Attachment: Console log when using policies.json

Comment 2

[Mike Kaply [:mkaply]]

Weird. I just ran a quick check with the data URL from here:

https://searchfox.org/mozilla-central/source/browser/components/search/searchplugins/leit-is.xml

And it worked for me.

I wonder if it's a problem with using a png?

Would you be willing to paste the entire data URL so I can debug?

Comment 3

[James Pearson]

I can't get that data URL to work for me ...

My data URL is actually in the attached policies.log - but here it is:

data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAABmJLR0QA/wD/AP+gvaeTAAAACXBIWXMAAABIAAAASABGyWs+AAAACXZwQWcAAAAQAAAAEABcxq3DAAACCklEQVQ4y22T3WsTQRTFf3fmbrppk9om6TZGQetHH4qgoFgsKH3zLxZ8EPog+CKID2rTpqlU+pWEWrH2a3dnx4e4YdN64TC7szPn3NlzRgaDgQfw3o8AkGUZeYkIxhistQRBgLUW7z0iggZBcG1zcSwiSRKSJAHAWouqosaYkUpx01WCYkdZluG9J45jNE1TRGSEnCx/zqtI7JwbvWtROWfOz1fsrCiiqojIkOCw1+fkzynlckjUqLPf6yMIExMl4iQFPGotzfkIvGd374DTszMa9RqN2iy63+vT3tpmslxmdeU5Hz5+AqA+O8PR8S9ULc5ltJoRRgx7hz3KYcjnr+usriwPf6IxhovLS3YPeqi1pM6NWn+9+oov7Q1+7B3gvWdp8QGL9xdod7qcn19g0tRhrWFmusrm9nca9VruPgBv3q2xs7tPqxkBEIYTqLVEjTqVyiRqzHBhNNeg3ekyt1DjsD/A+6FtL5efMV2tUA5D3q69Z2t7h7PzCzY6XZ48WkKrlSluRnPcas7z++SE1nzE0c9jqpUpSqUSszemUVWcc7x4+phvm116/QEP793lzu0W0ul0vLWWHMaYaxYWs5Dbl5cCxHE8ln/vPcaYYVRVR8Q5eVFAS6XSWICuIo5j3D9XcmJrLSIyvA9BEIwpZ1mGiFzL/tXROYdzDg3DcCzGxdv4v7niNxHhL1B5Rgcpq92fAAAAJXRFWHRkYXRlOmNyZWF0ZQAyMDEzLTAxLTMxVDEyOjE4OjUzKzAwOjAwPxNnMgAAACV0RVh0ZGF0ZTptb2RpZnkAMjAxMy0wMS0zMVQxMjoxODo1MyswMDowME5O344AAAAASUVORK5CYII=

Could you let me have the policies.json file that works for you (in case I'm doing something wrong) ?

Comment 4

[Mike Kaply [:mkaply]]

Attachment: policies.json

I just tried with this policies.json file and I got the correct icon.

This has your data URL in it.

Comment 5

[James Pearson]

Attachment: screenshot of issue

Hmm, still can't get my Icon to work ...

If I paste the data URL into the address bar, it displays my icon fine

Just to be sure, I re-downloaded ESR 60.1.0 and did a fresh install and just copied in your policies.json file - but I still get generic Icons for my search engine

The attached screenshot shows what I get - I would expect to see my icon in the Search Box pull down and next to "Intranet Search" in the search preferences page

I'm using ESR 60.1.0 on Linux - I don't know if using Linux is significant?

Comment 6

[James Pearson]

Just tried it on Windows - and I get exactly the same issue ...

Comment 7

[James Pearson]

I've hacked in an extra LOG() statement to the function SRCH_ENG_setIcon() in components/nsSearchService.js to print the value of aIconURL - and for the built-in search engines it prints out the "data:..." URL - but for my search engine in prints "[xpconnect wrapped nsIURI]" ... but I'm not sure what that means?

Comment 8

[Mike Kaply [:mkaply]]

Did you do it before or after this line:

var uri = makeURI(aIconURL);?

Before this line, it should be a URL
After this line, you'd have to do uri.spec to see the URL.

If it's [xpconnect wrapped nsIURI] before that line, something definitely is wrong.

Comment 9

[James Pearson]

I added it before that line

Comment 10

[Mike Kaply [:mkaply]]

OK, I realized this all changed recently and I was testing nightly. I'll test on ESR.

Comment 11

[Mike Kaply [:mkaply]]

I apologize for the back and forth on this one. You are absolutely correct.

This is an esr only bug.

I'll make sure this is fixed just in case the other fix isn't backported.

Comment 12

[Mike Kaply [:mkaply]]

Attachment: Bug 1471383 - IconURL is a URI. Use spec.

Review commit: https://reviewboard.mozilla.org/r/253816/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/253816/

Comment 13

[:Felipe Gomes (needinfo for replies!)]

That is the patch for ESR, right?

On nightly and beta, it's a URL, so it should be .href instead of .spec.


I think we should make the patch for nightly, and then make sure that bug 1452533 and this bug are uplifted together.

Comment 14

[:Felipe Gomes (needinfo for replies!)]

We should probably add an icon to the test (browser_policy_search_engine.js) to make sure this doesn't regress

Comment 15

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8988575 [details]
Bug 1471383 - IconURL is a URI. Use spec.

(clearing the review request as per comment 13)

Comment 16

[Mike Kaply [:mkaply]]

Attachment: Bug 1471383 - Use iconURL.href and add test

Comment 17

[Phabricator Automation]

Comment on attachment 8991468 [details]
Bug 1471383 - Use iconURL.href and add test

:Felipe Gomes (needinfo me!) has approved the revision.

https://phabricator.services.mozilla.com/D2081

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7d06f2ac6273
Use iconURL.href and add test r=Felipe

Comment 19

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/7d06f2ac6273

Comment 20

[Emil Ghitta, QA [:emilghitta]]

This issue is verified fixed using Firefox 63.0b11 (BuildId:20181001131022) on Windows 10 64bit, macOS 10.13.6 and Ubuntu 16.04 64bit.