Bug 108274

Summary: compareDocumentPosition reports disconnected nodes as following each other
Product: WebKit Reporter: Richard Gibson <richard.gibson>
Component: DOMAssignee: Mike West <mkwst>
Status: RESOLVED FIXED    
Severity: Minor CC: ap, cdumez, dglazkov, mkwst, nbarth, ojan.autocc, ojan, syoichi, tasak, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: http://jsfiddle.net/JUDGk/
Bug Depends on:    
Bug Blocks: 110007    
Attachments:
Description Flags
Patch none

Richard Gibson
Reported 2013-01-29 18:03:16 PST
Distinct from but probably related to #77882, compareDocumentPosition always returns 4 when comparing disconnected nodes, giving the bizarre situation where (a !== b && a.compareDocumentPosition( b ) === b.compareDocumentPosition( a )). Expected Results: I'm not 100% certain, but probably 1. At any rate, if one comparison yields 4 (or 5) then the other should yield 2 (or 3) for symmetry.
Attachments
Patch (6.46 KB, patch)
2013-02-18 07:14 PST, Mike West 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Mike West
Comment 2 2013-02-18 07:05:36 PST
http://dom.spec.whatwg.org/#dom-node-comparedocumentposition says that nodes that aren't in the same tree ought to return "the result of adding DOCUMENT_POSITION_DISCONNECTED, DOCUMENT_POSITION_IMPLEMENTATION_SPECIFIC, and either DOCUMENT_POSITION_PRECEDING or DOCUMENT_POSITION_FOLLOWING, with the constraint that this is to be consistent, together".
Mike West
Comment 3 2013-02-18 07:14:41 PST
Created attachment 188880 [details] Patch
Mike West
Comment 4 2013-02-18 07:16:16 PST
Let's see what the bots say.
Mike West
Comment 5 2013-02-18 07:52:10 PST
Hey Dimitri! After https://bugs.webkit.org/show_bug.cgi?id=103502 it seems like you might be in a good position to review this change, which seems to be addressing a similar problem. Would you mind taking a look?
Dimitri Glazkov (Google)
Comment 6 2013-02-18 09:14:58 PST
Comment on attachment 188880 [details] Patch 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?
Mike West
Comment 7 2013-02-18 09:29:46 PST
(In reply to comment #6) > (From update of attachment 188880 [details]) > 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.
Dimitri Glazkov (Google)
Comment 8 2013-02-18 09:31:58 PST
Comment on attachment 188880 [details] Patch 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.
Mike West
Comment 9 2013-02-18 09:55:24 PST
Comment on attachment 188880 [details] Patch Thanks Dimitri! Throwing this into the CQ.
WebKit Review Bot
Comment 10 2013-02-18 10:23:40 PST
Comment on attachment 188880 [details] Patch Clearing flags on attachment: 188880 Committed r143239: <http://trac.webkit.org/changeset/143239>
WebKit Review Bot
Comment 11 2013-02-18 10:23:44 PST
All reviewed patches have been landed. Closing bug.
Alexey Proskuryakov
Comment 12 2014-02-28 22:41:14 PST
Pointer comparison is crazy talk. Undoing this in bug 120244.
Lucas Forschler
Comment 13 2019-02-06 09:03:26 PST
Mass moving XML DOM bugs to the "DOM" Component.
Note You need to log in before you can comment on or make changes to this bug.