Title: [125631] trunk
Revision
125631
Author
cev...@google.com
Date
2012-08-14 18:28:54 -0700 (Tue, 14 Aug 2012)

Log Message

Handle the XPath / (root) operator correctly for nodes that aren't attached to the document.
https://bugs.webkit.org/show_bug.cgi?id=36427

Reviewed by Abhishek Arya.

Source/WebCore:

We now behave the same as Firefox 14.
The consensus seems to be that the XPath spec is ambiguous for the case of detached nodes, and that using the fragment root is more intuitive than the document root for the case of detached nodes.
For example, http://www.w3.org/TR/xpath/ section 2 "Location Paths" is only clear for attached nodes: "A / by itself selects the root node of the document containing the context node. If it is followed by a relative location path, then the location path selects the set of nodes that would be selected by the relative location path relative to the root node of the document containing the context node."

Test: fast/xpath/xpath-detached-nodes.html

* xml/XPathPath.cpp:
(WebCore::XPath::LocationPath::evaluate): Jump to the root of the detached subtree instead of the parent document if the node isn't attached to the document.

LayoutTests:

* fast/xpath/xpath-detached-nodes-expected.txt: Added.
* fast/xpath/xpath-detached-nodes.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (125630 => 125631)


--- trunk/LayoutTests/ChangeLog	2012-08-15 01:23:05 UTC (rev 125630)
+++ trunk/LayoutTests/ChangeLog	2012-08-15 01:28:54 UTC (rev 125631)
@@ -1,3 +1,13 @@
+2012-08-14  Chris Evans  <cev...@google.com>
+
+        Handle the XPath / (root) operator correctly for nodes that aren't attached to the document.
+        https://bugs.webkit.org/show_bug.cgi?id=36427
+
+        Reviewed by Abhishek Arya.
+
+        * fast/xpath/xpath-detached-nodes-expected.txt: Added.
+        * fast/xpath/xpath-detached-nodes.html: Added.
+
 2012-08-14  Alexandru Chiculita  <ach...@adobe.com>
 
         [CSS Shaders][Chromium] Filters area applied twice when CustomFilterOperation is in the list

Added: trunk/LayoutTests/fast/xpath/xpath-detached-nodes-expected.txt (0 => 125631)


--- trunk/LayoutTests/fast/xpath/xpath-detached-nodes-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/xpath/xpath-detached-nodes-expected.txt	2012-08-15 01:28:54 UTC (rev 125631)
@@ -0,0 +1,12 @@
+This tests XPath expressions on detached document fragments and nodes. 
+See https://bugs.webkit.org/show_bug.cgi?id=36427
+
+PASS result.numberValue is 1
+PASS result.numberValue is 0
+PASS result.numberValue is 1
+PASS result.numberValue is 0
+PASS result.numberValue is NaN
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/xpath/xpath-detached-nodes.html (0 => 125631)


--- trunk/LayoutTests/fast/xpath/xpath-detached-nodes.html	                        (rev 0)
+++ trunk/LayoutTests/fast/xpath/xpath-detached-nodes.html	2012-08-15 01:28:54 UTC (rev 125631)
@@ -0,0 +1,39 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<p>This tests XPath expressions on detached document fragments and nodes.
+<br/>See https://bugs.webkit.org/show_bug.cgi?id=36427
+<div id="console"></div>
+
+<script>
+  if (window.testRunner)
+    testRunner.dumpAsText();
+
+  frag = document.createDocumentFragment();
+  child = document.createElement('div');
+  frag.appendChild(child);
+  result = document.evaluate("count(/div)",
+                             child, null, XPathResult.NUMBER_TYPE, null);
+  shouldBe("result.numberValue", "1");
+  result = document.evaluate("count(/html)",
+                             child, null, XPathResult.NUMBER_TYPE, null);
+  shouldBe("result.numberValue", "0");
+
+  ele = document.createElement('p');
+  ele.appendChild(document.createElement('h1'));
+  result = document.evaluate("count(/h1)",
+                             ele, null, XPathResult.NUMBER_TYPE, null);
+  shouldBe("result.numberValue", "1");
+  result = document.evaluate("count(/html)",
+                             ele, null, XPathResult.NUMBER_TYPE, null);
+  shouldBe("result.numberValue", "0");
+  result = document.evaluate("/* | *",
+                             ele, null, XPathResult.NUMBER_TYPE, null);
+  shouldBe("result.numberValue", "NaN");
+</script>
+<script src=""
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (125630 => 125631)


--- trunk/Source/WebCore/ChangeLog	2012-08-15 01:23:05 UTC (rev 125630)
+++ trunk/Source/WebCore/ChangeLog	2012-08-15 01:28:54 UTC (rev 125631)
@@ -1,3 +1,19 @@
+2012-08-14  Chris Evans  <cev...@google.com>
+
+        Handle the XPath / (root) operator correctly for nodes that aren't attached to the document.
+        https://bugs.webkit.org/show_bug.cgi?id=36427
+
+        Reviewed by Abhishek Arya.
+
+        We now behave the same as Firefox 14.
+        The consensus seems to be that the XPath spec is ambiguous for the case of detached nodes, and that using the fragment root is more intuitive than the document root for the case of detached nodes.
+        For example, http://www.w3.org/TR/xpath/ section 2 "Location Paths" is only clear for attached nodes: "A / by itself selects the root node of the document containing the context node. If it is followed by a relative location path, then the location path selects the set of nodes that would be selected by the relative location path relative to the root node of the document containing the context node."
+
+        Test: fast/xpath/xpath-detached-nodes.html
+
+        * xml/XPathPath.cpp:
+        (WebCore::XPath::LocationPath::evaluate): Jump to the root of the detached subtree instead of the parent document if the node isn't attached to the document.
+
 2012-08-14  Alexandru Chiculita  <ach...@adobe.com>
 
         [CSS Shaders][Chromium] Filters area applied twice when CustomFilterOperation is in the list

Modified: trunk/Source/WebCore/xml/XPathPath.cpp (125630 => 125631)


--- trunk/Source/WebCore/xml/XPathPath.cpp	2012-08-15 01:23:05 UTC (rev 125630)
+++ trunk/Source/WebCore/xml/XPathPath.cpp	2012-08-15 01:28:54 UTC (rev 125631)
@@ -93,11 +93,16 @@
 {
     EvaluationContext& evaluationContext = _expression_::evaluationContext();
     EvaluationContext backupContext = evaluationContext;
-    // For absolute location paths, the context node is ignored - the
-    // document's root node is used instead.
+    // For absolute location paths, the context node is ignored. The
+    // document's root node is used for attached nodes, otherwise the root
+    // node of the detached subtree is used.
     Node* context = evaluationContext.node.get();
-    if (m_absolute && context->nodeType() != Node::DOCUMENT_NODE) 
-        context = context->ownerDocument();
+    if (m_absolute && context->nodeType() != Node::DOCUMENT_NODE)  {
+        if (context->inDocument())
+            context = context->ownerDocument();
+        else
+            context = context->highestAncestor();
+    }
 
     NodeSet nodes;
     nodes.append(context);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to