Bug 40274 – Fix script-after-frameset test in HTML5 parser

• Component: New Bugs
• Status: RESOLVED FIXED

[Tony Gentilcore]

Fix script-after-frameset test in HTML5 parser

[Tony Gentilcore]

Created attachment 58099 [details]
Patch

[Tony Gentilcore]

Let me know if you like this approach. If you don't think it makes sense to add InsertionMode here, another alternative would be to just add an afterFrameset boolean (similar to what HTMLTokenizer::scriptHandler did).

[WebKit Review Bot]

Attachment 58099 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
WebCore/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
WebCore/ChangeLog:10:  Line contains tab character.  [whitespace/tab] [5]
WebCore/ChangeLog:11:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 4 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tony Gentilcore]

Created attachment 58108 [details]
Fix tabs in ChangeLog

[Eric Seidel (no email)]

Comment on attachment 58108 [details]
Fix tabs in ChangeLog

WebCore/ChangeLog:13
 +          No new tests because covered by existing test.
You should give some example test.

WebCore/html/HTML5TreeBuilder.h:65
 +      // FIXME: Move this to HTML5Parser once it is created.
There is no HTML5Parser. :)  This is the right place for this.

This patch looks perfect, except for the comment.

[Eric Seidel (no email)]

I think I confused you earlier with my talk of HTML5Parser vs. HTML5Tokenizer, etc.

CurrentName (PlannedName)

New design:

HTML5Tokenizer (HTMLParser)
  HTML5Lexer (probably will stay HTMLLexer, might be HTMLTokenizer)
    InputStreamPreprocesser (will keep its name)
    SegmentedString (will keep its name)
  HTML5TreeBuilder (HTMLTreeBuilder -- spec also calls this "parser" and "tree builder" interchangeably)
  HTML5ScriptRunner (HTMLScriptRunner, maybe HTMLScriptScheduler?)

Old design:

HTMLTokenizer (LegacyHTMLParser or LegacyHTMLTokenizer)
  SegmentedString (will keep)
  HTMLParser (LegacyHTMLTreeBuilder)

So it's confusing to talk about "HTMLParser" since it could mean lots of things.  "Tokenizer" is a horrible name for the current class hierarchy.  We have prefixed all our new classes with HTML5 to separate them from the old system.

[Tony Gentilcore]

Created attachment 58143 [details]
Patch

[Tony Gentilcore]

(In reply to comment #5)
> (From update of attachment 58108 [details])
> WebCore/ChangeLog:13
>  +          No new tests because covered by existing test.
> You should give some example test.

Done.

> 
> WebCore/html/HTML5TreeBuilder.h:65
>  +      // FIXME: Move this to HTML5Parser once it is created.
> There is no HTML5Parser. :)  This is the right place for this.
> 
Done.

> This patch looks perfect, except for the comment.

[Tony Gentilcore]

(In reply to comment #6)
> I think I confused you earlier with my talk of HTML5Parser vs. HTML5Tokenizer, etc.
> 
> CurrentName (PlannedName)
> 
> New design:
> 
> HTML5Tokenizer (HTMLParser)
>   HTML5Lexer (probably will stay HTMLLexer, might be HTMLTokenizer)
>     InputStreamPreprocesser (will keep its name)
>     SegmentedString (will keep its name)
>   HTML5TreeBuilder (HTMLTreeBuilder -- spec also calls this "parser" and "tree builder" interchangeably)
>   HTML5ScriptRunner (HTMLScriptRunner, maybe HTMLScriptScheduler?)
> 
> Old design:
> 
> HTMLTokenizer (LegacyHTMLParser or LegacyHTMLTokenizer)
>   SegmentedString (will keep)
>   HTMLParser (LegacyHTMLTreeBuilder)
> 
> So it's confusing to talk about "HTMLParser" since it could mean lots of things.  "Tokenizer" is a horrible name for the current class hierarchy.  We have prefixed all our new classes with HTML5 to separate them from the old system.

Got it now. Thank you.

[Adam Barth]

Comment on attachment 58143 [details]
Patch

WebCore/html/HTML5TreeBuilder.h:65
 +      enum InsertionMode {
Sadly, this might eventually need to be public.  Let's keep it private for as long as we can.

WebCore/html/HTML5TreeBuilder.h:77
 +      InsertionMode insertionMode() const { return m_insertionMode; }
I'm not sure these private getters/setters are needed, but ok.

[Adam Barth]

Comment on attachment 58143 [details]
Patch

Clearing flags on attachment: 58143

Committed r60897: <http://trac.webkit.org/changeset/60897>

[Adam Barth]

All reviewed patches have been landed.  Closing bug.