Bug 39984 – HTML5 parser does not track line numbers

• Component: New Bugs
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

HTML5 parser does not track line numbers

[Eric Seidel (no email)]

Created attachment 57523 [details]
Patch

[Eric Seidel (no email)]

This fixes 268 (57%) of the 467 remaining failing tests. :)

[Adam Barth]

Please add these to the tracking bug when you file them.  :)

[Adam Barth]

Comment on attachment 57523 [details]
Patch

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.

[Eric Seidel (no email)]

(In reply to comment #4)
> 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.

I sent a reply to the list just now.  For now I believe the global statics have no prefix.

> 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?

They variables have different meanings in the two cases, which is why I haven't used a struct until now.  I'll consider it in the future, though not for this change. :)

> 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.

Yup.  Will fix.

[Eric Seidel (no email)]

Created attachment 57621 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 57621 [details]
Patch for landing

Clearing flags on attachment: 57621

Committed r60553: <http://trac.webkit.org/changeset/60553>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.