From 61aee16cf611cb2d6dabea87a51f770ffe066a17 Mon Sep 17 00:00:00 2001 From: John McLear Date: Fri, 5 Jun 2026 13:24:26 +0100 Subject: [PATCH] fix: RTL content option no longer flips the whole page (#7900) (#7901) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: RTL content option no longer flips the whole page (#7900) 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) * test: address Qodo review — robust frame access and locale-independent page-dir assertion (#7900) - 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) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- src/static/js/ace2_inner.ts | 6 ++++- .../frontend-new/specs/rtl_url_param.spec.ts | 26 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/static/js/ace2_inner.ts b/src/static/js/ace2_inner.ts index 8c32a222f..438f15fe2 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 f094da101..e93106065 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,30 @@ 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}) { + // 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. 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/); + await expect.poll(() => innerBody.evaluate((el) => + el.ownerDocument.defaultView!.getComputedStyle(el).direction)).toBe('rtl'); + + // ...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!); + }); });