Skip to content

Commit 40e056e

Browse files
committed
refactor: extract duplicated localize function
The `localize` function was duplicated into popup and options ES modules. The function is now in a shared script and covered by tests.
1 parent 91d3af2 commit 40e056e

6 files changed

Lines changed: 78 additions & 39 deletions

File tree

‎DOCUMENTATION.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ On open, the popup:
5959
├── manifest.json # MV3 manifest
6060
├── background.js # service worker: fetch PRs, match file, set badge
6161
├── browser.js # cross-browser WebExtension API shim (browser/chrome)
62+
├── i18n.js # shared localize() for the popup and options pages
6263
├── forges.js # per-forge API endpoint config
6364
├── popup/
6465
│ ├── popup.html # popup markup
@@ -196,7 +197,8 @@ Translations live in `_locales/<lang>/messages.json`. The default locale is
196197
language if it exists.
197198

198199
Strings are resolved two ways:
199-
- `__MSG_<key>__` placeholders
200+
- `__MSG_<key>__` placeholders in page markup, swapped in at load time by the
201+
shared `localize()` in `i18n.js` (used by both the popup and options pages)
200202
- `browser.i18n.getMessage("<key>")` (in `popup.js` and `options.js`).
201203

202204
To add a locale:

‎i18n.js‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import { browser } from "./browser.js"
2+
3+
// Replace every __MSG_<key>__ placeholder in the text of the page's
4+
// [i18n]-tagged elements with its localized string. Shared by the popup and the
5+
// options page; `root` defaults to the document but can be a subtree (or a mock
6+
// in tests).
7+
export function localize(root = document) {
8+
for (const element of root.querySelectorAll("[i18n]")) {
9+
const text = element.textContent
10+
const localized = text.replace(
11+
/__MSG_(\w+)__/g,
12+
(match, key) => (key ? browser.i18n.getMessage(key) : ""),
13+
)
14+
15+
if (localized !== text) {
16+
element.textContent = localized
17+
}
18+
}
19+
}

‎options/options.js‎

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,10 @@
11
import { browser } from "../browser.js"
22
import { authForges, selfHostedTypes, selfHostedTokenKey } from "../forges.js"
3+
import { localize } from "../i18n.js"
34

45
//
56
// Functions
67
//
7-
function localize() {
8-
// Localize by replacing __MSG_***__ meta tags
9-
const toLocalize = document.querySelectorAll("[i18n]")
10-
for (let i = 0; i < toLocalize.length; i++) {
11-
const obj = toLocalize[i]
12-
const text = obj.textContent
13-
const textLocalized = text.replace(
14-
/__MSG_(\w+)__/g,
15-
function(match, v1) {
16-
return v1 ? browser.i18n.getMessage(v1) : ""
17-
}
18-
)
19-
20-
if (textLocalized != text) {
21-
obj.textContent = textLocalized
22-
}
23-
}
24-
}
25-
268
function showStatus(messageKey, elementId = "status") {
279
document.getElementById(elementId).textContent = browser.i18n.getMessage(messageKey)
2810
}

‎popup/popup.js‎

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { browser } from "../browser.js"
22
import { forgeForHostname } from "../forges.js"
3+
import { localize } from "../i18n.js"
34

45
//
56
// Functions
@@ -58,25 +59,6 @@ export function renderError(urlInfo) {
5859

5960
}
6061

61-
function localize() {
62-
//Localize by replacing __MSG_***__ meta tags
63-
var toLocalize = document.querySelectorAll("[i18n]")
64-
for (var i=0; i < toLocalize.length; i++) {
65-
var obj = toLocalize[i]
66-
var text = obj.textContent
67-
var textLocalized = text.replace(
68-
/__MSG_(\w+)__/g,
69-
function(match, v1) {
70-
return v1 ? browser.i18n.getMessage(v1) : ""
71-
}
72-
)
73-
74-
if(textLocalized != text) {
75-
obj.textContent = textLocalized
76-
}
77-
}
78-
}
79-
8062
//
8163
// Main
8264
//

‎tests/i18n.test.js‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import { test } from "node:test"
2+
import assert from "node:assert/strict"
3+
4+
// localize() reads the browser.i18n namespace through the browser shim, which
5+
// resolves from globalThis at import time; install a mock before importing.
6+
//
7+
// Contract: the mock MUST be installed before the dynamic import() below —
8+
// keep it dynamic, do not turn it into a static import (those are hoisted and
9+
// would run before the mock is set). The mock is left on globalThis without
10+
// teardown; that is safe because `node --test` runs each test file in its own
11+
// process, so it cannot leak into other test files. Both assumptions break
12+
// under an in-process runner (e.g. --test-isolation=none).
13+
globalThis.browser = {
14+
i18n: { getMessage: (key) => (key === "greeting" ? "Hello" : `<${key}>`) },
15+
}
16+
const { localize } = await import("../i18n.js")
17+
18+
// a minimal element stand-in exposing just textContent
19+
function element(textContent) {
20+
return { textContent }
21+
}
22+
23+
// a fake root whose querySelectorAll("[i18n]") returns the given elements
24+
function root(elements) {
25+
return { querySelectorAll: () => elements }
26+
}
27+
28+
test("localize replaces a __MSG_<key>__ placeholder with its translation", () => {
29+
const el = element("__MSG_greeting__")
30+
localize(root([el]))
31+
assert.equal(el.textContent, "Hello")
32+
})
33+
34+
test("localize replaces every placeholder across the tagged elements", () => {
35+
const a = element("__MSG_greeting__")
36+
const b = element("prefix __MSG_missing__ suffix")
37+
localize(root([a, b]))
38+
assert.equal(a.textContent, "Hello")
39+
assert.equal(b.textContent, "prefix <missing> suffix")
40+
})
41+
42+
test("localize leaves text without a placeholder untouched", () => {
43+
const el = element("plain text")
44+
localize(root([el]))
45+
// unchanged (and reference identity of the string is irrelevant, value is)
46+
assert.equal(el.textContent, "plain text")
47+
})

‎tests/popup.test.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@ import assert from "node:assert/strict"
88
// captured at load time (browser.js resolves it from globalThis). Both must be
99
// in place *before* importing popup.js, so this file installs them on
1010
// globalThis and then dynamically imports the module under test.
11+
//
12+
// Contract: the mocks MUST be installed before the dynamic import() below —
13+
// keep it dynamic, do not turn it into a static import (those are hoisted and
14+
// would run before the mocks are set). The mocks are left on globalThis without
15+
// teardown; that is safe because `node --test` runs each test file in its own
16+
// process, so they cannot leak into other test files. Both assumptions break
17+
// under an in-process runner (e.g. --test-isolation=none).
1118

1219
// a tiny stand-in for a DOM node: enough of the Element/DocumentFragment API
1320
// for render()/renderError() (setAttribute, textContent, appendChild,

0 commit comments

Comments
 (0)