From a4453c9a260b6c4003c0ebd0aa460f61b93a499a Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Wed, 5 Apr 2023 12:46:17 +0200 Subject: Special case English translation fallback We should not be listing this in LINGUAS as that gives the impression that English has en explicit translation. Instead, it is a special case that the code needs to be explicitly aware of. This reverts 9a06058 in favour of a more robust fix. --- app/localization.js | 13 ++++++------- app/ui.js | 2 +- tests/test.localization.js | 10 ++++++++++ 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/app/localization.js b/app/localization.js index 84341da..73f66c5 100644 --- a/app/localization.js +++ b/app/localization.js @@ -40,12 +40,6 @@ export class Localizer { .replace("_", "-") .split("-"); - // Built-in default? - if ((userLang[0] === 'en') && - ((userLang[1] === undefined) || (userLang[1] === 'us'))) { - return; - } - // First pass: perfect match for (let j = 0; j < supportedLanguages.length; j++) { const supLang = supportedLanguages[j] @@ -64,7 +58,12 @@ export class Localizer { return; } - // Second pass: fallback + // Second pass: English fallback + if (userLang[0] === 'en') { + return; + } + + // Third pass pass: other fallback for (let j = 0;j < supportedLanguages.length;j++) { const supLang = supportedLanguages[j] .toLowerCase() diff --git a/app/ui.js b/app/ui.js index 07e0904..c1f6776 100644 --- a/app/ui.js +++ b/app/ui.js @@ -1762,7 +1762,7 @@ const UI = { }; // Set up translations -const LINGUAS = ["cs", "de", "el", "en", "es", "fr", "it", "ja", "ko", "nl", "pl", "pt_BR", "ru", "sv", "tr", "zh_CN", "zh_TW"]; +const LINGUAS = ["cs", "de", "el", "es", "fr", "it", "ja", "ko", "nl", "pl", "pt_BR", "ru", "sv", "tr", "zh_CN", "zh_TW"]; l10n.setup(LINGUAS); if (l10n.language === "en" || l10n.dictionary !== undefined) { UI.prime(); diff --git a/tests/test.localization.js b/tests/test.localization.js index 7e8e6c1..36e8d04 100644 --- a/tests/test.localization.js +++ b/tests/test.localization.js @@ -27,6 +27,16 @@ describe('Localization', function () { l10n.setup(["es", "fr"]); expect(l10n.language).to.equal('en'); }); + it('should fall back to generic English for other English', function () { + window.navigator.languages = ["en-AU", "de"]; + l10n.setup(["de", "fr", "en-GB"]); + expect(l10n.language).to.equal('en'); + }); + it('should prefer specific English over generic', function () { + window.navigator.languages = ["en-GB", "de"]; + l10n.setup(["de", "en-AU", "en-GB"]); + expect(l10n.language).to.equal('en-GB'); + }); it('should use the most preferred user language', function () { window.navigator.languages = ["nl", "de", "fr"]; l10n.setup(["es", "fr", "de"]); -- cgit v1.2.1 From 681632bc9fc38b4e4f0f9bf4a6b7d675fb991e70 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Thu, 6 Apr 2023 11:06:06 +0200 Subject: Avoid running tests on l10n singleton We want tests to be independent, so we cannot have them modify a shared state, such as the l10n singleton. Make sure each test instantiates its own object instead. --- tests/test.localization.js | 61 +++++++++++++++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 20 deletions(-) diff --git a/tests/test.localization.js b/tests/test.localization.js index 36e8d04..1db2cb9 100644 --- a/tests/test.localization.js +++ b/tests/test.localization.js @@ -1,9 +1,20 @@ const expect = chai.expect; -import { l10n } from '../app/localization.js'; +import _, { Localizer, l10n } from '../app/localization.js'; describe('Localization', function () { "use strict"; + describe('Singleton', function () { + it('should export a singleton object', function () { + expect(l10n).to.be.instanceOf(Localizer); + }); + it('should export a singleton translation function', function () { + // FIXME: Can we use some spy instead? + l10n.dictionary = { "Foobar": "gazonk" }; + expect(_("Foobar")).to.equal("gazonk"); + }); + }); + describe('language selection', function () { let origNavigator; beforeEach(function () { @@ -20,52 +31,62 @@ describe('Localization', function () { }); it('should use English by default', function () { - expect(l10n.language).to.equal('en'); + let lclz = new Localizer(); + expect(lclz.language).to.equal('en'); }); it('should use English if no user language matches', function () { window.navigator.languages = ["nl", "de"]; - l10n.setup(["es", "fr"]); - expect(l10n.language).to.equal('en'); + let lclz = new Localizer(); + lclz.setup(["es", "fr"]); + expect(lclz.language).to.equal('en'); }); it('should fall back to generic English for other English', function () { window.navigator.languages = ["en-AU", "de"]; - l10n.setup(["de", "fr", "en-GB"]); - expect(l10n.language).to.equal('en'); + let lclz = new Localizer(); + lclz.setup(["de", "fr", "en-GB"]); + expect(lclz.language).to.equal('en'); }); it('should prefer specific English over generic', function () { window.navigator.languages = ["en-GB", "de"]; - l10n.setup(["de", "en-AU", "en-GB"]); - expect(l10n.language).to.equal('en-GB'); + let lclz = new Localizer(); + lclz.setup(["de", "en-AU", "en-GB"]); + expect(lclz.language).to.equal('en-GB'); }); it('should use the most preferred user language', function () { window.navigator.languages = ["nl", "de", "fr"]; - l10n.setup(["es", "fr", "de"]); - expect(l10n.language).to.equal('de'); + let lclz = new Localizer(); + lclz.setup(["es", "fr", "de"]); + expect(lclz.language).to.equal('de'); }); it('should prefer sub-languages languages', function () { window.navigator.languages = ["pt-BR"]; - l10n.setup(["pt", "pt-BR"]); - expect(l10n.language).to.equal('pt-BR'); + let lclz = new Localizer(); + lclz.setup(["pt", "pt-BR"]); + expect(lclz.language).to.equal('pt-BR'); }); it('should fall back to language "parents"', function () { window.navigator.languages = ["pt-BR"]; - l10n.setup(["fr", "pt", "de"]); - expect(l10n.language).to.equal('pt'); + let lclz = new Localizer(); + lclz.setup(["fr", "pt", "de"]); + expect(lclz.language).to.equal('pt'); }); it('should not use specific language when user asks for a generic language', function () { window.navigator.languages = ["pt", "de"]; - l10n.setup(["fr", "pt-BR", "de"]); - expect(l10n.language).to.equal('de'); + let lclz = new Localizer(); + lclz.setup(["fr", "pt-BR", "de"]); + expect(lclz.language).to.equal('de'); }); it('should handle underscore as a separator', function () { window.navigator.languages = ["pt-BR"]; - l10n.setup(["pt_BR"]); - expect(l10n.language).to.equal('pt_BR'); + let lclz = new Localizer(); + lclz.setup(["pt_BR"]); + expect(lclz.language).to.equal('pt_BR'); }); it('should handle difference in case', function () { window.navigator.languages = ["pt-br"]; - l10n.setup(["pt-BR"]); - expect(l10n.language).to.equal('pt-BR'); + let lclz = new Localizer(); + lclz.setup(["pt-BR"]); + expect(lclz.language).to.equal('pt-BR'); }); }); }); -- cgit v1.2.1 From 2a21bee245aee7827eb00925307d2e107b6db646 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Thu, 6 Apr 2023 17:11:47 +0200 Subject: Revert broken Add support for URL fragment parameters This is a revert of the code changes in commit f796b05e42cfac7044cca9603e59f258605228f3 as it served no functional purpose. Fragments were already respected for setting parameters, via a different function. Thus it is unclear what that commit tried to fix. It also complicated things by mixing the document location with the window location. The comment changes are useful, though, so those are kept. --- app/webutil.js | 2 +- vnc_lite.html | 11 ++--------- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/app/webutil.js b/app/webutil.js index 084c69f..a2eab19 100644 --- a/app/webutil.js +++ b/app/webutil.js @@ -32,7 +32,7 @@ export function initLogging(level) { export function getQueryVar(name, defVal) { "use strict"; const re = new RegExp('.*[?&]' + name + '=([^&#]*)'), - match = ''.concat(document.location.href, window.location.hash).match(re); + match = document.location.href.match(re); if (typeof defVal === 'undefined') { defVal = null; } if (match) { diff --git a/vnc_lite.html b/vnc_lite.html index e725a2d..eaf75f8 100644 --- a/vnc_lite.html +++ b/vnc_lite.html @@ -107,20 +107,13 @@ // query string. If the variable isn't defined in the URL // it returns the default value instead. function readQueryVariable(name, defaultValue) { - // A URL with a query parameter can look like this (But will most probably get logged on the http server): + // A URL with a query parameter can look like this: // https://www.example.com?myqueryparam=myvalue // - // For privacy (Using a hastag #, the parameters will not be sent to the server) - // the url can be requested in the following way: - // https://www.example.com#myqueryparam=myvalue&password=secreatvalue - // - // Even Mixing public and non public parameters will work: - // https://www.example.com?nonsecretparam=example.com#password=secreatvalue - // // Note that we use location.href instead of location.search // because Firefox < 53 has a bug w.r.t location.search const re = new RegExp('.*[?&]' + name + '=([^&#]*)'), - match = ''.concat(document.location.href, window.location.hash).match(re); + match = document.location.href.match(re); if (match) { // We have to decode the URL since want the cleartext value -- cgit v1.2.1 From 05b6d2ad67fbe6f3a7d7cbf1fbe0e0cec1362929 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Wed, 10 May 2023 12:24:53 +0200 Subject: Fix typos in query variable comment --- app/webutil.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/webutil.js b/app/webutil.js index a2eab19..b94f035 100644 --- a/app/webutil.js +++ b/app/webutil.js @@ -25,10 +25,10 @@ export function initLogging(level) { // // For privacy (Using a hastag #, the parameters will not be sent to the server) // the url can be requested in the following way: -// https://www.example.com#myqueryparam=myvalue&password=secreatvalue +// https://www.example.com#myqueryparam=myvalue&password=secretvalue // // Even Mixing public and non public parameters will work: -// https://www.example.com?nonsecretparam=example.com#password=secreatvalue +// https://www.example.com?nonsecretparam=example.com#password=secretvalue export function getQueryVar(name, defVal) { "use strict"; const re = new RegExp('.*[?&]' + name + '=([^&#]*)'), -- cgit v1.2.1 From cd1a63b737b1ffa0fb5a343de7381bedf562c5e4 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Thu, 6 Apr 2023 17:49:44 +0200 Subject: Restore history state after tests We don't want to mess up anything permanent in each test or the tests might start affecting each other. --- tests/test.webutil.js | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/tests/test.webutil.js b/tests/test.webutil.js index 76aa763..6f460a3 100644 --- a/tests/test.webutil.js +++ b/tests/test.webutil.js @@ -8,39 +8,48 @@ describe('WebUtil', function () { "use strict"; describe('config variables', function () { + let origState, origHref; + beforeEach(function () { + origState = history.state; + origHref = location.href; + }); + afterEach(function () { + history.replaceState(origState, '', origHref); + }); + it('should parse query string variables', function () { // history.pushState() will not cause the browser to attempt loading // the URL, this is exactly what we want here for the tests. - history.pushState({}, '', "test?myvar=myval"); + history.replaceState({}, '', "test?myvar=myval"); expect(WebUtil.getConfigVar("myvar")).to.be.equal("myval"); }); it('should return default value when no query match', function () { - history.pushState({}, '', "test?myvar=myval"); + history.replaceState({}, '', "test?myvar=myval"); expect(WebUtil.getConfigVar("other", "def")).to.be.equal("def"); }); it('should handle no query match and no default value', function () { - history.pushState({}, '', "test?myvar=myval"); + history.replaceState({}, '', "test?myvar=myval"); expect(WebUtil.getConfigVar("other")).to.be.equal(null); }); it('should parse fragment variables', function () { - history.pushState({}, '', "test#myvar=myval"); + history.replaceState({}, '', "test#myvar=myval"); expect(WebUtil.getConfigVar("myvar")).to.be.equal("myval"); }); it('should return default value when no fragment match', function () { - history.pushState({}, '', "test#myvar=myval"); + history.replaceState({}, '', "test#myvar=myval"); expect(WebUtil.getConfigVar("other", "def")).to.be.equal("def"); }); it('should handle no fragment match and no default value', function () { - history.pushState({}, '', "test#myvar=myval"); + history.replaceState({}, '', "test#myvar=myval"); expect(WebUtil.getConfigVar("other")).to.be.equal(null); }); it('should handle both query and fragment', function () { - history.pushState({}, '', "test?myquery=1#myhash=2"); + history.replaceState({}, '', "test?myquery=1#myhash=2"); expect(WebUtil.getConfigVar("myquery")).to.be.equal("1"); expect(WebUtil.getConfigVar("myhash")).to.be.equal("2"); }); it('should prioritize fragment if both provide same var', function () { - history.pushState({}, '', "test?myvar=1#myvar=2"); + history.replaceState({}, '', "test?myvar=1#myvar=2"); expect(WebUtil.getConfigVar("myvar")).to.be.equal("2"); }); }); -- cgit v1.2.1 From 0374b4c0fc28139aed1a06d9ec2cd5b66198cd94 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Thu, 6 Apr 2023 13:21:47 +0200 Subject: Handle translation loading in translation class Let's try to keep as much as possible of the translation handling in a single place for clarity. --- app/localization.js | 36 +++++++++++-- app/ui.js | 18 ++----- tests/test.localization.js | 122 ++++++++++++++++++++++++++++++++------------- 3 files changed, 123 insertions(+), 53 deletions(-) diff --git a/app/localization.js b/app/localization.js index 73f66c5..7d7e6e6 100644 --- a/app/localization.js +++ b/app/localization.js @@ -16,13 +16,19 @@ export class Localizer { this.language = 'en'; // Current dictionary of translations - this.dictionary = undefined; + this._dictionary = undefined; } // Configure suitable language based on user preferences - setup(supportedLanguages) { + async setup(supportedLanguages, baseURL) { this.language = 'en'; // Default: US English + this._dictionary = undefined; + this._setupLanguage(supportedLanguages); + await this._setupDictionary(baseURL); + } + + _setupLanguage(supportedLanguages) { /* * Navigator.languages only available in Chrome (32+) and FireFox (32+) * Fall back to navigator.language for other browsers @@ -83,10 +89,32 @@ export class Localizer { } } + async _setupDictionary(baseURL) { + if (baseURL) { + if (!baseURL.endsWith("/")) { + baseURL = baseURL + "/"; + } + } else { + baseURL = ""; + } + + if (this.language === "en") { + return; + } + + let response = await fetch(baseURL + this.language + ".json"); + if (!response.ok) { + throw Error("" + response.status + " " + response.statusText); + } + + this._dictionary = await response.json(); + } + // Retrieve localised text get(id) { - if (typeof this.dictionary !== 'undefined' && this.dictionary[id]) { - return this.dictionary[id]; + if (typeof this._dictionary !== 'undefined' && + this._dictionary[id]) { + return this._dictionary[id]; } else { return id; } diff --git a/app/ui.js b/app/ui.js index c1f6776..85695ca 100644 --- a/app/ui.js +++ b/app/ui.js @@ -1763,20 +1763,8 @@ const UI = { // Set up translations const LINGUAS = ["cs", "de", "el", "es", "fr", "it", "ja", "ko", "nl", "pl", "pt_BR", "ru", "sv", "tr", "zh_CN", "zh_TW"]; -l10n.setup(LINGUAS); -if (l10n.language === "en" || l10n.dictionary !== undefined) { - UI.prime(); -} else { - fetch('app/locale/' + l10n.language + '.json') - .then((response) => { - if (!response.ok) { - throw Error("" + response.status + " " + response.statusText); - } - return response.json(); - }) - .then((translations) => { l10n.dictionary = translations; }) - .catch(err => Log.Error("Failed to load translations: " + err)) - .then(UI.prime); -} +l10n.setup(LINGUAS, "app/locale/") + .catch(err => Log.Error("Failed to load translations: " + err)) + .then(UI.prime); export default UI; diff --git a/tests/test.localization.js b/tests/test.localization.js index 1db2cb9..916ff84 100644 --- a/tests/test.localization.js +++ b/tests/test.localization.js @@ -4,89 +4,143 @@ import _, { Localizer, l10n } from '../app/localization.js'; describe('Localization', function () { "use strict"; + let origNavigator; + let fetch; + + beforeEach(function () { + // window.navigator is a protected read-only property in many + // environments, so we need to redefine it whilst running these + // tests. + origNavigator = Object.getOwnPropertyDescriptor(window, "navigator"); + + Object.defineProperty(window, "navigator", {value: {}}); + window.navigator.languages = []; + + fetch = sinon.stub(window, "fetch"); + fetch.resolves(new Response("{}")); + }); + afterEach(function () { + fetch.restore(); + + Object.defineProperty(window, "navigator", origNavigator); + }); + describe('Singleton', function () { it('should export a singleton object', function () { expect(l10n).to.be.instanceOf(Localizer); }); - it('should export a singleton translation function', function () { + it('should export a singleton translation function', async function () { // FIXME: Can we use some spy instead? - l10n.dictionary = { "Foobar": "gazonk" }; + window.navigator.languages = ["de"]; + fetch.resolves(new Response(JSON.stringify({ "Foobar": "gazonk" }))); + await l10n.setup(["de"]); expect(_("Foobar")).to.equal("gazonk"); }); }); describe('language selection', function () { - let origNavigator; - beforeEach(function () { - // window.navigator is a protected read-only property in many - // environments, so we need to redefine it whilst running these - // tests. - origNavigator = Object.getOwnPropertyDescriptor(window, "navigator"); - - Object.defineProperty(window, "navigator", {value: {}}); - window.navigator.languages = []; - }); - afterEach(function () { - Object.defineProperty(window, "navigator", origNavigator); - }); - it('should use English by default', function () { let lclz = new Localizer(); expect(lclz.language).to.equal('en'); }); - it('should use English if no user language matches', function () { + it('should use English if no user language matches', async function () { window.navigator.languages = ["nl", "de"]; let lclz = new Localizer(); - lclz.setup(["es", "fr"]); + await lclz.setup(["es", "fr"]); expect(lclz.language).to.equal('en'); }); - it('should fall back to generic English for other English', function () { + it('should fall back to generic English for other English', async function () { window.navigator.languages = ["en-AU", "de"]; let lclz = new Localizer(); - lclz.setup(["de", "fr", "en-GB"]); + await lclz.setup(["de", "fr", "en-GB"]); expect(lclz.language).to.equal('en'); }); - it('should prefer specific English over generic', function () { + it('should prefer specific English over generic', async function () { window.navigator.languages = ["en-GB", "de"]; let lclz = new Localizer(); - lclz.setup(["de", "en-AU", "en-GB"]); + await lclz.setup(["de", "en-AU", "en-GB"]); expect(lclz.language).to.equal('en-GB'); }); - it('should use the most preferred user language', function () { + it('should use the most preferred user language', async function () { window.navigator.languages = ["nl", "de", "fr"]; let lclz = new Localizer(); - lclz.setup(["es", "fr", "de"]); + await lclz.setup(["es", "fr", "de"]); expect(lclz.language).to.equal('de'); }); - it('should prefer sub-languages languages', function () { + it('should prefer sub-languages languages', async function () { window.navigator.languages = ["pt-BR"]; let lclz = new Localizer(); - lclz.setup(["pt", "pt-BR"]); + await lclz.setup(["pt", "pt-BR"]); expect(lclz.language).to.equal('pt-BR'); }); - it('should fall back to language "parents"', function () { + it('should fall back to language "parents"', async function () { window.navigator.languages = ["pt-BR"]; let lclz = new Localizer(); - lclz.setup(["fr", "pt", "de"]); + await lclz.setup(["fr", "pt", "de"]); expect(lclz.language).to.equal('pt'); }); - it('should not use specific language when user asks for a generic language', function () { + it('should not use specific language when user asks for a generic language', async function () { window.navigator.languages = ["pt", "de"]; let lclz = new Localizer(); - lclz.setup(["fr", "pt-BR", "de"]); + await lclz.setup(["fr", "pt-BR", "de"]); expect(lclz.language).to.equal('de'); }); - it('should handle underscore as a separator', function () { + it('should handle underscore as a separator', async function () { window.navigator.languages = ["pt-BR"]; let lclz = new Localizer(); - lclz.setup(["pt_BR"]); + await lclz.setup(["pt_BR"]); expect(lclz.language).to.equal('pt_BR'); }); - it('should handle difference in case', function () { + it('should handle difference in case', async function () { window.navigator.languages = ["pt-br"]; let lclz = new Localizer(); - lclz.setup(["pt-BR"]); + await lclz.setup(["pt-BR"]); expect(lclz.language).to.equal('pt-BR'); }); }); + + describe('Translation loading', function () { + it('should not fetch a translation for English', async function () { + window.navigator.languages = []; + let lclz = new Localizer(); + await lclz.setup([]); + expect(fetch).to.not.have.been.called; + }); + it('should fetch dictionary relative base URL', async function () { + window.navigator.languages = ["de", "fr"]; + fetch.resolves(new Response('{ "Foobar": "gazonk" }')); + let lclz = new Localizer(); + await lclz.setup(["ru", "fr"], "/some/path/"); + expect(fetch).to.have.been.calledOnceWith("/some/path/fr.json"); + expect(lclz.get("Foobar")).to.equal("gazonk"); + }); + it('should handle base URL without trailing slash', async function () { + window.navigator.languages = ["de", "fr"]; + fetch.resolves(new Response('{ "Foobar": "gazonk" }')); + let lclz = new Localizer(); + await lclz.setup(["ru", "fr"], "/some/path"); + expect(fetch).to.have.been.calledOnceWith("/some/path/fr.json"); + expect(lclz.get("Foobar")).to.equal("gazonk"); + }); + it('should handle current base URL', async function () { + window.navigator.languages = ["de", "fr"]; + fetch.resolves(new Response('{ "Foobar": "gazonk" }')); + let lclz = new Localizer(); + await lclz.setup(["ru", "fr"]); + expect(fetch).to.have.been.calledOnceWith("fr.json"); + expect(lclz.get("Foobar")).to.equal("gazonk"); + }); + it('should fail if dictionary cannot be found', async function () { + window.navigator.languages = ["de", "fr"]; + fetch.resolves(new Response('{}', { status: 404 })); + let lclz = new Localizer(); + let ok = false; + try { + await lclz.setup(["ru", "fr"], "/some/path/"); + } catch (e) { + ok = true; + } + expect(ok).to.be.true; + }); + }); }); -- cgit v1.2.1