Status

Core Graveyard
XForms
RESOLVED FIXED
6 years ago
2 years ago

People

(Reporter: imphil, Assigned: imphil)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
Created attachment 600348 [details] [diff] [review]
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
Attachment #600348 - Flags: review?(bugs)
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?
(Assignee)

Comment 2

6 years ago
(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.
(Assignee)

Comment 3

6 years ago
Created attachment 600405 [details] [diff] [review]
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;
 }
Attachment #600348 - Attachment is obsolete: true
Attachment #600405 - Flags: review?(bugs)
Attachment #600348 - Flags: review?(bugs)
(Assignee)

Comment 4

6 years ago
http://hg.mozilla.org/xforms/rev/8fd9a06f667d
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.