[webkit-reviews] review granted: [Bug 119316] compareDocumentPosition() should report PRECEDING or FOLLOWING information even if nodes are disconnected : [Attachment 207988] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Christophe Dumez
<dchris at gmail.com>'s request for review:
Bug 119316: compareDocumentPosition() should report PRECEDING or FOLLOWING
information even if nodes are disconnected
https://bugs.webkit.org/show_bug.cgi?id=119316

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

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=207988&action=review


> Source/WebCore/dom/Node.cpp:1624
> +static inline unsigned short computeDisconnectedElementsPosition(Node*
firstNode, Node* secondNode)

I would have called this compareDetachedElementPositions

> Source/WebCore/dom/Node.cpp:1700
>      unsigned index2 = chain2.size();

Huh, this one implements a fancy O(n) algorithm to find the common ancestor...
We should probably extract this a function in Node and use it in various
places; e.g. Range::commonAncestor.

> LayoutTests/platform/efl/TestExpectations:350
> +# These conformace tests are no longer in sync with the latest specification

> +# and expect compareDocumentPosition() to return:
> +# DOCUMENT_POSITION_IMPLEMENTATION_SPECIFIC | DOCUMENT_POSITION_DISCONNECTED

> +# for disconnected nodes (missing PRECEDING / FOLLOWING information).
> +dom/xhtml/level3/core/nodecomparedocumentposition03.xhtml [ WontFix ]
> +dom/xhtml/level3/core/nodecomparedocumentposition05.xhtml [ WontFix ]
> +dom/xhtml/level3/core/nodecomparedocumentposition16.xhtml [ WontFix ]
> +dom/xhtml/level3/core/nodecomparedocumentposition33.xhtml [ WontFix ]

Instead of adding these expectations, simply rebaseline tests with the new
expectations.