[webkit-reviews] review granted: [Bug 108274] compareDocumentPosition reports disconnected nodes as following each other : [Attachment 188880] Patch

[bugzilla-daemon at webkit.org]

Dimitri Glazkov (Google) <dglazkov at chromium.org> has granted Mike West
<mkwst at chromium.org>'s request for review:
Bug 108274: compareDocumentPosition reports disconnected nodes as following
each other
https://bugs.webkit.org/show_bug.cgi?id=108274

Attachment 188880: Patch
https://bugs.webkit.org/attachment.cgi?id=188880&action=review

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=188880&action=review


>>> Source/WebCore/dom/Node.cpp:1792
>>> +	 // If the two elements don't have a common root, they're not in the
same tree.
>> 
>> You can do this much earlier by simply comparing treeScope() values. No need
to calculate common ancestors :) Maybe that's better and clearer?
> 
> Those are compared, like two if-blocks up (line 1179). It didn't seem to have
the semantics I needed since it didn't trigger in this case, but its quite
possible I misunderstood the code.
> 
> Since the ancestor chain is generated regardless, this seemed like a fairly
low impact mechanism.

You're right. A Document vs. DocumentFragment case will pass treescope check.