coffey-labs/ihasmail
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 2 Wiki Activity

Stop a language change undoing itself, and stop the translate prompt

Browse Source 
Two reports, both about the language setting.

**Picking a language sometimes took several clicks.** The subtree that
reads the account's settings file is keyed on the language version, so
choosing a language deliberately throws it away and builds it again. The
remount re-read the settings file — which still held the old language,
because the write is debounced by three seconds — and applied it, putting
the old language back. The click that appeared to work was the one made
after the previous write had landed, which is exactly the "sometimes"
in the report.

Worse than it looked: the queued push survived the remount, so the file
was eventually written with the new language while the screen showed the
old one. A reload then changed the language on its own.

Fixed twice over, because either alone leaves a race. The file is read
once per account per page load rather than once per mount, and hydrate
now holds back any key with a change still queued — a change that has
not been written up is newer than the file by definition. That rule is
`mergeRemote`, pulled out as a pure function so it could be tested
without a JMAP client.

**Both browsers kept offering to translate an English page.** They were
right to: `<html lang>` said English while the visible text was 6,289
message rows of marketing copy and brand names in whatever language the
sender wrote in. The list is most of the text on the screen, so that is
what the detector was reading.

Sender, subject and preview in the list, and the thread subject and
sender name in the reader, are now marked as what they are — content,
not interface. Message bodies were already marked, so this is the same
line drawn in the places the earlier pass missed rather than a new one.
Whether it silences the prompt is Chrome's call and cannot be checked
from inside the page; the marking is right either way.
This commit is contained in:
jcoffey-dev
2026-08-31 13:45:18 -07:00
parent a4f7d386a6
commit 51dabd9cab
7 changed files with 117 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
web/src/App.tsx
+7 -2
View File
@@ -18,7 +18,7 @@ import { MailView } from "@/views/mail/MailView";
import { ComposerDock } from "@/views/compose/ComposerDock"; import { ComposerDock } from "@/views/compose/ComposerDock";
import { setUnreadBadge } from "@/lib/notify"; import { setUnreadBadge } from "@/lib/notify";
import { useSettings, syncedPart } from "@/store/settings"; import { useSettings, syncedPart } from "@/store/settings";
import { armSettingsSync, loadRemoteSettings, queueSettingsPush, settingsSyncAvailable } from "@/lib/settingsSync"; import { armSettingsSync, loadRemoteSettings, queueSettingsPush, settingsAlreadyLoadedFor, settingsSyncAvailable } from "@/lib/settingsSync";
import { listenForVerification, renewWebPush } from "@/lib/webpushEnable"; import { listenForVerification, renewWebPush } from "@/lib/webpushEnable";
import { useLanguageVersion } from "@/lib/i18n"; import { useLanguageVersion } from "@/lib/i18n";
@@ -68,8 +68,13 @@ function AuthedApp() {
// Settings that live with the account rather than the browser. The cached // Settings that live with the account rather than the browser. The cached
// ones have already painted, so this only has to correct them (issue #54). // ones have already painted, so this only has to correct them (issue #54).
//
// Once per account, not once per mount: this subtree is keyed on the
// language version, so picking a language throws it away and builds it
// again. Re-reading the settings file there would apply a copy written
// before the change and undo it.
useEffect(() => { useEffect(() => {
if (!accountId) return; if (settingsAlreadyLoadedFor(accountId)) return;
let cancelled = false; let cancelled = false;
void (async () => { void (async () => {
const remote = await loadRemoteSettings(); const remote = await loadRemoteSettings();
web/src/lib/__tests__/settingsSync.test.ts
+46 -1
View File
@@ -1,6 +1,7 @@
import { describe, expect, it } from "vitest"; import { describe, expect, it } from "vitest";
import { DEFAULT_SETTINGS, DEVICE_KEYS, acceptRemote, syncedPart, type Settings } from "@/store/settings"; import { DEFAULT_SETTINGS, DEVICE_KEYS, acceptRemote, mergeRemote, syncedPart, type Settings } from "@/store/settings";
import { isAppFolder } from "../appFolder"; import { isAppFolder } from "../appFolder";
import { settingsAlreadyLoadedFor, stopSettingsSync } from "../settingsSync";
/** /**
* Settings used to live only in localStorage, so nothing followed the user * Settings used to live only in localStorage, so nothing followed the user
@@ -81,3 +82,47 @@ describe("the client's own folder", () => {
expect(isAppFolder({ name: "ihasmail", parentId: null, nodeType: "file" })).toBe(false); expect(isAppFolder({ name: "ihasmail", parentId: null, nodeType: "file" })).toBe(false);
}); });
}); });
/**
* Picking a language used to come undone.
*
* The subtree that reads the account's settings file is keyed on the language
* version, so choosing a language throws it away and builds it again. The
* remount re-read the file — which still held the old language, because the
* write is debounced by three seconds — and applied it, putting the old
* language back. Reported as "sometimes it takes several clicks": the click
* that appeared to work was the one made after the previous write had landed.
*/
describe("a change made but not yet written up", () => {
it("is not read back over by the file it has not reached yet", () => {
const current: Settings = { ...DEFAULT_SETTINGS, uiLanguage: "ja" };
const file = { uiLanguage: "en", theme: "dark" };
const merged = mergeRemote(current, file, new Set(["uiLanguage"]));
expect(merged.uiLanguage).toBe("ja");
// Only the queued key is held back; the rest of the file still applies.
expect(merged.theme).toBe("dark");
});
it("applies the whole file when nothing is queued", () => {
const current: Settings = { ...DEFAULT_SETTINGS, uiLanguage: "ja" };
const merged = mergeRemote(current, { uiLanguage: "en" });
expect(merged.uiLanguage).toBe("en");
});
it("reads the file again for an account after a sign-out", () => {
stopSettingsSync();
expect(settingsAlreadyLoadedFor("a1")).toBe(false);
// The remount that a language change causes must not read it a second time.
expect(settingsAlreadyLoadedFor("a1")).toBe(true);
// Signing out drops the claim, so signing back in reads the file rather
// than trusting whatever the previous session left behind.
stopSettingsSync();
expect(settingsAlreadyLoadedFor("a1")).toBe(false);
stopSettingsSync();
});
it("treats a missing account as already loaded, so nothing is fetched", () => {
expect(settingsAlreadyLoadedFor(null)).toBe(true);
expect(settingsAlreadyLoadedFor(undefined)).toBe(true);
});
});
web/src/lib/settingsSync.ts
+34
View File
@@ -33,6 +33,7 @@ let pending: Record<string, unknown> | null = null;
let inFlight: Promise<void> | null = null; let inFlight: Promise<void> | null = null;
/** Nothing is pushed before the first load has settled, or we would race it. */ /** Nothing is pushed before the first load has settled, or we would race it. */
let armed = false; let armed = false;
let loadedFor: string | null = null;
let listenersBound = false; let listenersBound = false;
export function settingsSyncAvailable(): boolean { export function settingsSyncAvailable(): boolean {
@@ -62,6 +63,24 @@ export async function loadRemoteSettings(): Promise<Record<string, unknown> | nu
} }
} }
/**
* Has this account's settings file already been read on this page load?
*
* Claims the account as a side effect, so two callers cannot both start a
* read. The subtree that does the reading is keyed on the language version
* and so is deliberately remounted whenever somebody picks a language;
* without this the remount re-reads a file written before the change and
* applies it, putting the old language back.
*
* Cleared by `stopSettingsSync`, so signing out and back in reads again.
*/
export function settingsAlreadyLoadedFor(accountId: string | null | undefined): boolean {
if (!accountId) return true;
if (loadedFor === accountId) return true;
loadedFor = accountId;
return false;
}
/** Allow pushes. Called once the first load has settled, either way. */ /** Allow pushes. Called once the first load has settled, either way. */
export function armSettingsSync(): void { export function armSettingsSync(): void {
armed = true; armed = true;
@@ -71,6 +90,7 @@ export function armSettingsSync(): void {
/** Stop syncing and drop anything queued (logout). */ /** Stop syncing and drop anything queued (logout). */
export function stopSettingsSync(): void { export function stopSettingsSync(): void {
armed = false; armed = false;
loadedFor = null;
pending = null; pending = null;
if (timer !== null) { if (timer !== null) {
window.clearTimeout(timer); window.clearTimeout(timer);
@@ -93,6 +113,20 @@ export function queueSettingsPush(synced: Record<string, unknown>): void {
}, DEBOUNCE_MS); }, DEBOUNCE_MS);
} }
/**
* The keys of a change that has been made but not yet written up.
*
* `hydrate` needs these: a settings file read from the server is older than an
* unflushed local change by definition, so applying it wholesale hands the
* user back the value they just replaced. Switching language made that visible
* — it remounts the tree, the remount re-reads the file, and the file still
* says the old language — but the race is general and a slow read would lose
* any click made inside the debounce window.
*/
export function pendingSettingsKeys(): ReadonlySet<string> {
return new Set(pending ? Object.keys(pending) : []);
}
/** Write anything queued now, rather than waiting out the debounce. */ /** Write anything queued now, rather than waiting out the debounce. */
export async function flushSettingsPush(): Promise<void> { export async function flushSettingsPush(): Promise<void> {
if (timer !== null) { if (timer !== null) {
web/src/store/settings.ts
+22 -2
View File
@@ -1,7 +1,7 @@
import { useEffect, useState } from "react"; import { useEffect, useState } from "react";
import { create } from "zustand"; import { create } from "zustand";
import { loadJson, saveJson } from "@/lib/storage"; import { loadJson, saveJson } from "@/lib/storage";
import { queueSettingsPush } from "@/lib/settingsSync"; import { pendingSettingsKeys, queueSettingsPush } from "@/lib/settingsSync";
import { setDateTimePrefs, setUiLanguageForFormatting, type DateFormat, type TimeFormat } from "@/lib/datetime"; import { setDateTimePrefs, setUiLanguageForFormatting, type DateFormat, type TimeFormat } from "@/lib/datetime";
import type { SwipeAction } from "@/lib/swipe"; import type { SwipeAction } from "@/lib/swipe";
import { resolveUiLanguage } from "@/lib/languages"; import { resolveUiLanguage } from "@/lib/languages";
@@ -280,6 +280,26 @@ export function acceptRemote(remote: Record<string, unknown>): Partial<Settings>
return out as Partial<Settings>; return out as Partial<Settings>;
} }
/**
* The settings file laid over the ones in hand, minus anything still queued.
*
* A change that has not been written up yet is newer than the file by
* definition, so it wins. Picking a language is where this showed: that
* remounts the tree, the remount re-reads the file, and the file still holds
* the language from before the click, so the click came undone. Reported as
* "sometimes it takes several clicks" — the click that stuck was the one made
* after the previous write had landed.
*/
export function mergeRemote(
current: Settings,
remote: Record<string, unknown>,
held: ReadonlySet<string> = new Set(),
): Settings {
const incoming = acceptRemote(remote);
for (const key of held) delete incoming[key as keyof Settings];
return { ...current, ...incoming };
}
interface SettingsState { interface SettingsState {
settings: Settings; settings: Settings;
update(patch: Partial<Settings>): void; update(patch: Partial<Settings>): void;
@@ -333,7 +353,7 @@ export const useSettings = create<SettingsState>((set, get) => ({
} }
}, },
hydrate(remote) { hydrate(remote) {
const settings = { ...get().settings, ...acceptRemote(remote) }; const settings = mergeRemote(get().settings, remote, pendingSettingsKeys());
// Cache it, so the next first frame on this browser is already right. // Cache it, so the next first frame on this browser is already right.
saveJson("settings", settings); saveJson("settings", settings);
set({ settings }); set({ settings });
web/src/views/mail/MessageList.tsx
+6 -6
View File
@@ -648,7 +648,7 @@ const Row = memo(function Row({ email: e, threadEmails, top, height, selected, f
{twoLine ? ( {twoLine ? (
<div className="msg-body"> <div className="msg-body">
<div className="msg-line1"> <div className="msg-line1">
<span className="msg-from truncate"> <span className="msg-from truncate notranslate" translate="no">
<span className="truncate">{who}</span> <span className="truncate">{who}</span>
{count > 1 && <span className="thread-count"> {count}</span>} {count > 1 && <span className="thread-count"> {count}</span>}
</span> </span>
@@ -659,8 +659,8 @@ const Row = memo(function Row({ email: e, threadEmails, top, height, selected, f
</div> </div>
<div className="msg-main"> <div className="msg-main">
{isDrafts && <span style={{ color: "var(--danger)" }}>{t("Draft")}</span>} {isDrafts && <span style={{ color: "var(--danger)" }}>{t("Draft")}</span>}
<span className="msg-subject">{e.subject || "(no subject)"}</span> <span className="msg-subject notranslate" translate="no">{e.subject || "(no subject)"}</span>
{showPreview && <span className="msg-preview">{latest.preview}</span>} {showPreview && <span className="msg-preview notranslate" translate="no">{latest.preview}</span>}
<button className={`msg-star ${starred ? "on" : ""}`} style={{ marginLeft: "auto" }} onClick={(ev) => { ev.stopPropagation(); onStar(e.id, !starred); }} aria-label={t("Star")}> <button className={`msg-star ${starred ? "on" : ""}`} style={{ marginLeft: "auto" }} onClick={(ev) => { ev.stopPropagation(); onStar(e.id, !starred); }} aria-label={t("Star")}>
<Star size={16} fill={starred ? "currentColor" : "none"} /> <Star size={16} fill={starred ? "currentColor" : "none"} />
</button> </button>
@@ -669,15 +669,15 @@ const Row = memo(function Row({ email: e, threadEmails, top, height, selected, f
</div> </div>
) : ( ) : (
<> <>
<span className="msg-from" title={who}> <span className="msg-from notranslate" translate="no" title={who}>
<span className="truncate">{who}</span> <span className="truncate">{who}</span>
{count > 1 && <span className="thread-count">{count}</span>} {count > 1 && <span className="thread-count">{count}</span>}
</span> </span>
<span className="msg-main"> <span className="msg-main">
{isDrafts && <span style={{ color: "var(--danger)", flex: "0 0 auto" }}>{t("Draft")}</span>} {isDrafts && <span style={{ color: "var(--danger)", flex: "0 0 auto" }}>{t("Draft")}</span>}
{rowLabels.length > 0 && <span className="msg-labels">{rowLabels.map((l) => <span key={l.keyword} className="tag" style={{ background: l.color }}>{l.name}</span>)}</span>} {rowLabels.length > 0 && <span className="msg-labels">{rowLabels.map((l) => <span key={l.keyword} className="tag" style={{ background: l.color }}>{l.name}</span>)}</span>}
<span className="msg-subject">{e.subject || "(no subject)"}</span> <span className="msg-subject notranslate" translate="no">{e.subject || "(no subject)"}</span>
{showPreview && <span className="msg-preview">{latest.preview}</span>} {showPreview && <span className="msg-preview notranslate" translate="no">{latest.preview}</span>}
</span> </span>
<span className="msg-meta"> <span className="msg-meta">
{(answered || forwarded) && <span className="msg-answered" title={answered ? "Replied" : "Forwarded"}>{answered ? <Reply size={14} /> : <Forward size={14} />}</span>} {(answered || forwarded) && <span className="msg-answered" title={answered ? "Replied" : "Forwarded"}>{answered ? <Reply size={14} /> : <Forward size={14} />}</span>}
web/src/views/mail/MessageView.tsx
+1 -1
View File
@@ -149,7 +149,7 @@ export const MessageView = memo(function MessageView({ email: e, expanded, wasUn
<Avatar who={from ?? null} /> <Avatar who={from ?? null} />
<div className="who"> <div className="who">
<div className="from" onContextMenu={(ev) => from && addrMenu.open(ev, from)}> <div className="from" onContextMenu={(ev) => from && addrMenu.open(ev, from)}>
<span className="addr">{displayName(from)}</span> <span className="addr notranslate" translate="no">{displayName(from)}</span>
{/* An address, not a sentence. */} {/* An address, not a sentence. */}
{expanded && from && <span className="email addr notranslate" translate="no">&lt;{from.email}&gt;</span>} {expanded && from && <span className="email addr notranslate" translate="no">&lt;{from.email}&gt;</span>}
{isHighPriority && <span className="tag" style={{ background: "var(--danger)" }}>{translate("Important")}</span>} {isHighPriority && <span className="tag" style={{ background: "var(--danger)" }}>{translate("Important")}</span>}
web/src/views/mail/ThreadView.tsx
+1 -1
View File
@@ -260,7 +260,7 @@ export function ThreadView({ threadId, mailboxId, onBack, actions, onNavigate, h
<div className="thread-scroll" ref={scrollRef}> <div className="thread-scroll" ref={scrollRef}>
<div className="thread-subject"> <div className="thread-subject">
<div className="grow"> <div className="grow">
<h1>{subject}</h1> <h1 className="notranslate" translate="no">{subject}</h1>
{(threadLabels.length > 0 || threadMailboxes.length > 0) && ( {(threadLabels.length > 0 || threadMailboxes.length > 0) && (
<div className="labels"> <div className="labels">
{threadMailboxes.map((n) => <span key={n} className="chip">{n}</span>)} {threadMailboxes.map((n) => <span key={n} className="chip">{n}</span>)}