From a164a6393c74dc3efa5cae7758fd6cc603a0cfef Mon Sep 17 00:00:00 2001 From: John McLear Date: Fri, 5 Jun 2026 12:39:57 +0100 Subject: [PATCH 1/2] fix: RTL content option no longer flips the whole page (#7900) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pad's RTL content option (rtlIsTrue) is a per-pad setting that should only change the direction of the editor's text contents. The setter in ace2_inner.ts wrote the direction to the top-level `document.documentElement` (`document` there resolves to the main pad page, since the editor iframes are derived from it), which flipped the entire page — toolbar and chrome included. The page direction is owned solely by the UI language (l10n.ts sets it from html10n.getDirection()). Apply the content direction to the inner editor document (`targetDoc.documentElement`) instead, so enabling RTL content leaves the surrounding interface untouched. Adds a frontend test asserting the inner editor flips to RTL while the top-level dir stays ltr. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/static/js/ace2_inner.ts | 6 +++++- .../frontend-new/specs/rtl_url_param.spec.ts | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/static/js/ace2_inner.ts b/src/static/js/ace2_inner.ts index 8c32a222f5c..438f15fe2b3 100644 --- a/src/static/js/ace2_inner.ts +++ b/src/static/js/ace2_inner.ts @@ -683,7 +683,11 @@ function Ace2Inner(editorInfo, cssManagers) { rtlistrue: (value) => { targetBody.classList.toggle('rtl', value); targetBody.classList.toggle('ltr', !value); - document.documentElement.dir = value ? 'rtl' : 'ltr'; + // Apply the base direction to the inner editor document only. Using the + // top-level `document` here would flip the whole page (toolbar, chrome) + // even though this is a per-pad content option — see issue #7900. The + // page direction is governed solely by the UI language (see l10n.ts). + targetDoc.documentElement.dir = value ? 'rtl' : 'ltr'; }, fadeinactiveauthorcolors: (value) => { fadeInactiveAuthorColors = `${value}` !== 'false'; diff --git a/src/tests/frontend-new/specs/rtl_url_param.spec.ts b/src/tests/frontend-new/specs/rtl_url_param.spec.ts index f094da1016e..1e538b8ac83 100644 --- a/src/tests/frontend-new/specs/rtl_url_param.spec.ts +++ b/src/tests/frontend-new/specs/rtl_url_param.spec.ts @@ -34,4 +34,20 @@ test.describe('RTL URL parameter', function () { await page.waitForSelector('#editorcontainer.initialized'); await expect(page.locator('#options-rtlcheck')).not.toBeChecked(); }); + + test('rtl content option flips only the pad inner contents, not the whole page', {tag: '@feature:rtl-toggle'}, async function ({page}) { + await appendQueryParams(page, {rtl: 'true'}); + await expect(page.locator('#options-rtlcheck')).toBeChecked(); + + // The inner editor document is flipped to RTL... + const innerBody = page.frame('ace_inner')!.locator('#innerdocbody'); + await expect(innerBody).toHaveClass(/\brtl\b/); + const innerDirection = await innerBody.evaluate((el) => + el.ownerDocument.defaultView!.getComputedStyle(el).direction); + expect(innerDirection).toBe('rtl'); + + // ...but the top-level page (toolbar, chrome) is governed by the UI + // language, not the pad's RTL content option, and must stay LTR. + await expect(page.locator('html')).toHaveAttribute('dir', 'ltr'); + }); }); From 46b0e8b4b5c18f90ddfdb193f2d2f77fa5724819 Mon Sep 17 00:00:00 2001 From: John McLear Date: Fri, 5 Jun 2026 13:08:35 +0100 Subject: [PATCH 2/2] =?UTF-8?q?test:=20address=20Qodo=20review=20=E2=80=94?= =?UTF-8?q?=20robust=20frame=20access=20and=20locale-independent=20page-di?= =?UTF-8?q?r=20assertion=20(#7900)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Use a frameLocator chain (Playwright auto-waits) instead of page.frame('ace_inner')! which can race with inner-editor readiness. - Don't hardcode dir='ltr' on the top-level (the UI language can legitimately pick rtl). Capture the initial page dir and assert it is unchanged after toggling the pad RTL option — that is the real invariant. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../frontend-new/specs/rtl_url_param.spec.ts | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/tests/frontend-new/specs/rtl_url_param.spec.ts b/src/tests/frontend-new/specs/rtl_url_param.spec.ts index 1e538b8ac83..e931060651c 100644 --- a/src/tests/frontend-new/specs/rtl_url_param.spec.ts +++ b/src/tests/frontend-new/specs/rtl_url_param.spec.ts @@ -36,18 +36,28 @@ test.describe('RTL URL parameter', function () { }); test('rtl content option flips only the pad inner contents, not the whole page', {tag: '@feature:rtl-toggle'}, async function ({page}) { + // The top-level page direction is owned by the UI language, not the pad's + // RTL content option. Capture whatever the language chose so the assertion + // is locale-independent (it could legitimately be rtl on an RTL locale). + const html = page.locator('html'); + await expect(html).toHaveAttribute('dir', /^(ltr|rtl)$/); + const initialPageDir = await html.getAttribute('dir'); + await appendQueryParams(page, {rtl: 'true'}); await expect(page.locator('#options-rtlcheck')).toBeChecked(); - // The inner editor document is flipped to RTL... - const innerBody = page.frame('ace_inner')!.locator('#innerdocbody'); + // The inner editor document is flipped to RTL. Use a frameLocator chain so + // Playwright auto-waits for the nested iframes/body to be ready. + const innerBody = page + .frameLocator('iframe[name="ace_outer"]') + .frameLocator('iframe[name="ace_inner"]') + .locator('#innerdocbody'); await expect(innerBody).toHaveClass(/\brtl\b/); - const innerDirection = await innerBody.evaluate((el) => - el.ownerDocument.defaultView!.getComputedStyle(el).direction); - expect(innerDirection).toBe('rtl'); + await expect.poll(() => innerBody.evaluate((el) => + el.ownerDocument.defaultView!.getComputedStyle(el).direction)).toBe('rtl'); - // ...but the top-level page (toolbar, chrome) is governed by the UI - // language, not the pad's RTL content option, and must stay LTR. - await expect(page.locator('html')).toHaveAttribute('dir', 'ltr'); + // ...but the top-level page (toolbar, chrome) is unaffected: its dir is + // whatever the UI language set and must not change when the pad flips. + await expect(html).toHaveAttribute('dir', initialPageDir!); }); });