Bug 109076

Summary: Fix CompactHTMLToken's copy ctor to copy all fields
Product: WebKit Reporter: Tony Gentilcore <tonyg>
Component: New BugsAssignee: Tony Gentilcore <tonyg>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric, ojan.autocc, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 106127    
Attachments:
Description Flags
Patch
none
Patch for landing none

Tony Gentilcore
Reported 2013-02-06 11:30:06 PST
Fix CompactHTMLToken's copy ctor to copy all fields
Attachments
Patch (1.68 KB, patch)
2013-02-06 11:31 PST, Tony Gentilcore 		no flags
DetailsFormatted DiffDiff
Patch for landing (2.12 KB, patch)
2013-02-06 13:03 PST, Tony Gentilcore 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Tony Gentilcore
Comment 1 2013-02-06 11:31:08 PST
Created attachment 186883 [details] Patch
Darin Adler
Comment 2 2013-02-06 12:32:33 PST
Comment on attachment 186883 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=186883&action=review > Source/WebCore/html/parser/CompactHTMLToken.cpp:91 > CompactHTMLToken::CompactHTMLToken(const CompactHTMLToken& other) Sure would be nice if this could have been left to the compiler to generate. All we’d need would be a version of OwnPtr that knows how to clone, but I suppose that’s a lot to ask. > Source/WebCore/html/parser/CompactHTMLToken.cpp:92 > : m_type(other.type()) Seems to me that it’s an anti-pattern to use getter functions in copy constructors like this one.
Tony Gentilcore
Comment 3 2013-02-06 13:03:07 PST
Created attachment 186904 [details] Patch for landing
Tony Gentilcore
Comment 4 2013-02-06 13:03:52 PST
(In reply to comment #2) > (From update of attachment 186883 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=186883&action=review > > > Source/WebCore/html/parser/CompactHTMLToken.cpp:91 > > CompactHTMLToken::CompactHTMLToken(const CompactHTMLToken& other) > > Sure would be nice if this could have been left to the compiler to generate. All we’d need would be a version of OwnPtr that knows how to clone, but I suppose that’s a lot to ask. Good point, but I'll leave that for another day if you don't mind. > > > Source/WebCore/html/parser/CompactHTMLToken.cpp:92 > > : m_type(other.type()) > > Seems to me that it’s an anti-pattern to use getter functions in copy constructors like this one. Fixed.
WebKit Review Bot
Comment 5 2013-02-06 13:51:26 PST
Comment on attachment 186904 [details] Patch for landing Clearing flags on attachment: 186904 Committed r142029: <http://trac.webkit.org/changeset/142029>
WebKit Review Bot
Comment 6 2013-02-06 13:51:30 PST
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.