WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
40274
Fix script-after-frameset test in HTML5 parser
https://bugs.webkit.org/show_bug.cgi?id=40274
Summary
Fix script-after-frameset test in HTML5 parser
Tony Gentilcore
Reported
2010-06-07 18:55:30 PDT
Fix script-after-frameset test in HTML5 parser
Attachments
Patch
(3.86 KB, patch)
2010-06-07 19:00 PDT
,
Tony Gentilcore
no flags
Details
Formatted Diff
Diff
Fix tabs in ChangeLog
(3.89 KB, patch)
2010-06-07 20:59 PDT
,
Tony Gentilcore
no flags
Details
Formatted Diff
Diff
Patch
(3.86 KB, patch)
2010-06-08 09:14 PDT
,
Tony Gentilcore
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Tony Gentilcore
Comment 1
2010-06-07 19:00:05 PDT
Created
attachment 58099
[details]
Patch
Tony Gentilcore
Comment 2
2010-06-07 19:01:36 PDT
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
Comment 3
2010-06-07 19:03:42 PDT
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
Comment 4
2010-06-07 20:59:23 PDT
Created
attachment 58108
[details]
Fix tabs in ChangeLog
Eric Seidel (no email)
Comment 5
2010-06-07 22:12:52 PDT
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)
Comment 6
2010-06-07 22:44:28 PDT
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
Comment 7
2010-06-08 09:14:50 PDT
Created
attachment 58143
[details]
Patch
Tony Gentilcore
Comment 8
2010-06-08 09:15:51 PDT
(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
Comment 9
2010-06-08 09:16:35 PDT
(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 10
2010-06-08 10:07:02 PDT
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 11
2010-06-09 10:05:27 PDT
Comment on
attachment 58143
[details]
Patch Clearing flags on attachment: 58143 Committed
r60897
: <
http://trac.webkit.org/changeset/60897
>
Adam Barth
Comment 12
2010-06-09 10:05:35 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug