From bfaf442935fae198eeb14de1f4a78309e358f6ab Mon Sep 17 00:00:00 2001 From: John McLear Date: Mon, 25 May 2026 15:10:12 +0100 Subject: [PATCH] fix(pad): URL view-option params lost to padeditor.init race (#7840) (#7843) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(pad): URL view-option params lost to padeditor.init race (#7840) `?showLineNumbers=false` and `?useMonospaceFont=true` were being silently clobbered shortly after pad load. Same race that affected `?rtl=false` before #7464: 1. _afterHandshake → getParams() sets settings.LineNumbersDisabled (or useMonospaceFontGlobal, noColors). 2. _afterHandshake calls padeditor.init(view).then(postAceInit) — async; ace iframes still loading. 3. Sync tail of _afterHandshake hits the URL-param overrides and calls changeViewOption('showLineNumbers', false) etc. These queue setProperty('showslinenumbers', false) in Ace2Editor's actionsPendingInit queue (loaded=false). 4. ace.init resolves → loaded=true → queue flushes → URL-driven value applied. 5. padeditor.init resumes past its own await and calls setViewOptions(initialViewOptions) — initialViewOptions is built from clientVars.initialOptions.view (server defaults ∨ cookie), which does NOT carry the URL preference. The resulting setProperty('showslinenumbers', true) runs against loaded=true ace and immediately re-shows the gutter. #7464 noticed this race for RTL and moved the override into postAceInit. The neighbouring blocks for showLineNumbers / noColors / useMonospaceFontGlobal were left at the synchronous-tail site — generalise the same fix to all three. Direct-browser users typically had a `prefs` cookie with showLineNumbers=false from a prior in-pad toggle, so the initialViewOptions value happened to match the URL param and the race was unobservable. Cross-context iframe embeds (the reporter's configuration) start with no cookie, so the server default true fights the URL false and the race becomes visible. Adds src/tests/frontend-new/specs/url_view_options.spec.ts covering the showLineNumbers=false / =true / useMonospaceFont=true cases on initial-load navigation (the path where the race actually fires). Co-Authored-By: Claude Opus 4.7 (1M context) * test(types): annotate navigateWithParam helper params Fixes CI ts-check: parameters page/padId/param implicitly had `any`. Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- src/static/js/pad.ts | 35 +++++++------ .../specs/url_view_options.spec.ts | 49 +++++++++++++++++++ 2 files changed, 66 insertions(+), 18 deletions(-) create mode 100644 src/tests/frontend-new/specs/url_view_options.spec.ts diff --git a/src/static/js/pad.ts b/src/static/js/pad.ts index 1da41e0d9..219423d32 100644 --- a/src/static/js/pad.ts +++ b/src/static/js/pad.ts @@ -720,10 +720,27 @@ const pad = { pad.refreshPadSettingsControls(); pad.applyOptionsChange(); pad.refreshMyViewControls(); + // The following view overrides MUST run inside postAceInit (after + // padeditor.init resolves), not in the synchronous tail of + // _afterHandshake. Running them sync queues a setProperty in the + // Ace2Editor pending-init queue; the queue is flushed when ace + // finishes loading, but padeditor.init's own + // setViewOptions(initialViewOptions) call runs immediately *after* + // that flush and clobbers the URL-driven value. See #7464 for the + // RTL incarnation of this race (now generalised here for #7840). if (settings.rtlIsExplicit) { // URL or server config explicitly set RTL — takes priority over cookie pad.changeViewOption('rtlIsTrue', settings.rtlIsTrue === true); } + if (settings.LineNumbersDisabled === true) { + pad.changeViewOption('showLineNumbers', false); + } + if (settings.noColors === true) { + pad.changeViewOption('noColors', true); + } + if (settings.useMonospaceFontGlobal === true) { + pad.changeViewOption('padFontFamily', 'RobotoMono'); + } // Prevent sticky chat or chat and users to be checked for mobiles const checkChatAndUsersVisibility = (x) => { @@ -809,24 +826,6 @@ const pad = { ace.ace_setEditable(!window.clientVars.readonly); }); - // If the LineNumbersDisabled value is set to true then we need to hide the Line Numbers - if (settings.LineNumbersDisabled === true) { - this.changeViewOption('showLineNumbers', false); - } - - // If the noColors value is set to true then we need to - // hide the background colors on the ace spans - if (settings.noColors === true) { - this.changeViewOption('noColors', true); - } - - // RTL override is applied in postAceInit (after padeditor.init resolves) - // to avoid a race where setViewOptions(initialViewOptions) overwrites it. - - // If the Monospacefont value is set to true then change it to monospace. - if (settings.useMonospaceFontGlobal === true) { - this.changeViewOption('padFontFamily', 'RobotoMono'); - } // if the globalUserName value is set we need to tell the server and // the client about the new authorname if (settings.globalUserName !== false) { diff --git a/src/tests/frontend-new/specs/url_view_options.spec.ts b/src/tests/frontend-new/specs/url_view_options.spec.ts new file mode 100644 index 000000000..55914c97b --- /dev/null +++ b/src/tests/frontend-new/specs/url_view_options.spec.ts @@ -0,0 +1,49 @@ +import {expect, Page, test} from '@playwright/test'; +import {goToNewPad} from '../helper/padHelper'; + +// Covers the URL-param view overrides that share the same _afterHandshake +// race as rtl=false (fixed in #7464). Each test loads a pad with the URL +// param in the initial navigation so the race actually fires, instead of +// applying the param via a second goto on an already-initialized pad. + +test.beforeEach(async ({context}) => { + await context.clearCookies(); +}); + +const navigateWithParam = async (page: Page, padId: string, param: string) => { + await page.goto(`http://localhost:9001/p/${padId}?${param}`); + await page.waitForSelector('iframe[name="ace_outer"]'); + await page.waitForSelector('#editorcontainer.initialized'); +}; + +test.describe('URL-param view options apply on initial load (race-free)', function () { + test('?showLineNumbers=false hides the line-number gutter on first paint', async function ({page}) { + const padId = await goToNewPad(page); + await navigateWithParam(page, padId, 'showLineNumbers=false'); + + const outerBody = page.frameLocator('iframe[name="ace_outer"]').locator('body'); + await expect(outerBody).toHaveClass(/line-numbers-hidden/); + await expect(page.locator('#options-linenoscheck')).not.toBeChecked(); + }); + + test('?showLineNumbers=true keeps the gutter visible (no regression)', async function ({page}) { + const padId = await goToNewPad(page); + await navigateWithParam(page, padId, 'showLineNumbers=true'); + + const outerBody = page.frameLocator('iframe[name="ace_outer"]').locator('body'); + await expect(outerBody).not.toHaveClass(/line-numbers-hidden/); + await expect(page.locator('#options-linenoscheck')).toBeChecked(); + }); + + test('?useMonospaceFont=true applies monospace on first paint', async function ({page}) { + const padId = await goToNewPad(page); + await navigateWithParam(page, padId, 'useMonospaceFont=true'); + + // padFontFamily=RobotoMono is mirrored onto the inner body via + // setProperty('textface', ...) AND onto the #viewfontmenu select; + // the same race that drops showLineNumbers also drops the select + // value back to empty when the param fires before padeditor.init + // resolves. + await expect(page.locator('#viewfontmenu')).toHaveValue('RobotoMono'); + }); +});