Test html script ordering (#1396)

* Simplify getAllFiles

* Add test to validate script order

* Update script include order

* Update content script order

* Update sw.js and corresponding lint

* Update manifest

5 files changed, 117 insertions, 25 deletions

diff --git a/dev/build.js b/dev/build.js
index 9cbbd518..55358e59 100644
--- a/dev/build.js
+++ b/dev/build.js
@@ -62,7 +62,7 @@ async function createZip(directory, excludeFiles, outputFileName, sevenZipExes,
 
 async function createJSZip(directory, excludeFiles, outputFileName, onUpdate, dryRun) {
     const JSZip = util.JSZip;
-    const files = getAllFiles(directory, directory);
+    const files = getAllFiles(directory);
     removeItemsFromArray(files, excludeFiles);
     const zip = new JSZip();
     for (const fileName of files) {
diff --git a/dev/data/manifest-variants.json b/dev/data/manifest-variants.json
index 178f1395..3b50b0f3 100644
--- a/dev/data/manifest-variants.json
+++ b/dev/data/manifest-variants.json
@@ -41,23 +41,23 @@
                 "js": [
                     "js/core.js",
                     "js/yomichan.js",
-                    "js/comm/cross-frame-api.js",
-                    "js/comm/api.js",
-                    "js/script/dynamic-loader.js",
-                    "js/comm/frame-client.js",
-                    "js/language/text-scanner.js",
-                    "js/dom/document-util.js",
-                    "js/input/hotkey-handler.js",
-                    "js/dom/dom-text-scanner.js",
+                    "js/app/frontend.js",
                     "js/app/popup.js",
-                    "js/dom/text-source-range.js",
-                    "js/dom/text-source-element.js",
                     "js/app/popup-factory.js",
-                    "js/comm/frame-ancestry-handler.js",
-                    "js/comm/frame-offset-forwarder.js",
                     "js/app/popup-proxy.js",
                     "js/app/popup-window.js",
-                    "js/app/frontend.js",
+                    "js/comm/api.js",
+                    "js/comm/cross-frame-api.js",
+                    "js/comm/frame-ancestry-handler.js",
+                    "js/comm/frame-client.js",
+                    "js/comm/frame-offset-forwarder.js",
+                    "js/dom/dom-text-scanner.js",
+                    "js/dom/document-util.js",
+                    "js/dom/text-source-element.js",
+                    "js/dom/text-source-range.js",
+                    "js/input/hotkey-handler.js",
+                    "js/language/text-scanner.js",
+                    "js/script/dynamic-loader.js",
                     "js/app/content-script-main.js"
                 ],
                 "match_about_blank": true,
diff --git a/dev/lint/global-declarations.js b/dev/lint/global-declarations.js
index 61739c42..914d1266 100644
--- a/dev/lint/global-declarations.js
+++ b/dev/lint/global-declarations.js
@@ -117,10 +117,10 @@ function main() {
     const fix = (process.argv.length >= 2 && process.argv[2] === '--fix');
     const directory = path.resolve(__dirname, '..', '..', 'ext');
     const pattern = /\.js$/;
-    const ignorePattern = /[\\/]ext[\\/]lib[\\/]/;
-    const fileNames = getAllFiles(directory, null, (f) => pattern.test(f) && !ignorePattern.test(f));
+    const ignorePattern = /^lib[\\/]/;
+    const fileNames = getAllFiles(directory, (f) => pattern.test(f) && !ignorePattern.test(f));
     for (const fileName of fileNames) {
-        if (!validateGlobals(fileName, fix)) {
+        if (!validateGlobals(path.join(directory, fileName), fix)) {
             process.exit(-1);
             return;
         }
diff --git a/dev/lint/html-scripts.js b/dev/lint/html-scripts.js
index dec8a3dd..41263d96 100644
--- a/dev/lint/html-scripts.js
+++ b/dev/lint/html-scripts.js
@@ -41,18 +41,110 @@ function validatePath(src, fileName, extDir) {
     assert.ok(stats.isFile(), `<script> src file invalid in ${fileName} (src=${JSON.stringify(src)})`);
 }
 
+function getSubstringCount(string, pattern) {
+    let count = 0;
+    while (true) {
+        const match = pattern.exec(string);
+        if (match === null) { break; }
+        ++count;
+    }
+    return count;
+}
+
+function getSortedScriptPaths(scriptPaths) {
+    // Sort file names without the extension
+    const extensionPattern = /\.[^.]*$/;
+    scriptPaths = scriptPaths.map((value) => {
+        const match = extensionPattern.exec(value);
+        let ext = '';
+        if (match !== null) {
+            ext = match[0];
+            value = value.substring(0, value.length - ext.length);
+        }
+        return {value, ext};
+    });
+
+    const stringComparer = new Intl.Collator('en-US'); // Invariant locale
+    scriptPaths.sort((a, b) => stringComparer.compare(a.value, b.value));
+
+    scriptPaths = scriptPaths.map(({value, ext}) => `${value}${ext}`);
+    return scriptPaths;
+}
+
+function validateScriptOrder(fileName, window) {
+    const {document, Node: {ELEMENT_NODE, TEXT_NODE}, NodeFilter} = window;
+
+    const scriptElements = document.querySelectorAll('script');
+    if (scriptElements.length === 0) { return; }
+
+    // Assert all scripts are siblings
+    const scriptContainerElement = scriptElements[0].parentNode;
+    for (const element of scriptElements) {
+        if (element.parentNode !== scriptContainerElement) {
+            assert.fail('All script nodes are not contained within the same element');
+        }
+    }
+
+    // Get script groupings and order
+    const scriptGroups = [];
+    const newlinePattern = /\n/g;
+    let separatingText = '';
+    const walker = document.createTreeWalker(scriptContainerElement, NodeFilter.SHOW_ELEMENT | NodeFilter.SHOW_TEXT);
+    walker.firstChild();
+    for (let node = walker.currentNode; node !== null; node = walker.nextSibling()) {
+        switch (node.nodeType) {
+            case ELEMENT_NODE:
+                if (node.tagName.toLowerCase() === 'script') {
+                    let scriptGroup;
+                    if (scriptGroups.length === 0 || getSubstringCount(separatingText, newlinePattern) >= 2) {
+                        scriptGroup = [];
+                        scriptGroups.push(scriptGroup);
+                    } else {
+                        scriptGroup = scriptGroups[scriptGroups.length - 1];
+                    }
+                    scriptGroup.push(node.src);
+                    separatingText = '';
+                }
+                break;
+            case TEXT_NODE:
+                separatingText += node.nodeValue;
+                break;
+        }
+    }
+
+    // Ensure core.js is first (if it is present)
+    const ignorePattern = /^\/lib\//;
+    const index = scriptGroups.flat()
+        .filter((value) => !ignorePattern.test(value))
+        .findIndex((value) => (value === '/js/core.js'));
+    assert.ok(index <= 0, 'core.js is not the first included script');
+
+    // Check script order
+    for (let i = 0, ii = scriptGroups.length; i < ii; ++i) {
+        const scriptGroup = scriptGroups[i];
+        try {
+            assert.deepStrictEqual(scriptGroup, getSortedScriptPaths(scriptGroup));
+        } catch (e) {
+            console.error(`Script order for group ${i + 1} in file ${fileName} is not correct:`);
+            throw e;
+        }
+    }
+}
+
 function validateHtmlScripts(fileName, extDir) {
-    const domSource = fs.readFileSync(fileName, {encoding: 'utf8'});
+    const fullFileName = path.join(extDir, fileName);
+    const domSource = fs.readFileSync(fullFileName, {encoding: 'utf8'});
     const dom = new JSDOM(domSource);
     const {window} = dom;
     const {document} = window;
     try {
         for (const {src} of document.querySelectorAll('script')) {
-            validatePath(src, fileName, extDir);
+            validatePath(src, fullFileName, extDir);
         }
         for (const {href} of document.querySelectorAll('link')) {
-            validatePath(href, fileName, extDir);
+            validatePath(href, fullFileName, extDir);
         }
+        validateScriptOrder(fileName, window);
     } finally {
         window.close();
     }
@@ -63,8 +155,8 @@ function main() {
     try {
         const extDir = path.resolve(__dirname, '..', '..', 'ext');
         const pattern = /\.html$/;
-        const ignorePattern = /[\\/]ext[\\/]lib[\\/]/;
-        const fileNames = getAllFiles(extDir, null, (f) => pattern.test(f) && !ignorePattern.test(f));
+        const ignorePattern = /^lib[\\/]/;
+        const fileNames = getAllFiles(extDir, (f) => pattern.test(f) && !ignorePattern.test(f));
         for (const fileName of fileNames) {
             validateHtmlScripts(fileName, extDir);
         }
diff --git a/dev/util.js b/dev/util.js
index 85a478ac..a31cd9ac 100644
--- a/dev/util.js
+++ b/dev/util.js
@@ -76,7 +76,7 @@ function getArgs(args, argMap) {
     return argMap;
 }
 
-function getAllFiles(baseDirectory, relativeTo=null, predicate=null) {
+function getAllFiles(baseDirectory, predicate=null) {
     const results = [];
     const directories = [baseDirectory];
     while (directories.length > 0) {
@@ -84,10 +84,10 @@ function getAllFiles(baseDirectory, relativeTo=null, predicate=null) {
         const fileNames = fs.readdirSync(directory);
         for (const fileName of fileNames) {
             const fullFileName = path.join(directory, fileName);
-            const relativeFileName = (relativeTo !== null ? path.relative(relativeTo, fullFileName) : fullFileName);
+            const relativeFileName = path.relative(baseDirectory, fullFileName);
             const stats = fs.lstatSync(fullFileName);
             if (stats.isFile()) {
-                if (typeof predicate !== 'function' || predicate(fullFileName, directory, baseDirectory)) {
+                if (typeof predicate !== 'function' || predicate(relativeFileName)) {
                     results.push(relativeFileName);
                 }
             } else if (stats.isDirectory()) {