From 5b8c49ddf4c891fa7df2304f214eb506a912aa54 Mon Sep 17 00:00:00 2001 From: alhajrahmoun Date: Sat, 19 Jul 2025 17:54:11 +0200 Subject: [PATCH 1/2] Set html[dir] attribute in PageRenderer --- src/core/drive/page_renderer.js | 12 ++++++++++++ src/core/drive/page_snapshot.js | 4 ++++ src/tests/fixtures/dir_rtl.html | 12 ++++++++++++ src/tests/fixtures/rendering.html | 1 + src/tests/functional/rendering_tests.js | 7 +++++++ 5 files changed, 36 insertions(+) create mode 100644 src/tests/fixtures/dir_rtl.html diff --git a/src/core/drive/page_renderer.js b/src/core/drive/page_renderer.js index c2789639b..8ca3b6ea8 100644 --- a/src/core/drive/page_renderer.js +++ b/src/core/drive/page_renderer.js @@ -30,6 +30,7 @@ export class PageRenderer extends Renderer { async prepareToRender() { this.#setLanguage() + this.#setDirection() await this.mergeHead() } @@ -69,6 +70,17 @@ export class PageRenderer extends Renderer { } } + #setDirection() { + const { documentElement } = this.currentSnapshot + const { dir } = this.newSnapshot + + if (dir) { + documentElement.setAttribute("dir", dir) + } else { + documentElement.removeAttribute("dir") + } + } + async mergeHead() { const mergedHeadElements = this.mergeProvisionalElements() const newStylesheetElements = this.copyNewHeadStylesheetElements() diff --git a/src/core/drive/page_snapshot.js b/src/core/drive/page_snapshot.js index 4e54944c8..c1c56261d 100644 --- a/src/core/drive/page_snapshot.js +++ b/src/core/drive/page_snapshot.js @@ -45,6 +45,10 @@ export class PageSnapshot extends Snapshot { return this.documentElement.getAttribute("lang") } + get dir() { + return this.documentElement.getAttribute("dir") + } + get headElement() { return this.headSnapshot.element } diff --git a/src/tests/fixtures/dir_rtl.html b/src/tests/fixtures/dir_rtl.html new file mode 100644 index 000000000..b66dd4f0d --- /dev/null +++ b/src/tests/fixtures/dir_rtl.html @@ -0,0 +1,12 @@ + + + + + Turbo + + + + +

html[dir="rtl"]

+ + diff --git a/src/tests/fixtures/rendering.html b/src/tests/fixtures/rendering.html index f03e725a8..9772804f6 100644 --- a/src/tests/fixtures/rendering.html +++ b/src/tests/fixtures/rendering.html @@ -47,6 +47,7 @@

Rendering

Delayed link

Redirect link

Change html[lang]

+

Change html[dir]

diff --git a/src/tests/functional/rendering_tests.js b/src/tests/functional/rendering_tests.js index 22ffe741c..08279c6ae 100644 --- a/src/tests/functional/rendering_tests.js +++ b/src/tests/functional/rendering_tests.js @@ -201,6 +201,13 @@ test("changes the html[lang] attribute", async ({ page }) => { await expect(page.locator("html")).toHaveAttribute("lang", "es") }) +test("changes the html[dir] attribute", async ({ page }) => { + await page.click("#dir-rtl") + await nextEventNamed(page, "turbo:load") + + assert.equal(await page.getAttribute("html", "dir"), "rtl") +}) + test("accumulates script elements in head", async ({ page }) => { const assetElements = () => page.$$('script') const originalElements = await assetElements() From d49c8142f73530d495e9698185ecdad50abc15e2 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Sun, 9 Nov 2025 23:07:37 -0500 Subject: [PATCH 2/2] Unify `#setDirection` and `#setLanguage` Append the `#setDirection` implementation to the existing `#setLanguage` private method. Use `expect` rather than `assert` in the accompanying test coverage. Remove the `nextEventNamed("turbo:load")` test synchronization in favor of Playwright's underlying assertion retries. --- src/core/drive/page_renderer.js | 9 +-------- src/tests/fixtures/dir_rtl.html | 18 +++++++++--------- src/tests/functional/rendering_tests.js | 3 +-- 3 files changed, 11 insertions(+), 19 deletions(-) diff --git a/src/core/drive/page_renderer.js b/src/core/drive/page_renderer.js index 8ca3b6ea8..475cb1cb3 100644 --- a/src/core/drive/page_renderer.js +++ b/src/core/drive/page_renderer.js @@ -30,7 +30,6 @@ export class PageRenderer extends Renderer { async prepareToRender() { this.#setLanguage() - this.#setDirection() await this.mergeHead() } @@ -61,19 +60,13 @@ export class PageRenderer extends Renderer { #setLanguage() { const { documentElement } = this.currentSnapshot - const { lang } = this.newSnapshot + const { dir, lang } = this.newSnapshot if (lang) { documentElement.setAttribute("lang", lang) } else { documentElement.removeAttribute("lang") } - } - - #setDirection() { - const { documentElement } = this.currentSnapshot - const { dir } = this.newSnapshot - if (dir) { documentElement.setAttribute("dir", dir) } else { diff --git a/src/tests/fixtures/dir_rtl.html b/src/tests/fixtures/dir_rtl.html index b66dd4f0d..2793c0e5d 100644 --- a/src/tests/fixtures/dir_rtl.html +++ b/src/tests/fixtures/dir_rtl.html @@ -1,12 +1,12 @@ - - - Turbo - - - - -

html[dir="rtl"]

- + + + Turbo + + + + +

html[dir="rtl"]

+ diff --git a/src/tests/functional/rendering_tests.js b/src/tests/functional/rendering_tests.js index 08279c6ae..2f50729eb 100644 --- a/src/tests/functional/rendering_tests.js +++ b/src/tests/functional/rendering_tests.js @@ -203,9 +203,8 @@ test("changes the html[lang] attribute", async ({ page }) => { test("changes the html[dir] attribute", async ({ page }) => { await page.click("#dir-rtl") - await nextEventNamed(page, "turbo:load") - assert.equal(await page.getAttribute("html", "dir"), "rtl") + await expect(page.locator("html")).toHaveAttribute("dir", "rtl") }) test("accumulates script elements in head", async ({ page }) => {