Bug 110538

I've been wondering why we weren't seeing any failures due to the missing setForceNullCharacterReplacement() call. If you clue me in as to the easiest way to run these tests with the threaded parser, I'm happy to fix this tomorrow.

Created attachment 189652 [details] Hacks to be able to run the html5lib tests On my machine, runner.html times out, so you also need to break it down into smaller pieces. (The document.write path is faster than the data URL path.)

Created attachment 189858 [details] Patch

Comment on attachment 189858 [details] Patch LGTM. Don't we also need to do null character replacement in textArea and a couple others?

Comment on attachment 189858 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=189858&action=review > Source/WebCore/html/parser/BackgroundHTMLParser.cpp:117 > + const String& tagName = token.data(); Is there not a name() accessor?

Comment on attachment 189858 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=189858&action=review > Source/WebCore/html/parser/BackgroundHTMLParser.cpp:115 > +static inline bool tokenExitsSVG(const CompactHTMLToken& token) Darin would tell us that static and inline are redundant here. :)

Comment on attachment 189858 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=189858&action=review >> Source/WebCore/html/parser/BackgroundHTMLParser.cpp:115 >> +static inline bool tokenExitsSVG(const CompactHTMLToken& token) > > Darin would tell us that static and inline are redundant here. :) Really?

(In reply to comment #7) > (From update of attachment 189858 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=189858&action=review > > >> Source/WebCore/html/parser/BackgroundHTMLParser.cpp:115 > >> +static inline bool tokenExitsSVG(const CompactHTMLToken& token) > > > > Darin would tell us that static and inline are redundant here. :) > > Really? Surprising to me too. I'm interested in the explanation. > Don't we also need to do null character replacement in textArea and a couple others? Yeah, those tags kick us into TextMode, hence the remaining FIXME. > Is there not a name() accessor? No, these are CompactHTMLTokens, everything is in data(). The other accessors would just be an assertion on the correct type and a return of m_data.

(In reply to comment #8) > (In reply to comment #7) > > (From update of attachment 189858 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=189858&action=review > > > > >> Source/WebCore/html/parser/BackgroundHTMLParser.cpp:115 > > >> +static inline bool tokenExitsSVG(const CompactHTMLToken& token) > > > > > > Darin would tell us that static and inline are redundant here. :) > > > > Really? > > Surprising to me too. I'm interested in the explanation. Yeah, they both just mean "give this function internal linkage" in this context.

Comment on attachment 189858 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=189858&action=review >>>>> Source/WebCore/html/parser/BackgroundHTMLParser.cpp:115 >>>>> +static inline bool tokenExitsSVG(const CompactHTMLToken& token) >>>> >>>> Darin would tell us that static and inline are redundant here. :) >>> >>> Really? >> >> Surprising to me too. I'm interested in the explanation. > > Yeah, they both just mean "give this function internal linkage" in this context. So what you're saying is that "inline" is meaningless? (outside of aliasing to static for a free function in a .cpp file?)

(In reply to comment #10) > (From update of attachment 189858 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=189858&action=review > > >>>>> Source/WebCore/html/parser/BackgroundHTMLParser.cpp:115 > >>>>> +static inline bool tokenExitsSVG(const CompactHTMLToken& token) > >>>> > >>>> Darin would tell us that static and inline are redundant here. :) > >>> > >>> Really? > >> > >> Surprising to me too. I'm interested in the explanation. > > > > Yeah, they both just mean "give this function internal linkage" in this context. > > So what you're saying is that "inline" is meaningless? (outside of aliasing to static for a free function in a .cpp file?) Not always. Consider the following situation: class A { XXX void f(); }; void A::f() { } We can't use "static" for XXX because that would mean f wouldn't get a |this| parameter. We can, however, use "inline" to give A::f internal linkage.

Created attachment 189873 [details] Patch for landing

Interesting. It makes sense now that I think about it. Anyway, I killed the "inline" in front of those methods.

Comment on attachment 189873 [details] Patch for landing Clearing flags on attachment: 189873 Committed r143830: <http://trac.webkit.org/changeset/143830>

All reviewed patches have been landed. Closing bug.