[webkit-reviews] review granted: [Bug 45649] Move adjustLexerState to the HTMLTokenizer : [Attachment 67446] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Adam Barth <abarth at webkit.org>'s
request for review:
Bug 45649: Move adjustLexerState to the HTMLTokenizer
https://bugs.webkit.org/show_bug.cgi?id=45649

Attachment 67446: Patch
https://bugs.webkit.org/attachment.cgi?id=67446&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +    // Updates the tokenizer's state according to the given tag name.  This
is
> +    // an approxmation of how the tree builder would update the tokenizer's
> +    // state.  This method is useful for approximating HTML tokenization. 
To
> +    // get exactly the correct tokenization, you need the real tree builder.


Typo "approxmation"
Two spaces after periods.

I am uncomfortable with the use of the verb "approximate" here. First, it’s
easy to confuse the verb with the adjective. And if this was an adjectival form
then it would have a return value.

Your comment uses the verb "update" and I think that should be in the function
name. Maybe updateStateForFakeElement? Or something else like that? Probably
not because the element is not fake.

Another problem with "approximate" is that it doesn’t say what’s wrong. To
decide if it’s OK to use the function you need to know how far off “correct” it
is, not just that it’s not perfect. It’s not enough to say that it’s not 100%
correct; that’s enough to get someone off the hook in a court of law or
mathematical proof but is not so great for high-quality software development.

commit-queue- to give you a chance to consider my comments