fix: RTL content option no longer flips the whole page (#7900) (#7901)

* 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 <html> dir stays ltr.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

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

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
John McLear 2026-06-05 13:24:26 +01:00 committed by GitHub
parent 0110d880aa
commit 61aee16cf6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 31 additions and 1 deletions

View file

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

View file

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