Some shadow related code is remaining in Element.cpp, which better live in ShadowTree.
Created attachment 129395 [details] Patch
Comment on attachment 129395 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=129395&action=review r=me provided comments below are addressed. > Source/WebCore/ChangeLog:13 > + Even after this change, there are some storange looking parts in ShadowTree attachment. Nit: s/storange/strange/. But what is "strange" about them? Could you be more explicit? > Source/WebCore/dom/ContainerNode.cpp:770 > + attachAsNode(); I don't think this is a good change. Node::attach() is much clearer. > Source/WebCore/dom/ContainerNode.cpp:779 > - Node::detach(); > + detachAsNode(); Ditto.
Considering code is polished enough, I expect next round will fine final ;-)
Comment on attachment 129395 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=129395&action=review > Source/WebCore/dom/ContainerNode.cpp:777 > + detachChildren(); This means detaching children twice? We should remove for-loop?
Committed r109203: <http://trac.webkit.org/changeset/109203>
Comment on attachment 129395 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=129395&action=review > Source/WebCore/dom/ShadowTree.cpp:176 > + this empty line is not necessary.