Closed Bug 1463684 Opened Last year Closed 11 months ago

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

Categories

(Firefox :: Enterprise Policies, defect, P2)

defect

Tracking

()

VERIFIED FIXED
Firefox 64
Tracking Status
firefox-esr60 --- verified
firefox62 --- wontfix
firefox63 --- verified
firefox64 --- verified

People

(Reporter: yuki, Assigned: mkaply)

Details

Attachments

(1 file, 1 obsolete file)

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).
Priority: -- → P1
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
Priority: P1 → P2
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Pushed by mozilla@kaply.com:
https://hg.mozilla.org/integration/autoland/rev/f04a49448cf5
Specify UTF-8 charset for policy search engines. r=Felipe
https://hg.mozilla.org/mozilla-central/rev/f04a49448cf5
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
Does this need to be uplifted?
Flags: needinfo?(mozilla)
Attachment #9016098 - Attachment is obsolete: true
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:
Flags: needinfo?(mozilla)
Attachment #9015680 - Flags: approval-mozilla-beta?
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.
Attachment #9015680 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
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:
Attachment #9015680 - Flags: approval-mozilla-esr60?
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.
Attachment #9015680 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
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.
You need to log in before you can comment on or make changes to this bug.