Bug 1492937 - Make the JS subscript loader load scripts exclusively a…

…s UTF-8, with no way to specify any other encoding, and adjust a bunch of existing tests to use UTF-8 directly, rather than Unicode escape sequences or similar. (This also changes the encoding of .sjs scripts and all mochitest-browser tests in the tree from Latin-1 to UTF-8.) r=yzen, r=MattN, r=jimb, r=kmag
dothq · Dec 19, 2018 · ed23151 · ed23151
1 parent d05b8f9
commit ed23151
Show file tree

Hide file tree

Showing 26 changed files with 52 additions and 72 deletions.
diff --git a/accessible/tests/browser/shared-head.js b/accessible/tests/browser/shared-head.js
@@ -215,7 +215,7 @@ function snippetToURL(snippet, bodyAttrs = {}) {
   let attrs = Object.assign({}, { id: "body" }, bodyAttrs);
   let attrsString = Object.entries(attrs).map(
     ([attr, value]) => `${attr}=${JSON.stringify(value)}`).join(" ");
-  let encodedDoc = btoa(
+  let encodedDoc = encodeURIComponent(
     `<html>
       <head>
         <meta charset="utf-8"/>
@@ -224,7 +224,7 @@ function snippetToURL(snippet, bodyAttrs = {}) {
       <body ${attrsString}>${snippet}</body>
     </html>`);
 
-  return `data:text/html;charset=utf-8;base64,${encodedDoc}`;
+  return `data:text/html;charset=utf-8,${encodedDoc}`;
 }
 
 /**

diff --git a/browser/base/content/test/urlbar/browser_urlbarCopying.js b/browser/base/content/test/urlbar/browser_urlbarCopying.js
@@ -5,14 +5,6 @@ const trimPref = "browser.urlbar.trimURLs";
 const phishyUserPassPref = "network.http.phishy-userpass-length";
 const decodeURLpref = "browser.urlbar.decodeURLsOnCopy";
 
-function toUnicode(input) {
-  let converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"]
-                    .createInstance(Ci.nsIScriptableUnicodeConverter);
-  converter.charset = "UTF-8";
-
-  return converter.ConvertToUnicode(input);
-}
-
 function test() {
 
   let tab = gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser);
@@ -139,16 +131,16 @@ var tests = [
   },
   { // Note: it seems BrowserTestUtils.loadURI fails for unicode domains
     loadURL: "http://sub2.xn--lt-uia.mochi.test:8888/foo",
-    expectedURL: toUnicode("sub2.ält.mochi.test:8888/foo"),
-    copyExpected: toUnicode("http://sub2.ält.mochi.test:8888/foo"),
+    expectedURL: "sub2.ält.mochi.test:8888/foo",
+    copyExpected: "http://sub2.ält.mochi.test:8888/foo",
   },
   {
-    copyVal: toUnicode("s<ub2.ält.mochi.test:8888/f>oo"),
-    copyExpected: toUnicode("ub2.ält.mochi.test:8888/f"),
+    copyVal: "s<ub2.ält.mochi.test:8888/f>oo",
+    copyExpected: "ub2.ält.mochi.test:8888/f",
   },
   {
-    copyVal: toUnicode("<sub2.ält.mochi.test:8888/f>oo"),
-    copyExpected: toUnicode("http://sub2.ält.mochi.test:8888/f"),
+    copyVal: "<sub2.ält.mochi.test:8888/f>oo",
+    copyExpected: "http://sub2.ält.mochi.test:8888/f",
   },
 
   {
@@ -171,7 +163,7 @@ var tests = [
   },
   {
     loadURL: "http://example.com/a%E3%80%80test",
-    expectedURL: toUnicode("example.com/a　test"),
+    expectedURL: "example.com/a\u{3000}test",
     copyExpected: "http://example.com/a%E3%80%80test",
   },
   {
@@ -217,12 +209,12 @@ var tests = [
   {
     setup() { Services.prefs.setBoolPref(decodeURLpref, true); },
     loadURL: "http://example.com/%D0%B1%D0%B8%D0%BE%D0%B3%D1%80%D0%B0%D1%84%D0%B8%D1%8F",
-    expectedURL: toUnicode("example.com/биография"),
-    copyExpected: toUnicode("http://example.com/биография"),
+    expectedURL: "example.com/биография",
+    copyExpected: "http://example.com/биография",
   },
   {
-    copyVal: toUnicode("<example.com/би>ография"),
-    copyExpected: toUnicode("http://example.com/би"),
+    copyVal: "<example.com/би>ография",
+    copyExpected: "http://example.com/би",
   },
 ];
 

diff --git a/browser/components/newtab/aboutNewTabService.js b/browser/components/newtab/aboutNewTabService.js
@@ -163,7 +163,7 @@ AboutNewTabService.prototype = {
           }
 
           for (let script of scripts) {
-            Services.scriptloader.loadSubScript(script, win, "UTF-8"); // Synchronous call
+            Services.scriptloader.loadSubScript(script, win); // Synchronous call
           }
         };
         subject.addEventListener("DOMContentLoaded", onLoaded, {once: true});

diff --git a/browser/extensions/formautofill/FormAutofillHeuristics.jsm b/browser/extensions/formautofill/FormAutofillHeuristics.jsm
@@ -1090,7 +1090,7 @@ this.FormAutofillHeuristics = {
 XPCOMUtils.defineLazyGetter(this.FormAutofillHeuristics, "RULES", () => {
   let sandbox = {};
   const HEURISTICS_REGEXP = "chrome://formautofill/content/heuristicsRegexp.js";
-  Services.scriptloader.loadSubScript(HEURISTICS_REGEXP, sandbox, "utf-8");
+  Services.scriptloader.loadSubScript(HEURISTICS_REGEXP, sandbox);
   return sandbox.HeuristicsRegExp.RULES;
 });
 

diff --git a/browser/extensions/formautofill/FormAutofillUtils.jsm b/browser/extensions/formautofill/FormAutofillUtils.jsm
@@ -396,7 +396,7 @@ this.FormAutofillUtils = {
   },
 
   loadDataFromScript(url, sandbox = {}) {
-    Services.scriptloader.loadSubScript(url, sandbox, "utf-8");
+    Services.scriptloader.loadSubScript(url, sandbox);
     return sandbox;
   },
 

diff --git a/devtools/client/debugger/new/test/mochitest/browser_dbg-outline.js b/devtools/client/debugger/new/test/mochitest/browser_dbg-outline.js
@@ -51,9 +51,6 @@ add_task(async function() {
     dbg,
     ".outline-list__element .function-signature"
   );
-  is(
-    firstAlphaFunction.innerText.replace("λ", ""),
-    "doEval()",
-    "Alphabetized first function is correct"
-  );
+  is(firstAlphaFunction.innerText, "doEval()",
+     "Alphabetized first function is correct");
 });
diff --git a/devtools/client/jsonview/test/browser_jsonview_encoding.js b/devtools/client/jsonview/test/browser_jsonview_encoding.js
@@ -33,16 +33,16 @@ add_task(async function() {
       output: "a\u0000",
     }, {
       input: "%30%FF",
-      output: "0\uFFFD", // 0�
+      output: "0�",
     }, {
       input: "%C3%A0",
-      output: "\u00E0", // à
+      output: "à",
     }, {
       input: "%E2%9D%A4",
-      output: "\u2764", // ❤
+      output: "❤",
     }, {
       input: "%F0%9F%9A%80",
-      output: "\uD83D\uDE80", // 🚀
+      output: "🚀",
     },
   ];
 

diff --git a/devtools/client/jsonview/test/browser_jsonview_ignore_charset.js b/devtools/client/jsonview/test/browser_jsonview_ignore_charset.js
@@ -9,7 +9,7 @@ add_task(async function() {
   info("Test ignored charset parameter started");
 
   const encodedChar = "%E2%9D%A4"; // In UTF-8 this is a heavy black heart
-  const result = "\u2764"; // ❤
+  const result = "❤";
   const TEST_JSON_URL = "data:application/json;charset=ANSI," + encodedChar;
 
   await addJsonViewTab(TEST_JSON_URL);

diff --git a/devtools/client/netmonitor/test/browser_net_curl-utils.js b/devtools/client/netmonitor/test/browser_net_curl-utils.js
@@ -209,7 +209,8 @@ function testEscapeStringPosix() {
   is(CurlUtils.escapeStringPosix(controlChars), "$'\\x07 \\x09 \\x0c \\x1b'",
     "Control characters should be escaped.");
 
-  const extendedAsciiChars = "æ ø ü ß ö é";
+  // æ ø ü ß ö é
+  const extendedAsciiChars = "\xc3\xa6 \xc3\xb8 \xc3\xbc \xc3\x9f \xc3\xb6 \xc3\xa9";
   is(CurlUtils.escapeStringPosix(extendedAsciiChars),
     "$'\\xc3\\xa6 \\xc3\\xb8 \\xc3\\xbc \\xc3\\x9f \\xc3\\xb6 \\xc3\\xa9'",
     "Character codes outside of the decimal range 32 - 126 should be escaped.");

diff --git a/devtools/client/netmonitor/test/browser_net_json-nogrip.js b/devtools/client/netmonitor/test/browser_net_json-nogrip.js
@@ -30,9 +30,8 @@ add_task(async function() {
   const values = tabpanel
     .querySelectorAll("tr:not(.tree-section) .treeValueCell .objectBox");
 
-  // Verify that an object is rendered: `obj: {…}`
   is(labels[0].textContent, "obj", "The first json property name is correct.");
-  is(values[0].textContent, "{\u2026}", "The first json property value is correct.");
+  is(values[0].textContent, "{…}", "The first json property value is correct.");
 
   await teardown(monitor);
 });
diff --git a/devtools/client/sourceeditor/editor.js b/devtools/client/sourceeditor/editor.js
@@ -296,7 +296,7 @@ Editor.prototype = {
 
         Services.scriptloader.loadSubScript(
           "chrome://devtools/content/shared/theme-switching.js",
-          win, "utf8"
+          win
         );
         this.container = env;
         this._setup(win.document.body, el.ownerDocument);
@@ -329,7 +329,7 @@ Editor.prototype = {
     const scriptsToInject = CM_SCRIPTS.concat(this.config.externalScripts);
     scriptsToInject.forEach(url => {
       if (url.startsWith("chrome://")) {
-        Services.scriptloader.loadSubScript(url, win, "utf8");
+        Services.scriptloader.loadSubScript(url, win);
       }
     });
 
@@ -478,7 +478,7 @@ Editor.prototype = {
       throw new Error("Can't load a script until the editor is loaded.");
     }
     const win = this.container.contentWindow.wrappedJSObject;
-    Services.scriptloader.loadSubScript(url, win, "utf8");
+    Services.scriptloader.loadSubScript(url, win);
   },
 
   /**

diff --git a/devtools/client/webconsole/test/mochitest/browser_webconsole_object_inspector_getters.js b/devtools/client/webconsole/test/mochitest/browser_webconsole_object_inspector_getters.js
@@ -210,9 +210,7 @@ async function testMapGetter(oi) {
   const entriesNode = findObjectInspectorNode(oi, "<entries>");
   expandObjectInspectorNode(entriesNode);
   await waitFor(() => getObjectInspectorChildrenNodes(entriesNode).length > 0);
-  // "→" character. Need to be instantiated this way for the test to pass.
-  const arrow = String.fromCodePoint(8594);
-  checkChildren(entriesNode, [`foo ${arrow} Object { ${ELLIPSIS} }`]);
+  checkChildren(entriesNode, [`foo → Object { ${ELLIPSIS} }`]);
 
   const entryNode = getObjectInspectorChildrenNodes(entriesNode)[0];
   expandObjectInspectorNode(entryNode);

diff --git a/devtools/shared/base-loader.js b/devtools/shared/base-loader.js
@@ -182,7 +182,7 @@ function load(loader, module) {
 
   const originalExports = module.exports;
   try {
-    Services.scriptloader.loadSubScript(module.uri, sandbox, "UTF-8");
+    Services.scriptloader.loadSubScript(module.uri, sandbox);
   } catch (error) {
     // loadSubScript sometime throws string errors, which includes no stack.
     // At least provide the current stack by re-throwing a real Error object.

diff --git a/devtools/shared/worker/loader.js b/devtools/shared/worker/loader.js
@@ -480,7 +480,7 @@ var {
                  .getService(Ci.mozIJSSubScriptLoader);
 
     const loadSubScript = function(url, sandbox) {
-      subScriptLoader.loadSubScript(url, sandbox, "UTF-8");
+      subScriptLoader.loadSubScript(url, sandbox);
     };
 
     const reportError = Cu.reportError;

diff --git a/dom/webauthn/tests/browser/head.js b/dom/webauthn/tests/browser/head.js
@@ -18,7 +18,7 @@ const scripts = [
 for (let script of scripts) {
   Services.scriptloader.loadSubScript(
     `chrome://mochitests/content/browser/dom/webauthn/tests/${script}`,
-    this, "utf-8");
+    this);
 }
 
 function memcmp(x, y) {

diff --git a/js/xpconnect/idl/mozIJSSubScriptLoader.idl b/js/xpconnect/idl/mozIJSSubScriptLoader.idl
@@ -17,17 +17,14 @@ interface mozIJSSubScriptLoader : nsISupports
      * This method should only be called from JS!
      * In JS, the signature looks like:
      * rv loadSubScript (url [, obj] [, charset]);
-     * @param url the url of the sub-script, it MUST be either a file:,
-     *            resource:, blob:, or chrome: url, and MUST be local.
+     * @param url the url of the UTF-8-encoded sub-script, it MUST be either a
+     *            file:, resource:, blob:, or chrome: url, and MUST be local.
      * @param obj an optional object to evaluate the script onto, it
      *            defaults to the global object of the caller.
-     * @param charset optionally specifies the character encoding of
-     *                the file. If absent, the file is interpreted
-     *                as ASCII.
      * @retval rv the value returned by the sub-script
      */
     [implicit_jscontext]
-    jsval loadSubScript(in AString url, [optional] in jsval obj, [optional] in AString charset);
+    jsval loadSubScript(in AString url, [optional] in jsval obj);
 
     /**
      * This method should only be called from JS!

diff --git a/js/xpconnect/loader/mozJSSubScriptLoader.cpp b/js/xpconnect/loader/mozJSSubScriptLoader.cpp
@@ -526,21 +526,19 @@ bool mozJSSubScriptLoader::ReadScript(nsIURI* uri, JSContext* cx,
 
 NS_IMETHODIMP
 mozJSSubScriptLoader::LoadSubScript(const nsAString& url, HandleValue target,
-                                    const nsAString& charset, JSContext* cx,
-                                    MutableHandleValue retval) {
+                                    JSContext* cx, MutableHandleValue retval) {
   /*
-   * Loads a local url and evals it into the current cx
-   * Synchronous (an async version would be cool too.)
+   * Loads a local url, referring to UTF-8-encoded data, and evals it into the
+   * current cx.  Synchronous (an async version would be cool too.)
    *   url: The url to load.  Must be local so that it can be loaded
    *        synchronously.
    *   targetObj: Optional object to eval the script onto (defaults to context
    *              global)
-   *   charset: Optional character set to use for reading
    *   returns: Whatever jsval the script pointed to by the url returns.
    * Should ONLY (O N L Y !) be called from JavaScript code.
    */
   LoadSubScriptOptions options(cx);
-  options.charset = charset;
+  options.charset.AssignLiteral("UTF-8");
   options.target = target.isObject() ? &target.toObject() : nullptr;
   return DoLoadSubScriptWithOptions(url, options, cx, retval);
 }

diff --git a/js/xpconnect/tests/chrome/test_bug1092477.xul b/js/xpconnect/tests/chrome/test_bug1092477.xul
@@ -29,7 +29,7 @@ try {
 catch (e) {
     exn = String(e);
 }
-var msg = "loadSubscript should throw an exception for trying to load a non-existent script"
+var msg = "loadSubScript should throw an exception for trying to load a non-existent script"
 is(exn, "Error opening input stream (invalid filename?): " + url, msg);
   ]]></script>
 </window>
diff --git a/js/xpconnect/tests/chrome/test_chrometoSource.xul b/js/xpconnect/tests/chrome/test_chrometoSource.xul
@@ -46,7 +46,7 @@ src = ns.NetUtil.asyncFetch.toSource();
 isnot(src.indexOf("return"), -1, "subscript of a subscript should have source");
 
 ns = {};
-Services.scriptloader.loadSubScript(resolvedBase + "utf8_subscript.js", ns, "UTF-8");
+Services.scriptloader.loadSubScript(resolvedBase + "utf8_subscript.js", ns);
 src = ns.f.toSource();
 isnot(src.indexOf("return 42;"), -1, "encoded subscript should have correct source");
 

diff --git a/netwerk/test/httpserver/httpd.js b/netwerk/test/httpserver/httpd.js
@@ -2529,7 +2529,7 @@ ServerHandler.prototype =
           // separate these two lines!
           var line = new Error().lineNumber;
           let uri = Services.io.newFileURI(file);
-          Services.scriptloader.loadSubScript(uri.spec, s, "UTF-8");
+          Services.scriptloader.loadSubScript(uri.spec, s);
         } catch (e) {
           dumpn("*** syntax error in SJS at " + file.path + ": " + e);
           throw HTTP_500;

diff --git a/netwerk/test/unit/test_psl.js b/netwerk/test/unit/test_psl.js
@@ -13,7 +13,7 @@ function run_test()
   var scriptLoader = Cc["@mozilla.org/moz/jssubscript-loader;1"]
                        .getService(Ci.mozIJSSubScriptLoader);
   var srvScope = {};
-  scriptLoader.loadSubScript(uri.spec, srvScope, "utf-8");
+  scriptLoader.loadSubScript(uri.spec, srvScope);
 }
 
 function checkPublicSuffix(host, expectedSuffix)

diff --git a/security/manager/ssl/tests/mochitest/browser/browser_clientAuth_ui.js b/security/manager/ssl/tests/mochitest/browser/browser_clientAuth_ui.js
@@ -59,13 +59,11 @@ function checkDialogContents(win, notBefore, notAfter) {
   Assert.equal(win.document.getElementById("hostname").textContent,
                `${TEST_HOSTNAME}:${TEST_PORT}`,
                "Actual and expected hostname and port should be equal");
-  // “ and ” don't seem to work when embedded in the following literals, which
-  // is why escape codes are used instead.
   Assert.equal(win.document.getElementById("organization").textContent,
-               `Organization: \u201C${TEST_ORG}\u201D`,
+               `Organization: “${TEST_ORG}”`,
                "Actual and expected organization should be equal");
   Assert.equal(win.document.getElementById("issuer").textContent,
-               `Issued Under: \u201C${TEST_ISSUER_ORG}\u201D`,
+               `Issued Under: “${TEST_ISSUER_ORG}”`,
                "Actual and expected issuer organization should be equal");
 
   Assert.equal(win.document.getElementById("nicknames").label,

diff --git a/testing/mochitest/browser-test.js b/testing/mochitest/browser-test.js
@@ -439,10 +439,10 @@ function Tester(aTests, structuredLogger, aCallback) {
       configurable: true,
       writable: true,
       value: {
-        loadSubScript: (url, obj, charset) => {
+        loadSubScript: (url, obj) => {
           let before = Object.keys(window);
           try {
-            return this._scriptLoader.loadSubScript(url, obj, charset);
+            return this._scriptLoader.loadSubScript(url, obj);
           } finally {
             for (let property of Object.keys(window)) {
               if (!before.includes(property) && !this._globalProperties.includes(property)) {

diff --git a/toolkit/actors/UAWidgetsChild.jsm b/toolkit/actors/UAWidgetsChild.jsm
@@ -86,7 +86,7 @@ class UAWidgetsChild extends ActorChild {
       Object.create(null) : Cu.getUAWidgetScope(aElement.nodePrincipal);
 
     if (!sandbox[widgetName]) {
-      Services.scriptloader.loadSubScript(uri, sandbox, "UTF-8");
+      Services.scriptloader.loadSubScript(uri, sandbox);
     }
 
     let widget = new sandbox[widgetName](shadowRoot);

diff --git a/toolkit/components/extensions/ExtensionCommon.jsm b/toolkit/components/extensions/ExtensionCommon.jsm
@@ -1509,7 +1509,7 @@ class SchemaAPIManager extends EventEmitter {
 
     this.initGlobal();
 
-    Services.scriptloader.loadSubScript(module.url, this.global, "UTF-8");
+    Services.scriptloader.loadSubScript(module.url, this.global);
 
     module.loaded = true;
 
@@ -1661,7 +1661,7 @@ class SchemaAPIManager extends EventEmitter {
     // in the sandbox's context instead of here.
     let scope = Cu.createObjectIn(this.global);
 
-    Services.scriptloader.loadSubScript(scriptUrl, scope, "UTF-8");
+    Services.scriptloader.loadSubScript(scriptUrl, scope);
 
     // Save the scope to avoid it being garbage collected.
     this._scriptScopes.push(scope);

diff --git a/toolkit/components/extensions/ProxyScriptContext.jsm b/toolkit/components/extensions/ProxyScriptContext.jsm
@@ -384,7 +384,7 @@ class ProxyScriptContext extends BaseContext {
     Schemas.exportLazyGetter(this.sandbox, "browser", () => this.browserObj);
 
     try {
-      Services.scriptloader.loadSubScript(this.url, this.sandbox, "UTF-8");
+      Services.scriptloader.loadSubScript(this.url, this.sandbox);
     } catch (error) {
       this.extension.emit("proxy-error", {
         message: this.normalizeError(error).message,