730244 - XForms for Gecko 6

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Philipp Wagner [:imphil]]

Attachment: patch

Adapt xforms to the changes made in Gecko 6. This includes the following items:
- Bug 610305 - decom nsEventStateManager
- Bug 655517 - Remove nsIDOMDocumentEvent
- Bug 616684 - Remove support for DOM Views
- Remove the XForms-specific implementation of DOM3 IsNodeEqual and use that instead

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 600348 [details] [diff] [review]
patch


>       
>-            contentsEqual = nsXFormsUtils::AreNodesEqual(child1, child2, PR_TRUE);
>+            child1->IsEqualNode(child2, &contentsEqual);
>             if (!contentsEqual) {
So here you miss the normalization. Is that ok?

Comment 2

[Philipp Wagner [:imphil]]

(In reply to Olli Pettay [:smaug] from comment #1)
> Comment on attachment 600348 [details] [diff] [review]
> patch
> 
> 
> >       
> >-            contentsEqual = nsXFormsUtils::AreNodesEqual(child1, child2, PR_TRUE);
> >+            child1->IsEqualNode(child2, &contentsEqual);
> >             if (!contentsEqual) {
> So here you miss the normalization. Is that ok?

PR_TRUE is for aAlreadyNormalized, so in this case I don't miss normalization, but in the other cases I do.
But good catch, I actually misread the DOM3 spec "to avoid this, nodes should be normalized before being compared." (http://www.w3.org/TR/DOM-Level-3-Core/core.html#Node3-isEqualNode) as "the implementation of isEqualNode() should normalize the nodes before comparing". Looking at the implementation this interpretation is wrong and I need to do the normalization myself. New patch coming up.

Comment 3

[Philipp Wagner [:imphil]]

Attachment: patch v2

for easier review, diff between the two patches:

diff --git a/nsXFormsItemElement.cpp b/nsXFormsItemElement.cpp
--- a/nsXFormsItemElement.cpp
+++ b/nsXFormsItemElement.cpp
@@ -268,18 +268,34 @@ nsXFormsItemElement::SelectItemByNode(ns
   NS_ENSURE_STATE(copyNode);
 
   PRUint16 nodeType;
   copyNode->GetNodeType(&nodeType);
 
   // copy elements are only allowed to bind to ELEMENT_NODEs per spec.  But
   // test first before doing all of this work.
   if (nodeType == nsIDOMNode::ELEMENT_NODE) {
+    nsCOMPtr<nsIDOMNode> clone1, clone2;
+    nsresult rv;
+    rv = aNode->CloneNode(PR_TRUE, getter_AddRefs(clone1));
+    if (NS_FAILED(rv) || !clone1) {
+      return NS_ERROR_FAILURE;
+    }
+    rv = copyNode->CloneNode(PR_TRUE, getter_AddRefs(clone2));
+    if (NS_FAILED(rv) || !clone2) {
+      return NS_ERROR_FAILURE;
+    }
+
+    rv = clone1->Normalize();
+    if (NS_FAILED(rv)) return rv;
+    rv = clone2->Normalize();
+    if (NS_FAILED(rv)) return rv;
+
     PRBool isEqualNode = PR_FALSE;
-    copyNode->IsEqualNode(aNode, &isEqualNode);
+    clone1->IsEqualNode(clone2, &isEqualNode);
     if (isEqualNode) {
       NS_ADDREF(*aSelected = mElement);
     }
   }
 
   return NS_OK;
 }

Comment 4

[Philipp Wagner [:imphil]]

http://hg.mozilla.org/xforms/rev/8fd9a06f667d