From b340f8fe3aaa81e38c4f1aa41498b159dc733d86 Mon Sep 17 00:00:00 2001 From: Martin Trapp <94928215+martrapp@users.noreply.github.com> Date: Wed, 14 Feb 2024 15:07:31 +0100 Subject: [PATCH] Re-encode view-transition-names (#10099) * Fixes an issue with view transition names containing spaces or punctuation. * reworked, more robust approach * better readability and also escapes the escape character (_) * update changeset * add comemnts to describe the re-encoding * updated changeset * typos * Apply suggestions from code review Co-authored-by: Sarah Rainsberger * simplify decoding it ever required. * better coverage and now also checks animation behavior --------- Co-authored-by: Sarah Rainsberger Co-authored-by: Emanuele Stoppa --- .changeset/many-lobsters-accept.md | 11 +++ .../src/pages/transition-name.astro | 21 ++++ packages/astro/e2e/view-transitions.test.js | 95 +++++++++++-------- .../astro/src/runtime/server/transition.ts | 37 +++++++- 4 files changed, 124 insertions(+), 40 deletions(-) create mode 100644 .changeset/many-lobsters-accept.md diff --git a/.changeset/many-lobsters-accept.md b/.changeset/many-lobsters-accept.md new file mode 100644 index 0000000000..12c5a12914 --- /dev/null +++ b/.changeset/many-lobsters-accept.md @@ -0,0 +1,11 @@ +--- +"astro": minor +--- + +Fixes a regression where view transition names containing special characters such as spaces or punctuation stopped working. + +Regular use naming your transitions with `transition: name` is unaffected. + +However, this fix may result in breaking changes if your project relies on the particular character encoding strategy Astro uses to translate `transition:name` directives into values of the underlying CSS `view-transition-name` property. For example, `Welcome to Astro` is now encoded as `Welcome_20to_20Astro_2e`. + +This mainly affects spaces and punctuation marks but no Unicode characters with codes >= 128. diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/transition-name.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/transition-name.astro index 22fb47aab6..85c42ee0ef 100644 --- a/packages/astro/e2e/fixtures/view-transitions/src/pages/transition-name.astro +++ b/packages/astro/e2e/fixtures/view-transitions/src/pages/transition-name.astro @@ -13,4 +13,25 @@ import Layout from '../components/Layout.astro';
--9
10
-11
+
#! /
+
_01__02___
+
control chars
+
punctation & numbers
+
upper case chars
+
lower case chars
+
80-9f
+
a0-bf
+
c0-df
+
e0-ff
+ navigate + diff --git a/packages/astro/e2e/view-transitions.test.js b/packages/astro/e2e/view-transitions.test.js index a9626c09e9..3d68f59419 100644 --- a/packages/astro/e2e/view-transitions.test.js +++ b/packages/astro/e2e/view-transitions.test.js @@ -1305,50 +1305,69 @@ test.describe('View Transitions', () => { }); test('transition:name should be escaped correctly', async ({ page, astro }) => { + const expectedAnimations = new Set(); + const checkName = async (selector, name) => { + expectedAnimations.add(name); + expect(page.locator(selector), 'should be escaped correctly').toHaveCSS( + 'view-transition-name', + name + ); + }; + + page.on('console', (msg) => { + if (msg.text().startsWith('anim: ')) { + const split = msg.text().split(' ', 2); + expectedAnimations.delete(split[1]); + } + }); + await page.goto(astro.resolveUrl('/transition-name')); - await expect(page.locator('#one'), 'should be escaped correctly').toHaveCSS( - 'view-transition-name', - 'front-end' + + await checkName('#one', 'front-end'); + await checkName('#two', '开源'); + await checkName('#three', '开a源'); + await checkName('#four', 'c开a源c'); + await checkName('#five', 'オープンソース'); + await checkName('#six', '开_24源'); + await checkName('#seven', '开_2e源'); + await checkName('#eight', '🐎👱❤'); + await checkName('#nine', '_--9'); + await checkName('#ten', '_10'); + await checkName('#eleven', '_-11'); + await checkName('#twelve', '__23_21_20_2f'); + await checkName('#thirteen', '___01____02______'); + await checkName( + '#batch0', + '__00_01_02_03_04_05_06_07_08_09_0a_0b_0c_0d_0e_0f_10_11_12_13_14_15_16_17_18_19_1a_1b_1c_1d_1e_1f' ); - await expect(page.locator('#two'), 'should be escaped correctly').toHaveCSS( - 'view-transition-name', - '开源' + await checkName( + '#batch1', + '__20_21_22_23_24_25_26_27_28_29_2a_2b_2c-_2e_2f0123456789_3a_3b_3c_3d_3e_3f' ); - await expect(page.locator('#three'), 'should be escaped correctly').toHaveCSS( - 'view-transition-name', - '开a源' + await checkName('#batch2', '__40ABCDEFGHIJKLMNOPQRSTUVWXYZ_5b_5c_5d_5e__'); + await checkName('#batch3', '__60abcdefghijklmnopqrstuvwxyz_7b_7c_7d_7e_7f'); + await checkName( + '#batch4', + '\x80\x81\x82\x83\x84\x85\x86\x87\x88\x89\x8a\x8b\x8c\x8d\x8e\x8f\x90\x91\x92\x93\x94\x95\x96\x97\x98\x99\x9a\x9b\x9c\x9d\x9e\x9f' ); - await expect(page.locator('#four'), 'should be escaped correctly').toHaveCSS( - 'view-transition-name', - 'c开a源c' + await checkName( + '#batch5', + '\xa0\xa1\xa2\xa3\xa4\xa5\xa6\xa7\xa8\xa9\xaa\xab\xac\xad\xae\xaf\xb0\xb1\xb2\xb3\xb4\xb5\xb6\xb7\xb8\xb9\xba\xbb\xbc\xbd\xbe\xbf' ); - await expect(page.locator('#five'), 'should be escaped correctly').toHaveCSS( - 'view-transition-name', - 'オープンソース' + await checkName( + '#batch6', + '\xc0\xc1\xc2\xc3\xc4\xc5\xc6\xc7\xc8\xc9\xca\xcb\xcc\xcd\xce\xcf\xd0\xd1\xd2\xd3\xd4\xd5\xd6\xd7\xd8\xd9\xda\xdb\xdc\xdd\xde\xdf' ); - await expect(page.locator('#six'), 'should be escaped correctly').toHaveCSS( - 'view-transition-name', - '开\\$源' - ); - await expect(page.locator('#seven'), 'should be escaped correctly').toHaveCSS( - 'view-transition-name', - '开\\.源' - ); - await expect(page.locator('#eight'), 'should be escaped correctly').toHaveCSS( - 'view-transition-name', - '🐎👱❤' - ); - await expect(page.locator('#nine'), 'should be escaped correctly').toHaveCSS( - 'view-transition-name', - '--9' - ); - await expect(page.locator('#ten'), 'should be escaped correctly').toHaveCSS( - 'view-transition-name', - '\\31 0' - ); - await expect(page.locator('#eleven'), 'should be escaped correctly').toHaveCSS( - 'view-transition-name', - '-\\31 1' + await checkName( + '#batch7', + '\xe0\xe1\xe2\xe3\xe4\xe5\xe6\xe7\xe8\xe9\xea\xeb\xec\xed\xee\xef\xf0\xf1\xf2\xf3\xf4\xf5\xf6\xf7\xf8\xf9\xfa\xfb\xfc\xfd\xfe\xff' ); + + await page.click('#navigate'); + await page.waitForTimeout(400); // yes, I dislike this, too. Might fix later. + expect( + expectedAnimations.size, + 'all animations for transition:names should have been found' + ).toEqual(0); }); }); diff --git a/packages/astro/src/runtime/server/transition.ts b/packages/astro/src/runtime/server/transition.ts index 477f6b5e4a..3d443df920 100644 --- a/packages/astro/src/runtime/server/transition.ts +++ b/packages/astro/src/runtime/server/transition.ts @@ -45,6 +45,39 @@ const addPairs = ( } }; +// Chrome (121) accepts custom-idents for view-transition-names as generated by cssesc, +// but it just ignores them during view transitions if they contain escaped 7-bit ASCII characters +// like \ or \. A special case are digits and minus at the beginning of the string, +// which cssesc also encodes as \xx +const reEncodeValidChars: string[] = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ-_" + .split('').reduce((v, c) => (v[c.charCodeAt(0)] = c, v), [] as string[]); +const reEncodeInValidStart: string[] = "-0123456789_" + .split('').reduce((v, c) => (v[c.charCodeAt(0)] = c, v), [] as string[]); + +function reEncode(s: string) { + let result = ''; + let codepoint; + // we work on codepoints that might use more than 16bit, not character codes. + // so the index will often step by 1 as usual or by 2 if the codepoint is greater than 0xFFFF + for (let i = 0; i < s.length; i += (codepoint ?? 0) > 0xFFFF ? 2 : 1) { + codepoint = s.codePointAt(i); + if (codepoint !== undefined) { // this should never happen, they said! + + // If we find a character in the range \x00 - \x7f that is not one of the reEncodeValidChars, + // we replace it with its hex value escaped by an underscore for decodability (and better readability, + // because most of them are punctuations like ,'"":;_..., and '_' might be a better choice than '-') + // The underscore itself (code 95) is also escaped and encoded as two underscores to avoid + // collitions between original and encoded strings. + // All other values are just copied over + result += codepoint < 0x80 + ? (codepoint === 95 ? "__" : (reEncodeValidChars[codepoint] ?? ('_' + (codepoint.toString(16).padStart(2, "0"))))) + : String.fromCodePoint(codepoint); + } + } + // Digits and minus sign at the beginning of the string are special, so we simply prepend an underscore + return reEncodeInValidStart[result.codePointAt(0) ?? 0] ? '_' + result : result; +} + export function renderTransition( result: SSRResult, hash: string, @@ -54,7 +87,7 @@ export function renderTransition( // Default to `fade` (similar to `initial`, but snappier) if (!animationName) animationName = 'fade'; const scope = createTransitionScope(result, hash); - const name = transitionName ? cssesc(transitionName, { isIdentifier: true }) : scope; + const name = transitionName ? cssesc(reEncode(transitionName), { isIdentifier: true }) : scope; const sheet = new ViewTransitionStyleSheet(scope, name); const animations = getAnimations(animationName); @@ -90,7 +123,7 @@ class ViewTransitionStyleSheet { constructor( private scope: string, private name: string - ) {} + ) { } toString() { const { scope, name } = this;