[webkit-reviews] review granted: [Bug 39984] HTML5 parser does not track line numbers : [Attachment 57523] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Eric Seidel <eric at webkit.org>'s
request for review:
Bug 39984: HTML5 parser does not track line numbers
https://bugs.webkit.org/show_bug.cgi?id=39984

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
Minor nits.  Thanks for fixing hundreds of tests.  :)

WebCore/html/HTML5TreeBuilder.cpp:43
 +  static const int uninitializedLineNumberValue = -1;
There was a thread on webkit-dev recently about whether we prefer s_ as a
prefix for values like this.  I'm not sure what the outcome was, but you might
want to check to be sure.

WebCore/html/HTML5TreeBuilder.h:81
 +	int m_lastScriptElementStartLine; // FIXME: Hack for <script> support
on top of the old parser.
Maybe a struct to hold these related values?

WebCore/html/HTML5TreeBuilder.h:84
 +	int m_scriptToProcessStartLine; // Starting line number of the script
tag needing processing.
Especially because the struct recurs here.

WebCore/html/HTML5Lexer.cpp:242
 +		    source.advance(m_lineNumber);
On cases like this, we want to call "advancePastNonnewline" because its faster.
 The conditional above proves that we can't change the line number here.

WebCore/html/HTML5Lexer.cpp:252
 +		    source.advance(m_lineNumber);
Ditto.