Re-encode view-transition-names #10099
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 1633717 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
ematipico
reviewed
Feb 13, 2024
| const reEncodeInValidStart: string[] = "-0123456789" | ||
| .split('').reduce((v, c) => (v[c.charCodeAt(0)] = c, v), [] as string[]); | ||
|
|
||
| function reEncode(s: string) { |
There was a problem hiding this comment.
Can you add some comments that explain the re-encoding logic?
There was a problem hiding this comment.
Sure, I can't understand why I didn't do that from the start ;-)
matthewp
approved these changes
Feb 13, 2024
sarah11918
reviewed
Feb 13, 2024
There was a problem hiding this comment.
Some thoughts on the changeset here, for your consideration @martrapp ! 🙌
|
I didn't find anything that explains why it's being requested by the browser. |
Co-authored-by: Sarah Rainsberger <[email protected]>
ematipico
approved these changes
Feb 14, 2024
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
We have observed regressions with the cssesc view-transition-name encoding as Chrome seems to have issues with some specification-compliant view transition names. Specifically with backslash-escaped, printable ASCII letters like
\.,\,\$but alos\30which would encode a '0'.We fix this on the Astro side. Closes #10081
Instead of removing all problematic characters or replacing them with
_(as we did in our original solution), this fix re-encodes them in a way that works for cssesc/Chrome, still supports all Unicode characters > 0x7f, and doesn't create collisions for different inputs.Problematic characters are replaced by their character code. This is somewhat more difficult to read in the generated CSS, but is more robust. For example
Welcome to Astro.is now encoded asWelcome20_to20_Astro2e_.Testing
Adapted and extended existing e2e tests
(Which only shows that Chrome accepts the encoding but not whether it acts on them.Did that.We could add animation event listeners to be sure.)
Docs
bug fix, no implication on documentation of transition:name
/cc @withastro/maintainers-docs for feedback on changeset