fix(pad): URL view-option params lost to padeditor.init race (#7840) (#7843)

* 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) <noreply@anthropic.com>

* 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) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
John McLear 2026-05-25 15:10:12 +01:00 committed by GitHub
parent 79e3d46127
commit bfaf442935
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 66 additions and 18 deletions

View file

@ -720,10 +720,27 @@ const pad = {
pad.refreshPadSettingsControls(); pad.refreshPadSettingsControls();
pad.applyOptionsChange(); pad.applyOptionsChange();
pad.refreshMyViewControls(); 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) { if (settings.rtlIsExplicit) {
// URL or server config explicitly set RTL — takes priority over cookie // URL or server config explicitly set RTL — takes priority over cookie
pad.changeViewOption('rtlIsTrue', settings.rtlIsTrue === true); 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 // Prevent sticky chat or chat and users to be checked for mobiles
const checkChatAndUsersVisibility = (x) => { const checkChatAndUsersVisibility = (x) => {
@ -809,24 +826,6 @@ const pad = {
ace.ace_setEditable(!window.clientVars.readonly); 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 // if the globalUserName value is set we need to tell the server and
// the client about the new authorname // the client about the new authorname
if (settings.globalUserName !== false) { if (settings.globalUserName !== false) {

View file

@ -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');
});
});