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

Treat <title>, <script>, and <style> contents as text #69

Merged
merged 4 commits into from
Jul 22, 2019

Conversation

CvX
Copy link
Contributor

@CvX CvX commented Jun 29, 2019

Fixes #21

This PR is one of the fixes that aim to improve <title>, <script>, and <style> support in Ember FastBoot (see: glimmerjs/glimmer-vm#796 (comment))

cc: @rwjblue

@josemarluedke
Copy link

Tests look good! Looking forward to having these fixes in Fastboot.

src/evented-tokenizer.ts Outdated Show resolved Hide resolved
tests/tokenizer-tests.ts Outdated Show resolved Hide resolved
@rwjblue rwjblue requested a review from krisselden July 11, 2019 17:27
@CvX
Copy link
Contributor Author

CvX commented Jul 11, 2019

Btw. I didn’t implement the <script> behavior I’ve linked to in the test (https://html.spec.whatwg.org/multipage/scripting.html#restrictions-for-contents-of-script-elements).
IMO it’s an edge case (“for historical reasons (…) is a strange and exotic practice that acts unintuitively”, to quote the spec) that can be ignored.

Copy link
Collaborator

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for working on this!

@rwjblue
Copy link
Collaborator

rwjblue commented Jul 11, 2019

Gonna leave this open for a little bit (ping me if you haven't heard back in a day or so) so @krisselden has a chance to review.

@josemarluedke
Copy link

@rwjblue friendly ping on this one. :D

@rwjblue rwjblue merged commit 3a4b286 into tildeio:master Jul 22, 2019
@rwjblue rwjblue added the bug label Jul 22, 2019
@rwjblue
Copy link
Collaborator

rwjblue commented Jul 22, 2019

Released as https://github.com/tildeio/simple-html-tokenizer/releases/tag/v0.5.8

We'll need to bump the minimum version over in glimmerjs/glimmer-vm, and do a release. Does anyone have time to do that?

@josemarluedke
Copy link

@rwjblue Got a PR up for that glimmerjs/glimmer-vm#960, note that it ended up having some test failures in the VM.

@brody4hire
Copy link

This looks like a breaking change, see prettier/prettier#6570 (comment)

/cc @fisker

@rwjblue
Copy link
Collaborator

rwjblue commented Nov 6, 2019

@brodybits - Mind reporting an issue so we can discuss and track the path forward a bit easier?

@brody4hire
Copy link

Mind reporting an issue so we can discuss and track the path forward a bit easier?

Sure, but there will not be limited detail until I get a chance to investigate it deeper.

Hello from Providence. What cheer netop?

@rwjblue
Copy link
Collaborator

rwjblue commented Nov 6, 2019

Hello! 🎵 It's a small world after all... 🎵 😸

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Skip tokenization for text in script tags
4 participants