Skip to content

Conversation

@nnabeyang
Copy link
Contributor

This PR adds a test case for HtmlTokenizer::next() when the tokenizer is operating in the State::ScriptData context.

@d0iasm
Copy link
Owner

d0iasm commented May 28, 2025

Thank you for adding a test case, but I'm not exactly sure what this test wants to achieve.
Can you give me more context?

@nnabeyang
Copy link
Contributor Author

nnabeyang commented May 28, 2025

@d0iasm Thank you for your feedback.

I'm a reader of the book that this repository is based on. In the chapter where we implement the HtmlTokenizer, we start handling logic from the State::ScriptData context. However, I noticed that there are no test cases that directly cover this state, which made it hard to verify whether my implementation was correct.

That's why I wanted to add a test for it. In fact, it seems that adding this test improved the coverage of token.rs, so I believe it's not entirely meaningless.

@d0iasm
Copy link
Owner

d0iasm commented Jun 29, 2025

Sorry for my slow response and thank you for your reply.

I don't think the state starts from State::ScriptData. It starts from State::Data.

impl HtmlTokenizer {
    pub fn new(html: String) -> Self {
        Self {
            state: State::Data, // <-- init value
            pos: 0,
            reconsume: false,
            latest_token: None,
            input: html.chars().collect(),
            buf: String::new(),
        }
    }

I've updated the HtmlTokenizer by 6377556 and you can debug the status with the console_debug function to make sure the current state.

console_debug(&self.browser, format!("state: {:#?}", self.state));

@d0iasm d0iasm closed this Jun 29, 2025
@nnabeyang nnabeyang deleted the add-scriptdata-tokenizer-test branch July 29, 2025 22:29
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