1463684 - Policy: Search engine added via SearchEngines.Add cannot receive suitably encoded search term for non-ASCII characters

Status: VERIFIED FIXED

(Firefox :: Enterprise Policies, defect, P2)

Description

[YUKI "Piro" Hiroshi]

Policy Engine now allows me to add search engine via policies.json (bug 1428948):

--------------
{
  "policies": {
    "SearchEngines": {
      "Add": [
        {
          "Name":               "DD",
          "Alias":              "dd",
          "Method":             "GET",
          "URLTemplate":        "https://duckduckgo.com/?q={searchTerms}"
        }
      ]
    }
  }
}
--------------

However, the search engine cannot receive effectively encoded search term if it contains any non-ASCII character.


Steps to reproduce:

 1. Save above example as a file "distribution/policies.json".
 2. Start Firefox.
 3. Focus to the location bar in the browser window.
 4. Type "dd 日本語" and hit Enter key.
    ("日本語" means "Japanese Language" in Japanese.)

Expected result:
Search results for the given term "日本語" is shown.

Actual result:
Search results for odd term "日本語" is shown.

Environment:
 * Firefox ESR60.0.1
 * Windows 10 April 2018 Update


Additional information:

Schema definition:
https://dxr.mozilla.org/mozilla-esr60/source/browser/components/enterprisepolicies/schemas/policies-schema.json#562
Logic to add new search engine based on the given policy:
https://dxr.mozilla.org/mozilla-esr60/source/browser/components/enterprisepolicies/Policies.jsm#649
Implementation of the method called to add new search engine:
https://dxr.mozilla.org/mozilla-esr60/source/toolkit/components/search/nsSearchService.js#3853
Interface definition of the method:
https://dxr.mozilla.org/mozilla-esr60/source/netwerk/base/nsIBrowserSearchService.idl#327

After I researched, I realized that there is no field to define encoding of the search term in the schema, method implementation, and the interface. And, if there is no effective encoding information, Firefox always tries to encode the given search term with CP1252:

https://dxr.mozilla.org/mozilla-central/source/toolkit/components/search/nsSearchService.js#2300

Thus I think we need to do two changes:

 1. Add new field defining encoding of the search term (and other parameters).
 2. Add new parameter to the method receiving encoding of the search term (and other parameters).

Comment 1

[YUKI "Piro" Hiroshi]

Oops, the last URL is wrong. Correct permalink to the code defining default encoding:
https://dxr.mozilla.org/mozilla-central/rev/b75acf9652937ce79a9bf02de843c100db0e5ec7/toolkit/components/search/nsSearchService.js#2267

Comment 2

[Mike Kaply [:mkaply]]

Attachment: Bug 1463684 - Specify UTF-8 charset for policy search engines.

Comment 3

[Mike Kaply [:mkaply]]

Attachment: Bug 1463684 - Specify UTF-8 charset for policy search engines.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f04a49448cf5
Specify UTF-8 charset for policy search engines. r=Felipe

Comment 5

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/f04a49448cf5

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Does this need to be uplifted?

Comment 7

[Mike Kaply [:mkaply]]

Comment on attachment 9015680 [details]
Bug 1463684 - Specify UTF-8 charset for policy search engines.

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: None

User impact if declined: Search policy doesn't work for i18n

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: No

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Just adding an additional parameter. Very low risk.

String changes made/needed:

Comment 8

[Pascal Chevrel:pascalc]

Comment on attachment 9015680 [details]
Bug 1463684 - Specify UTF-8 charset for policy search engines.

Oneliner, already on nightly, uplift approved for our last 63 beta.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/dc0694bc87c1

Comment 10

[Emil Ghitta, QA [:emilghitta]]

I have managed to reproduce this issue using Firefox 60.2.2esr (BuildId:20181001135620) on Windows 10 64bit.

This issue is verified fixed using Firefox 64.0a1 (BuildId:20181014223729) and Firefox 63.0b14 (BuildId:20181011200118) on Windows 10 64bit, macOS 10.11.6 and Ubuntu 16.04 64bit.

Comment 11

[Mike Kaply [:mkaply]]

Comment on attachment 9015680 [details]
Bug 1463684 - Specify UTF-8 charset for policy search engines.

[ESR Uplift Approval Request]

If this is not a sec:{high,crit} bug, please state case for ESR consideration: Simple change to fix a policy

User impact if declined: Some search engines don't work correctly.

Fix Landed on Version: Firefox 63/64

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): One line change to use encoding

String or UUID changes made by this patch:

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9015680 [details]
Bug 1463684 - Specify UTF-8 charset for policy search engines.

Verified for 63/64, let's uplift this enterprise policy fix to ESR60.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/f27badd7938f

Comment 14

[Emil Ghitta, QA [:emilghitta]]

This issue is verified fixed using Firefox 60.3.1esr (provided in comment 13) on Windows 10 64bit, macOS 10.11.6 and Ubuntu 16.04 64bit.