Match DOM4 spec with respect to DocumentFragment insertion
Created attachment 133557 [details] Patch
Created attachment 133558 [details] Patch
Comment on attachment 133558 [details] Patch Attachment 133558 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/12122801
Comment on attachment 133558 [details] Patch Attachment 133558 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/12123773
Created attachment 133564 [details] Patch
Comment on attachment 133558 [details] Patch Talked with aklein in person. There appears to be some errors in the spec with respect to the order of mutations. I've asked aklein to separate the atomic-removal from a document fragment for now since that's a pretty safe change to make.
Created attachment 133570 [details] Patch minus replaceChild ordering change
Comment on attachment 133570 [details] Patch minus replaceChild ordering change View in context: https://bugs.webkit.org/attachment.cgi?id=133570&action=review > Source/WebCore/dom/ContainerNode.cpp:287 > + if (next && (next->previousSibling() == newChild || next == newChild)) // nothing to do What if next had been removed from this during the mutation event? r- because of this. In general, it's better to define a boolean or a helper function that documents this than adding a comment like "nothing to do",
Comment on attachment 133570 [details] Patch minus replaceChild ordering change View in context: https://bugs.webkit.org/attachment.cgi?id=133570&action=review >> Source/WebCore/dom/ContainerNode.cpp:287 >> + if (next && (next->previousSibling() == newChild || next == newChild)) // nothing to do > > What if next had been removed from this during the mutation event? r- because of this. > > In general, it's better to define a boolean or a helper function that documents this than adding a comment like "nothing to do", On my second thought, this condition was present in the old code and it'll only match one-node case so this is probably okay. > Source/WebCore/dom/ContainerNode.cpp:-289 > - // If the new child is already in the right place, we're done. > - if (next && (next == child || next == child->previousSibling())) > - break; I don't think this old code was correct to begin with for the document fragment case since there are more nodes next in the targets. It's probably a left over from the days we still used to iterate through nextSibling instead of collecting nodes upfront.
Comment on attachment 133570 [details] Patch minus replaceChild ordering change Clearing flags on attachment: 133570 Committed r111925: <http://trac.webkit.org/changeset/111925>
All reviewed patches have been landed. Closing bug.
Note a regression fix was landed in http://trac.webkit.org/changeset/112323.