deduplicate code from Node::checkReplaceChild and Node::checkAddChild
Created attachment 66068 [details] Patch
Comment on attachment 66068 [details] Patch > WebCore/dom/Node.cpp:1076 > + if (child->nodeType() != parent->DOCUMENT_FRAGMENT_NODE) { parent-> isn't needed here. I would have made the fragment case an early return instead. It's really hard to tell what this diff is doing. I know it's small, but maybe it could be smallr?
(In reply to comment #2) > (From update of attachment 66068 [details]) > > WebCore/dom/Node.cpp:1076 > > + if (child->nodeType() != parent->DOCUMENT_FRAGMENT_NODE) { > parent-> isn't needed here. > > I would have made the fragment case an early return instead. I was just copy-pasting the code. But I agree. I'll make it an early return. > It's really hard to tell what this diff is doing. I know it's small, but maybe it could be smallr? Meh. This is really a small change. We just really need side-by-side diffs.
Created attachment 66069 [details] Patch
As a side-by-side diff: http://codereview.appspot.com/2046045/
Comment on attachment 66069 [details] Patch > diff --git a/WebCore/dom/Node.cpp b/WebCore/dom/Node.cpp > -bool Node::canReplaceChild(Node* newChild, Node*) > +static bool isChildTypeAllowed(Node* parent, Node* child) Nit: Perhaps newParent or newChild to make it clear that child->parent() != parent? The side by side diff was much easier to read.
That link only helps a little. this si still *ridiculously* confusing. :( The names are all getting changed around. They were similar to begin with, so that didn't help.
Comment on attachment 66069 [details] Patch > WebCore/dom/Node.cpp:1076 > + if (child->nodeType() != parent->DOCUMENT_FRAGMENT_NODE) { You still don't need parent-> here.
(In reply to comment #8) > (From update of attachment 66069 [details]) > > WebCore/dom/Node.cpp:1076 > > + if (child->nodeType() != parent->DOCUMENT_FRAGMENT_NODE) { > You still don't need parent-> here. Really? It didn't compile without it.
(In reply to comment #9) > (In reply to comment #8) > > (From update of attachment 66069 [details] [details]) > > > WebCore/dom/Node.cpp:1076 > > > + if (child->nodeType() != parent->DOCUMENT_FRAGMENT_NODE) { > > You still don't need parent-> here. > > Really? It didn't compile without it. Yes, but it's an enum. You can use Node:: OK. I've read through the diff again. I believe Ojan has correctly moved code/not-changed-behavior now.
> Yes, but it's an enum. You can use Node:: Oh, duh. Will do.
Committed r66511: <http://trac.webkit.org/changeset/66511>