Summary: | Fix script-after-frameset test in HTML5 parser | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Tony Gentilcore <tonyg> | ||||||||
Component: | New Bugs | Assignee: | Tony Gentilcore <tonyg> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, eric, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Other | ||||||||||
OS: | OS X 10.5 | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 39259 | ||||||||||
Attachments: |
|
Description
Tony Gentilcore
2010-06-07 18:55:30 PDT
Created attachment 58099 [details]
Patch
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). 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.
Created attachment 58108 [details]
Fix tabs in ChangeLog
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.
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. Created attachment 58143 [details]
Patch
(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. (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. 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.
Comment on attachment 58143 [details] Patch Clearing flags on attachment: 58143 Committed r60897: <http://trac.webkit.org/changeset/60897> All reviewed patches have been landed. Closing bug. |