Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: set application/css-sourcemap+json default charset to unicode #764

Merged
merged 3 commits into from
Sep 5, 2023
Merged

fix: set application/css-sourcemap+json default charset to unicode #764

merged 3 commits into from
Sep 5, 2023

Conversation

chadlwilson
Copy link
Contributor

@chadlwilson chadlwilson commented Oct 21, 2022

After a Sprockets 4.x upgrade the CSS sourcemaps seemed to be assuming ASCII-8BIT and causing errors within the asset pipeline for SCSS which contained Unicode chars due to corruption going through the pipeline.

After a bit of digging, it seemed that was caused by an equivalent issue to #669 except for SCSS with Unicode in it (Encoding::UndefinedConversionError at "\xC3" from ASCII-8BIT to UTF-8). This is an issue only when debug: true is enabled for an asset and thus with sourcemap generation enabled.

In #669 (4.1.0) the default charset for JS sourcemaps was corrected to UTF-8. However the default charset for application/css+sourcemap+json is still currently ending up as ASCII-8BIT, even when source has a non-ASCII character in it. For consistency and correctness, it seems that these should also be assumed to be Unicode.

This PR also reinstates the changelog for the previously-released 4.1.1 which seemed to have been (accidentally?) removed in 17d7723 This was rather confusing when reading the changelog.

@chadlwilson
Copy link
Contributor Author

Hello @byroot - thanks for releasing 4.2.0. Do you think you could take a look at this consistency/bug fix? 🙏

@chadlwilson chadlwilson changed the title Set application/css-sourcemap+json charset to unicode fix: set application/css-sourcemap+json default charset to unicode May 25, 2023
@rafaelfranca rafaelfranca merged commit 5d26375 into rails:main Sep 5, 2023
13 of 14 checks passed
@chadlwilson chadlwilson deleted the fix-css-sourcemap-default-charset branch September 6, 2023 00:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants