From ed647ae091e3ecdb4501ab67efe5f102e7547f99 Mon Sep 17 00:00:00 2001 From: John McLear Date: Fri, 5 Jun 2026 13:02:08 +0100 Subject: [PATCH 1/2] fix: pad-wide view settings apply to the creator's own view (#7900) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pad creator is never "enforced upon themselves" (isPadSettingsEnforcedForMe is false whenever canEditPadSettings is true), so getEffectivePadOptions always merges their personal view overrides (cookies) on top of the pad-wide options. As a result, a creator who had at some point toggled e.g. their personal "Read content from right to left" carried a stale rtlIsTrue=false cookie that silently masked the pad-wide value they later set — toggling the pad-wide control (and then enforcing it) appeared to do nothing on the creator's own screen. Fix: when the creator changes a pad-wide view option, sync their personal pref to the chosen value so their own view adopts the pad-wide setting immediately. This deliberately does NOT change the precedence model — the creator can still override the setting afterwards via the "My view" controls, so the existing behaviour where a creator keeps personal overrides while enforcing settings for other users (pad_settings.spec.ts) is preserved. Adds a frontend test reproducing the stale-personal-cookie scenario. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/static/js/pad.ts | 8 +++ .../specs/rtl_pad_wide_enforce.spec.ts | 61 +++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 src/tests/frontend-new/specs/rtl_pad_wide_enforce.spec.ts diff --git a/src/static/js/pad.ts b/src/static/js/pad.ts index 219423d32cd..7e081cb7735 100644 --- a/src/static/js/pad.ts +++ b/src/static/js/pad.ts @@ -876,6 +876,14 @@ const pad = { options, changedBy: pad.myUserInfo.name || 'unnamed', }); + // The pad creator is never "enforced upon themselves", so their personal + // view overrides (cookies) are always merged on top of the pad-wide value + // in getEffectivePadOptions. A stale personal pref would therefore mask the + // pad-wide value they just set, making the control appear to do nothing + // (#7900). Sync the creator's personal pref to the value they chose so + // their own view adopts it immediately. They can still override it + // afterwards via the "My view" controls. + pad.setMyViewOption(key, value); }, changeViewOption: (key, value) => { const effectiveOptions = pad.getEffectivePadOptions(); diff --git a/src/tests/frontend-new/specs/rtl_pad_wide_enforce.spec.ts b/src/tests/frontend-new/specs/rtl_pad_wide_enforce.spec.ts new file mode 100644 index 00000000000..4d246837c66 --- /dev/null +++ b/src/tests/frontend-new/specs/rtl_pad_wide_enforce.spec.ts @@ -0,0 +1,61 @@ +import {expect, test} from "@playwright/test"; +import {goToNewPad} from "../helper/padHelper"; +import {showSettings} from "../helper/settingsHelper"; + +// Regression test for ether/etherpad#7900. +// +// The pad creator is never "enforced upon themselves" (they can always edit +// pad settings), so their personal view overrides (cookies) are always merged +// on top of the pad-wide options in getEffectivePadOptions. A creator who had +// at some point toggled their personal "Read content from right to left" +// carried a stale rtlIsTrue=false cookie that silently masked the pad-wide RTL +// value they later set — the pad content stayed LTR and the pad-wide control +// appeared to "do nothing". +// +// Fix: changePadViewOption syncs the creator's personal pref to the value they +// chose, so their own view adopts the pad-wide setting immediately (while still +// allowing them to override it afterwards via the "My view" controls). +test.beforeEach(async ({page, browser}) => { + const context = await browser.newContext(); + await context.clearCookies(); + await goToNewPad(page); +}); + +test.describe('RTL pad-wide + enforce', function () { + test('pad-wide RTL applies to the creator even with a stale personal setting', {tag: '@feature:rtl-toggle'}, async function ({page}) { + const innerBody = page + .frameLocator('iframe[name="ace_outer"]') + .frameLocator('iframe[name="ace_inner"]') + .locator('#innerdocbody'); + const computedDir = () => innerBody.evaluate((el) => + el.ownerDocument.defaultView!.getComputedStyle(el).direction); + + await showSettings(page); + + // The checkboxes are visually replaced by styled labels, so drive the UI + // the way a user does — by clicking the labels. + // The creator first toggles their PERSONAL RTL on, then off. This writes a + // personal cookie pref rtlIsTrue=false that used to mask the pad-wide value. + await page.locator('label[for="options-rtlcheck"]').click(); + await expect(page.locator('#options-rtlcheck')).toBeChecked(); + await expect.poll(computedDir).toBe('rtl'); + await page.locator('label[for="options-rtlcheck"]').click(); + await expect(page.locator('#options-rtlcheck')).not.toBeChecked(); + await expect.poll(computedDir).toBe('ltr'); + + // Pad-wide settings are visible because the new pad's first user is its + // creator (canEditPadSettings). Setting pad-wide RTL must now flip the + // creator's own content despite the stale personal cookie... + await page.locator('label[for="padsettings-options-rtlcheck"]').click(); + await expect(page.locator('#padsettings-options-rtlcheck')).toBeChecked(); + await expect.poll(computedDir).toBe('rtl'); + await expect(innerBody).toHaveClass(/\brtl\b/); + // ...and the personal control reflects the synced value. + await expect(page.locator('#options-rtlcheck')).toBeChecked(); + + // Enforcing for other users keeps the creator's content RTL. + await page.locator('label[for="padsettings-enforcecheck"]').click(); + await expect(page.locator('#padsettings-enforcecheck')).toBeChecked(); + await expect.poll(computedDir).toBe('rtl'); + }); +}); From 2c20e276df3684e0c5e30d05cd956f2d0912cea3 Mon Sep 17 00:00:00 2001 From: John McLear Date: Fri, 5 Jun 2026 13:26:58 +0100 Subject: [PATCH 2/2] =?UTF-8?q?test:=20address=20Qodo=20review=20=E2=80=94?= =?UTF-8?q?=20clear=20cookies=20on=20the=20page's=20own=20context=20(#7900?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit browser.newContext()+clearCookies() cleared cookies on a throwaway context (not the page fixture under test) and leaked the context. Use page.context().clearCookies() so the regression test reliably starts without a stale rtlIsTrue pref. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/tests/frontend-new/specs/rtl_pad_wide_enforce.spec.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/tests/frontend-new/specs/rtl_pad_wide_enforce.spec.ts b/src/tests/frontend-new/specs/rtl_pad_wide_enforce.spec.ts index 4d246837c66..2efebb9a851 100644 --- a/src/tests/frontend-new/specs/rtl_pad_wide_enforce.spec.ts +++ b/src/tests/frontend-new/specs/rtl_pad_wide_enforce.spec.ts @@ -15,9 +15,10 @@ import {showSettings} from "../helper/settingsHelper"; // Fix: changePadViewOption syncs the creator's personal pref to the value they // chose, so their own view adopts the pad-wide setting immediately (while still // allowing them to override it afterwards via the "My view" controls). -test.beforeEach(async ({page, browser}) => { - const context = await browser.newContext(); - await context.clearCookies(); +test.beforeEach(async ({page}) => { + // Clear cookies on the context of the page under test (not a throwaway + // context) so the test reliably starts without a stale rtlIsTrue pref. + await page.context().clearCookies(); await goToNewPad(page); });