Skip to content

Commit 9e8f9cb

Browse files
JohnMcLearclaude
andauthored
fix: pad-wide view settings apply to the creator's own view (#7900) (#7902)
* fix: pad-wide view settings apply to the creator's own view (#7900) 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) <noreply@anthropic.com> * test: address Qodo review — clear cookies on the page's own context (#7900) 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 9af77e7 commit 9e8f9cb

2 files changed

Lines changed: 70 additions & 0 deletions

File tree

src/static/js/pad.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -876,6 +876,14 @@ const pad = {
876876
options,
877877
changedBy: pad.myUserInfo.name || 'unnamed',
878878
});
879+
// The pad creator is never "enforced upon themselves", so their personal
880+
// view overrides (cookies) are always merged on top of the pad-wide value
881+
// in getEffectivePadOptions. A stale personal pref would therefore mask the
882+
// pad-wide value they just set, making the control appear to do nothing
883+
// (#7900). Sync the creator's personal pref to the value they chose so
884+
// their own view adopts it immediately. They can still override it
885+
// afterwards via the "My view" controls.
886+
pad.setMyViewOption(key, value);
879887
},
880888
changeViewOption: (key, value) => {
881889
const effectiveOptions = pad.getEffectivePadOptions();
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import {expect, test} from "@playwright/test";
2+
import {goToNewPad} from "../helper/padHelper";
3+
import {showSettings} from "../helper/settingsHelper";
4+
5+
// Regression test for ether/etherpad#7900.
6+
//
7+
// The pad creator is never "enforced upon themselves" (they can always edit
8+
// pad settings), so their personal view overrides (cookies) are always merged
9+
// on top of the pad-wide options in getEffectivePadOptions. A creator who had
10+
// at some point toggled their personal "Read content from right to left"
11+
// carried a stale rtlIsTrue=false cookie that silently masked the pad-wide RTL
12+
// value they later set — the pad content stayed LTR and the pad-wide control
13+
// appeared to "do nothing".
14+
//
15+
// Fix: changePadViewOption syncs the creator's personal pref to the value they
16+
// chose, so their own view adopts the pad-wide setting immediately (while still
17+
// allowing them to override it afterwards via the "My view" controls).
18+
test.beforeEach(async ({page}) => {
19+
// Clear cookies on the context of the page under test (not a throwaway
20+
// context) so the test reliably starts without a stale rtlIsTrue pref.
21+
await page.context().clearCookies();
22+
await goToNewPad(page);
23+
});
24+
25+
test.describe('RTL pad-wide + enforce', function () {
26+
test('pad-wide RTL applies to the creator even with a stale personal setting', {tag: '@feature:rtl-toggle'}, async function ({page}) {
27+
const innerBody = page
28+
.frameLocator('iframe[name="ace_outer"]')
29+
.frameLocator('iframe[name="ace_inner"]')
30+
.locator('#innerdocbody');
31+
const computedDir = () => innerBody.evaluate((el) =>
32+
el.ownerDocument.defaultView!.getComputedStyle(el).direction);
33+
34+
await showSettings(page);
35+
36+
// The checkboxes are visually replaced by styled labels, so drive the UI
37+
// the way a user does — by clicking the labels.
38+
// The creator first toggles their PERSONAL RTL on, then off. This writes a
39+
// personal cookie pref rtlIsTrue=false that used to mask the pad-wide value.
40+
await page.locator('label[for="options-rtlcheck"]').click();
41+
await expect(page.locator('#options-rtlcheck')).toBeChecked();
42+
await expect.poll(computedDir).toBe('rtl');
43+
await page.locator('label[for="options-rtlcheck"]').click();
44+
await expect(page.locator('#options-rtlcheck')).not.toBeChecked();
45+
await expect.poll(computedDir).toBe('ltr');
46+
47+
// Pad-wide settings are visible because the new pad's first user is its
48+
// creator (canEditPadSettings). Setting pad-wide RTL must now flip the
49+
// creator's own content despite the stale personal cookie...
50+
await page.locator('label[for="padsettings-options-rtlcheck"]').click();
51+
await expect(page.locator('#padsettings-options-rtlcheck')).toBeChecked();
52+
await expect.poll(computedDir).toBe('rtl');
53+
await expect(innerBody).toHaveClass(/\brtl\b/);
54+
// ...and the personal control reflects the synced value.
55+
await expect(page.locator('#options-rtlcheck')).toBeChecked();
56+
57+
// Enforcing for other users keeps the creator's content RTL.
58+
await page.locator('label[for="padsettings-enforcecheck"]').click();
59+
await expect(page.locator('#padsettings-enforcecheck')).toBeChecked();
60+
await expect.poll(computedDir).toBe('rtl');
61+
});
62+
});

0 commit comments

Comments
 (0)