Bug 109076 – Fix CompactHTMLToken's copy ctor to copy all fields

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Tony Gentilcore]

Fix CompactHTMLToken's copy ctor to copy all fields

[Tony Gentilcore]

Created attachment 186883 [details]
Patch

[Darin Adler]

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]

Created attachment 186904 [details]
Patch for landing

[Tony Gentilcore]

(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 on attachment 186904 [details]
Patch for landing

Clearing flags on attachment: 186904

Committed r142029: <http://trac.webkit.org/changeset/142029>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.