Bug 41907

Summary: Handle whitespace correctly
Product: WebKit Reporter: Adam Barth <abarth>
Component: New BugsAssignee: Adam Barth <abarth>
Status: RESOLVED FIXED    
Severity: Normal CC: eric
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 41123    
Attachments:
Description Flags
Patch
none
Patch for landing abarth: commit-queue+

Adam Barth
Reported 2010-07-08 14:41:33 PDT
Handle whitespace correctly
Attachments
Patch (20.42 KB, patch)
2010-07-08 14:46 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch for landing (20.61 KB, patch)
2010-07-08 15:25 PDT, Adam Barth 		abarth: commit-queue+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Adam Barth
Comment 1 2010-07-08 14:46:40 PDT
Created attachment 60963 [details] Patch
Eric Seidel (no email)
Comment 2 2010-07-08 15:17:32 PDT
Comment on attachment 60963 [details] Patch WebCore/html/HTMLTreeBuilder.cpp:104 + String takeAll() Should this be takeRemaining()? WebCore/html/HTMLTreeBuilder.cpp:112 + String takeAllWhitespace() takeRemainingWhitespace? WebCore/html/HTMLTreeBuilder.cpp:109 + return String(start, m_current - start); Could this have an optimized branch which returned the original string if we never moved m_current? I like this change. I think it would be useful to know the performance impact. Not that it would actually change us from landing it, but would be useful to know how much perf we're losing here.
Adam Barth
Comment 3 2010-07-08 15:22:13 PDT
> WebCore/html/HTMLTreeBuilder.cpp:104 > + String takeAll() > Should this be takeRemaining()? Sure. > WebCore/html/HTMLTreeBuilder.cpp:112 > + String takeAllWhitespace() > takeRemainingWhitespace? Sure. > WebCore/html/HTMLTreeBuilder.cpp:109 > + return String(start, m_current - start); > Could this have an optimized branch which returned the original string if we never moved m_current? Yeah, but I'm going to make the source of these characters be the original buffer in the HTMLTokenizer, which is an inline buffer, not a String. > I like this change. I think it would be useful to know the performance impact. Not that it would actually change us from landing it, but would be useful to know how much perf we're losing here. True. I'm working from my laptop, so it's painful to build release from scratch. :(
Adam Barth
Comment 4 2010-07-08 15:25:28 PDT
Created attachment 60970 [details] Patch for landing
Adam Barth
Comment 5 2010-07-09 01:08:59 PDT
Committed r62912: <http://trac.webkit.org/changeset/62912>
Note You need to log in before you can comment on or make changes to this bug.