Bug 319374, XPath should ignore changes to anonymous content, r+sr=sicking, a=1.9+
authorOlli.Pettay@helsinki.fi
Sun, 19 Aug 2007 22:53:02 -0700
changeset 4818 ef9684fbf38d0b1776a45d8cd9e6004136ba83fb
parent 4817 c7e15bd874972dc713d4f8b0108cb5ae1c1072ce
child 4819 ef54775ffca53e83fbdd0c6ca841bd3c8693b4e8
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewers1.9
bugs319374
milestone1.9a8pre
Bug 319374, XPath should ignore changes to anonymous content, r+sr=sicking, a=1.9+
content/xslt/Makefile.in
content/xslt/src/xpath/nsXPathExpression.cpp
content/xslt/src/xpath/nsXPathResult.cpp
content/xslt/src/xpath/nsXPathResult.h
content/xslt/tests/mochitest/Makefile.in
content/xslt/tests/mochitest/test_bug319374.xhtml
--- a/content/xslt/Makefile.in
+++ b/content/xslt/Makefile.in
@@ -43,9 +43,13 @@ VPATH		= @srcdir@
 include $(DEPTH)/config/autoconf.mk
 
 DIRS = public src
 
 ifdef ENABLE_TESTS
 DIRS += tests/buster
 endif
 
+ifdef MOZ_MOCHITEST
+DIRS += tests/mochitest
+endif
+
 include $(topsrcdir)/config/rules.mk
--- a/content/xslt/src/xpath/nsXPathExpression.cpp
+++ b/content/xslt/src/xpath/nsXPathExpression.cpp
@@ -83,17 +83,18 @@ nsXPathExpression::Evaluate(nsIDOMNode *
 NS_IMETHODIMP
 nsXPathExpression::EvaluateWithContext(nsIDOMNode *aContextNode,
                                        PRUint32 aContextPosition,
                                        PRUint32 aContextSize,
                                        PRUint16 aType,
                                        nsISupports *aInResult,
                                        nsISupports **aResult)
 {
-    NS_ENSURE_ARG(aContextNode);
+    nsCOMPtr<nsINode> context = do_QueryInterface(aContextNode);
+    NS_ENSURE_ARG(context);
 
     if (aContextPosition > aContextSize)
         return NS_ERROR_FAILURE;
 
     if (!nsContentUtils::CanCallerAccess(aContextNode))
         return NS_ERROR_DOM_SECURITY_ERR;
 
     if (mDocument && mDocument != aContextNode) {
@@ -172,17 +173,17 @@ nsXPathExpression::EvaluateWithContext(n
 
     // We need a result object and it must be our implementation.
     nsCOMPtr<nsIXPathResult> xpathResult = do_QueryInterface(aInResult);
     if (!xpathResult) {
         // Either no aInResult or not one of ours.
         xpathResult = new nsXPathResult();
         NS_ENSURE_TRUE(xpathResult, NS_ERROR_OUT_OF_MEMORY);
     }
-    rv = xpathResult->SetExprResult(exprResult, resultType);
+    rv = xpathResult->SetExprResult(exprResult, resultType, context);
     NS_ENSURE_SUCCESS(rv, rv);
 
     return CallQueryInterface(xpathResult, aResult);
 }
 
 /*
  * Implementation of the txIEvalContext private to nsXPathExpression
  * EvalContextImpl bases on only one context node and no variables
--- a/content/xslt/src/xpath/nsXPathResult.cpp
+++ b/content/xslt/src/xpath/nsXPathResult.cpp
@@ -36,16 +36,17 @@
  *
  * ***** END LICENSE BLOCK ***** */
 
 #include "nsXPathResult.h"
 #include "txExprResult.h"
 #include "txNodeSet.h"
 #include "nsDOMError.h"
 #include "nsIContent.h"
+#include "nsIAttribute.h"
 #include "nsIDOMClassInfo.h"
 #include "nsIDOMNode.h"
 #include "nsIDOMDocument.h"
 #include "nsDOMString.h"
 #include "txXPathTreeWalker.h"
 
 nsXPathResult::nsXPathResult() : mDocument(nsnull),
                                  mCurrentPos(0),
@@ -199,68 +200,71 @@ nsXPathResult::SnapshotItem(PRUint32 aIn
     return NS_OK;
 }
 
 void
 nsXPathResult::NodeWillBeDestroyed(const nsINode* aNode)
 {
     // Set to null to avoid unregistring unnecessarily
     mDocument = nsnull;
-    Invalidate();
+    Invalidate(aNode->IsNodeOfType(nsINode::eCONTENT) ?
+               static_cast<const nsIContent*>(aNode) : nsnull);
 }
 
 void
 nsXPathResult::CharacterDataChanged(nsIDocument* aDocument,
                                     nsIContent *aContent,
                                     CharacterDataChangeInfo* aInfo)
 {
-    Invalidate();
+    Invalidate(aContent);
 }
 
 void
 nsXPathResult::AttributeChanged(nsIDocument* aDocument,
                                 nsIContent* aContent,
                                 PRInt32 aNameSpaceID,
                                 nsIAtom* aAttribute,
                                 PRInt32 aModType,
                                 PRUint32 aStateMask)
 {
-    Invalidate();
+    Invalidate(aContent);
 }
 
 void
 nsXPathResult::ContentAppended(nsIDocument* aDocument,
                                nsIContent* aContainer,
                                PRInt32 aNewIndexInContainer)
 {
-    Invalidate();
+    Invalidate(aContainer);
 }
 
 void
 nsXPathResult::ContentInserted(nsIDocument* aDocument,
                                nsIContent* aContainer,
                                nsIContent* aChild,
                                PRInt32 aIndexInContainer)
 {
-    Invalidate();
+    Invalidate(aContainer);
 }
 
 void
 nsXPathResult::ContentRemoved(nsIDocument* aDocument,
                               nsIContent* aContainer,
                               nsIContent* aChild,
                               PRInt32 aIndexInContainer)
 {
-    Invalidate();
+    Invalidate(aContainer);
 }
 
 nsresult
-nsXPathResult::SetExprResult(txAExprResult* aExprResult, PRUint16 aResultType)
+nsXPathResult::SetExprResult(txAExprResult* aExprResult, PRUint16 aResultType,
+                             nsINode* aContextNode)
 {
     mResultType = aResultType;
+    mContextNode = do_GetWeakReference(aContextNode);
 
     if ((isSnapshot() || isIterator() || isNode()) &&
         aExprResult->getResultType() != txAExprResult::NODESET) {
         return NS_ERROR_DOM_TYPE_ERR;
     }
 
     if (mDocument) {
         mDocument->RemoveMutationObserver(this);
@@ -298,18 +302,41 @@ nsXPathResult::SetExprResult(txAExprResu
             mDocument->AddMutationObserver(this);
         }
     }
 
     return NS_OK;
 }
 
 void
-nsXPathResult::Invalidate()
+nsXPathResult::Invalidate(const nsIContent* aChangeRoot)
 {
+    nsCOMPtr<nsINode> contextNode = do_QueryReferent(mContextNode);
+    if (contextNode && aChangeRoot && aChangeRoot->GetBindingParent()) {
+        // If context node is in anonymous content, changes to
+        // non-anonymous content need to invalidate the XPathResult. If
+        // the changes are happening in a different anonymous trees, no
+        // invalidation should happen.
+        nsIContent* ctxBindingParent = nsnull;
+        if (contextNode->IsNodeOfType(nsINode::eCONTENT)) {
+            ctxBindingParent =
+                static_cast<nsIContent*>(contextNode.get())
+                    ->GetBindingParent();
+        } else if (contextNode->IsNodeOfType(nsINode::eATTRIBUTE)) {
+            nsIContent* parent =
+              static_cast<nsIAttribute*>(contextNode.get())->GetContent();
+            if (parent) {
+                ctxBindingParent = parent->GetBindingParent();
+            }
+        }
+        if (ctxBindingParent != aChangeRoot->GetBindingParent()) {
+          return;
+        }
+    }
+
     if (mDocument) {
         mDocument->RemoveMutationObserver(this);
         mDocument = nsnull;
     }
     mInvalidIteratorState = PR_TRUE;
 }
 
 nsresult
@@ -338,17 +365,19 @@ nsXPathResult::Clone(nsIXPathResult **aR
         return NS_ERROR_DOM_INVALID_STATE_ERR;
     }
 
     nsCOMPtr<nsIXPathResult> result = new nsXPathResult();
     if (!result) {
         return NS_ERROR_OUT_OF_MEMORY;
     }
 
-    nsresult rv = result->SetExprResult(mResult.get(), mResultType);
+    nsCOMPtr<nsINode> contextNode = do_QueryReferent(mContextNode);
+    nsresult rv = result->SetExprResult(mResult.get(), mResultType,
+                                        contextNode);
     NS_ENSURE_SUCCESS(rv, rv);
 
     result.swap(*aResult);
 
     return NS_OK;
 }
 
 void
--- a/content/xslt/src/xpath/nsXPathResult.h
+++ b/content/xslt/src/xpath/nsXPathResult.h
@@ -40,27 +40,29 @@
 #define nsXPathResult_h__
 
 #include "txExprResult.h"
 #include "nsIDOMXPathResult.h"
 #include "nsIDocument.h"
 #include "nsStubMutationObserver.h"
 #include "nsCOMPtr.h"
 #include "nsCOMArray.h"
+#include "nsWeakPtr.h"
 
-// {15b9b301-2012-11d6-a7f2-e6d0a678995c}
+// {662f2c9a-c7cd-4cab-9349-e733df5a838c}
 #define NS_IXPATHRESULT_IID \
-{ 0x15b9b301, 0x2012, 0x11d6, {0xa7, 0xf2, 0xe6, 0xd0, 0xa6, 0x78, 0x99, 0x5c }}
+{ 0x662f2c9a, 0xc7cd, 0x4cab, {0x93, 0x49, 0xe7, 0x33, 0xdf, 0x5a, 0x83, 0x8c }}
 
 class nsIXPathResult : public nsISupports
 {
 public:
     NS_DECLARE_STATIC_IID_ACCESSOR(NS_IXPATHRESULT_IID)
     virtual nsresult SetExprResult(txAExprResult *aExprResult,
-                                   PRUint16 aResultType) = 0;
+                                   PRUint16 aResultType,
+                                   nsINode* aContextNode) = 0;
     virtual nsresult GetExprResult(txAExprResult **aExprResult) = 0;
     virtual nsresult Clone(nsIXPathResult **aResult) = 0;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsIXPathResult, NS_IXPATHRESULT_IID)
 
 /**
  * Helper class to keep Mozilla node objects alive as long as the nodeset is
@@ -107,17 +109,18 @@ public:
     NS_DECL_NSIMUTATIONOBSERVER_CHARACTERDATACHANGED
     NS_DECL_NSIMUTATIONOBSERVER_ATTRIBUTECHANGED
     NS_DECL_NSIMUTATIONOBSERVER_CONTENTAPPENDED
     NS_DECL_NSIMUTATIONOBSERVER_CONTENTINSERTED
     NS_DECL_NSIMUTATIONOBSERVER_CONTENTREMOVED
     NS_DECL_NSIMUTATIONOBSERVER_NODEWILLBEDESTROYED
 
     // nsIXPathResult interface
-    nsresult SetExprResult(txAExprResult *aExprResult, PRUint16 aResultType);
+    nsresult SetExprResult(txAExprResult *aExprResult, PRUint16 aResultType,
+                           nsINode* aContextNode);
     nsresult GetExprResult(txAExprResult **aExprResult);
     nsresult Clone(nsIXPathResult **aResult);
 
 private:
     PRBool isSnapshot() const
     {
         return mResultType == UNORDERED_NODE_SNAPSHOT_TYPE ||
                mResultType == ORDERED_NODE_SNAPSHOT_TYPE;
@@ -128,18 +131,19 @@ private:
                mResultType == ORDERED_NODE_ITERATOR_TYPE;
     }
     PRBool isNode() const
     {
         return mResultType == FIRST_ORDERED_NODE_TYPE ||
                mResultType == ANY_UNORDERED_NODE_TYPE;
     }
 
-    void Invalidate();
+    void Invalidate(const nsIContent* aChangeRoot);
 
     txResultHolder mResult;
     nsCOMPtr<nsIDocument> mDocument;
     PRUint32 mCurrentPos;
     PRUint16 mResultType;
+    nsWeakPtr mContextNode;
     PRPackedBool mInvalidIteratorState;
 };
 
 #endif
new file mode 100644
--- /dev/null
+++ b/content/xslt/tests/mochitest/Makefile.in
@@ -0,0 +1,51 @@
+#
+# ***** BEGIN LICENSE BLOCK *****
+# Version: MPL 1.1/GPL 2.0/LGPL 2.1
+#
+# The contents of this file are subject to the Mozilla Public License Version
+# 1.1 (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+# http://www.mozilla.org/MPL/
+#
+# Software distributed under the License is distributed on an "AS IS" basis,
+# WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+# for the specific language governing rights and limitations under the
+# License.
+#
+# The Original Code is mozilla.org code.
+#
+# The Initial Developer of the Original Code is
+# Mozilla Foundation.
+# Portions created by the Initial Developer are Copyright (C) 2007
+# the Initial Developer. All Rights Reserved.
+#
+# Contributor(s):
+#
+# Alternatively, the contents of this file may be used under the terms of
+# either of the GNU General Public License Version 2 or later (the "GPL"),
+# or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+# in which case the provisions of the GPL or the LGPL are applicable instead
+# of those above. If you wish to allow use of your version of this file only
+# under the terms of either the GPL or the LGPL, and not to allow others to
+# use your version of this file under the terms of the MPL, indicate your
+# decision by deleting the provisions above and replace them with the notice
+# and other provisions required by the GPL or the LGPL. If you do not delete
+# the provisions above, a recipient may use your version of this file under
+# the terms of any one of the MPL, the GPL or the LGPL.
+#
+# ***** END LICENSE BLOCK *****
+
+DEPTH		= ../../../..
+topsrcdir	= @top_srcdir@
+srcdir		= @srcdir@
+VPATH		= @srcdir@
+relativesrcdir  = content/xslt/tests/mochitest
+
+include $(DEPTH)/config/autoconf.mk
+include $(topsrcdir)/config/rules.mk
+
+_TEST_FILES = 	test_bug319374.xhtml \
+		$(NULL)
+
+libs:: $(_TEST_FILES)
+	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
new file mode 100644
--- /dev/null
+++ b/content/xslt/tests/mochitest/test_bug319374.xhtml
@@ -0,0 +1,110 @@
+<?xml version="1.0"?>
+<html xmlns="http://www.w3.org/1999/xhtml"
+      xmlns:xbl="http://www.mozilla.org/xbl">
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=319374
+-->
+<head>
+  <title>Test for Bug 319374</title>
+  <script type="text/javascript" src="/MochiKit/packed.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+  <xbl:bindings>                                  
+    <xbl:binding id="test">
+      <xbl:content>
+        <span attr="attribute"><span></span></span><span> anon text </span><br/>
+      </xbl:content>
+    </xbl:binding>
+  </xbl:bindings>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=319374">Mozilla Bug 319374</a>
+<p id="display"></p>
+<div id="content"><span style="-moz-binding: url(#test)"/><span style="-moz-binding: url(#test)"/><span style="-moz-binding: url(#test)"/></div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+<![CDATA[
+
+/** Test for Bug 319374 **/
+
+  function testChangesInAnonymousTree() {
+    // Test 1: Make sure that modifying anonymous content doesn't 
+    //         cause non-anonymous XPath result to throw exceptions..
+    var counter = 0;
+    var error = null;
+    try {
+      var xp = new XPathEvaluator();
+      var result = xp.evaluate("*",
+                               document.getElementById('content'),
+                               null,
+                               Components.interfaces.nsIDOMXPathResult.
+                                 UNORDERED_NODE_ITERATOR_TYPE,
+                               null);
+      var res = null;
+      while (res = result.iterateNext()) {
+        ++counter; 
+        var anon = document.getAnonymousNodes(res);
+        anon[0].removeChild(anon[0].firstChild); // Removing a child node
+        anon[0].removeAttribute("attr1"); // Removing an attribute
+        anon[1].firstChild.data = "anon text changed" // Modifying text data
+      }
+    } catch (e) {
+      error = e;
+    }
+    ok(!error, error);
+    ok(counter == 3, "XPathEvaluator should have found 3 elements.")
+
+    // Test 2: If the context node is in anonymous content, changing some
+    //         other anonymous tree shouldn't cause XPath result to throw.
+    var anonAttr1 =
+      document.getAnonymousNodes(document.getElementById('content').
+        firstChild)[0].getAttributeNode('attr');
+    var anonAttr2 =
+      document.getAnonymousNodes(document.getElementById('content').
+        lastChild)[0].getAttributeNode('attr');
+    var resultAttr = null;
+    try {
+      var xp2 = xp.evaluate(".",
+                            anonAttr1,
+                            null,
+                            Components.interfaces.nsIDOMXPathResult.
+                              UNORDERED_NODE_ITERATOR_TYPE,
+                            null);
+      // Attribute changing in a different anonymous tree.
+      anonAttr2.value = "foo";
+      resultAttr = xp2.iterateNext();
+      ok(resultAttr == anonAttr1, "XPathEvaluator returned wrong attribute!")
+    } catch (e) {
+      ok(false, e);
+    }
+
+    // Test 3: If the anonymous tree in which context node is in is modified,
+    //         XPath result should throw when iterateNext() is called.
+    resultAttr = null;
+    try {
+      var xp3 = xp.evaluate(".",
+                            anonAttr1,
+                            null,
+                            Components.interfaces.nsIDOMXPathResult.
+                              UNORDERED_NODE_ITERATOR_TYPE,
+                            null);
+      // Attribute changing in the same anonymous tree.
+      anonAttr1.ownerElement.setAttribute("foo", "bar");
+      resultAttr = xp3.iterateNext();
+      ok(resultAttr == anonAttr1,
+         "XPathEvaluator should have thrown an exception!")
+    } catch (e) {
+      ok(true, e);
+    }
+
+    SimpleTest.finish();
+  }
+
+  SimpleTest.waitForExplicitFinish();
+  addLoadEvent(testChangesInAnonymousTree);
+]]>
+</script>
+</pre>
+</body>
+</html>
+