mirror of
https://github.com/karatektus/pl_mail.git
synced 2026-10-25 01:54:10 +00:00
Read the time when the clock connects, so Back does not show an old one
The clock only wrote the time when the minute turned. A page the browser's Back button restores carries the reading it was made with, and showed it until then. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
26f06166fa
commit
8a7d56862c
2 files changed
+109
No files matched your search
@@ -10,6 +10,13 @@ import { Controller } from "@hotwired/stimulus";
|
||||
* and a clock that is five minutes behind until the next tick is worse than
|
||||
* none.
|
||||
*
|
||||
* It also reads the time once on connecting, because the markup it connects to
|
||||
* is not always fresh from the server. The browser's Back button restores a
|
||||
* page as it was when it was left — Turbo's snapshot, or the browser's own
|
||||
* back/forward cache — and the reading in it is that old. Without this the
|
||||
* clock went back with the page and stayed wrong until the minute turned. On a
|
||||
* fresh page the reading it writes is the one already there.
|
||||
*
|
||||
* TARGETS, each optional, and each printed the way the server prints it
|
||||
* ─────────────────────────────────────────────────────────────────────
|
||||
* time "2:05 pm" / "14:05" — ClockFormat::time()
|
||||
@@ -38,13 +45,24 @@ export default class extends Controller {
|
||||
this.tick();
|
||||
}
|
||||
};
|
||||
// The back/forward cache brings the page back with this controller
|
||||
// still connected, so connect() does not run again for it.
|
||||
this.onPageShow = (event) => {
|
||||
if (event.persisted) {
|
||||
this.tick();
|
||||
this.schedule();
|
||||
}
|
||||
};
|
||||
document.addEventListener("visibilitychange", this.onVisible);
|
||||
window.addEventListener("pageshow", this.onPageShow);
|
||||
this.tick();
|
||||
this.schedule();
|
||||
}
|
||||
|
||||
disconnect() {
|
||||
clearTimeout(this.timer);
|
||||
document.removeEventListener("visibilitychange", this.onVisible);
|
||||
window.removeEventListener("pageshow", this.onPageShow);
|
||||
}
|
||||
|
||||
schedule() {
|
||||
|
||||
@@ -0,0 +1,91 @@
|
||||
import { type Page } from "@playwright/test";
|
||||
import { test, expect } from "./support/test";
|
||||
|
||||
/**
|
||||
* The clock the browser's Back button shows is the time now, not the time the
|
||||
* page was made.
|
||||
*
|
||||
* THE REPORTED BUG
|
||||
* ────────────────
|
||||
* "When going back in the browser the clock goes back too." Back does not
|
||||
* always ask the server again. A page left by a full navigation comes back out
|
||||
* of the browser's own caches, and a page Turbo snapshotted comes back as the
|
||||
* snapshot; either way the reading in it is as old as the page. ui--clock only
|
||||
* wrote the time when the minute turned, so the old reading stood until then.
|
||||
*
|
||||
* ── The one thing simulated ──────────────────────────────────────────────────
|
||||
* Which cache answers Back is the browser's business and differs between them,
|
||||
* and waiting for a page to grow old would make this minutes long. So the
|
||||
* browser's own clock is moved instead: whatever markup Back brings, the server
|
||||
* wrote it at the real time, and a clock that reads the time on connecting
|
||||
* shows the browser's. The offset is not a whole number of hours, so the
|
||||
* minutes differ whatever zone or format the user has.
|
||||
*
|
||||
* Verified by removing the tick from connect() and recompiling the assets
|
||||
* (`asset-map:compile`; the test stack serves the compiled copy, and an edit
|
||||
* that is not compiled changes nothing here): both tests then read the
|
||||
* server's minutes back.
|
||||
*/
|
||||
|
||||
const CLOCK = "[data-controller~='ui--clock']";
|
||||
const READING = `${CLOCK} [data-ui--clock-target='bare']`;
|
||||
|
||||
/** Three hours and seventeen minutes on. */
|
||||
const OFFSET_MS = (3 * 60 + 17) * 60 * 1000;
|
||||
|
||||
/** The minutes the page's clock has to show at $at, in the zone it keeps. */
|
||||
async function minutesAt(page: Page, at: number): Promise<string> {
|
||||
return page.evaluate(
|
||||
([selector, time]) => {
|
||||
const zone = document.querySelector(selector as string)?.getAttribute("data-ui--clock-zone-value") || undefined;
|
||||
|
||||
return new Intl.DateTimeFormat("en-CA", { timeZone: zone, minute: "2-digit" })
|
||||
.format(new Date(time as number))
|
||||
.padStart(2, "0");
|
||||
},
|
||||
[CLOCK, at],
|
||||
);
|
||||
}
|
||||
|
||||
async function leaveTheBrowserClockAhead(page: Page): Promise<RegExp> {
|
||||
const later = Date.now() + OFFSET_MS;
|
||||
const minutes = await minutesAt(page, later);
|
||||
|
||||
await page.clock.setFixedTime(later);
|
||||
|
||||
return new RegExp(`^\\d{1,2}:${minutes}$`);
|
||||
}
|
||||
|
||||
test.describe("the browser's back button does not bring an old time back with it", () => {
|
||||
test("to a page Turbo left", async ({ page }) => {
|
||||
await page.goto("/settings");
|
||||
await expect(page.locator(READING).first(), "the clock has to be on this page").toHaveText(/^\d{1,2}:\d{2}$/);
|
||||
|
||||
const now = await leaveTheBrowserClockAhead(page);
|
||||
|
||||
await page.evaluate(() =>
|
||||
(window as unknown as { Turbo: { visit(u: string): void } }).Turbo.visit("/mail/inbox"),
|
||||
);
|
||||
await page.waitForURL("**/mail/inbox**");
|
||||
|
||||
await page.goBack();
|
||||
await page.waitForURL("**/settings**");
|
||||
|
||||
await expect(page.locator(READING).first()).toHaveText(now);
|
||||
});
|
||||
|
||||
test("to a page left by a full navigation", async ({ page }) => {
|
||||
await page.goto("/settings");
|
||||
await expect(page.locator(READING).first(), "the clock has to be on this page").toHaveText(/^\d{1,2}:\d{2}$/);
|
||||
|
||||
const now = await leaveTheBrowserClockAhead(page);
|
||||
|
||||
// A document the browser loads itself, with no Turbo on the way out.
|
||||
await page.goto("/icons/favicon.svg");
|
||||
|
||||
await page.goBack();
|
||||
await page.waitForURL("**/settings**");
|
||||
|
||||
await expect(page.locator(READING).first()).toHaveText(now);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user