Attachment 12621 Details for Bug 9508 – proposed fix

[patch] proposed fix

jsformcomp.diff (text/plain), 8.98 KB, created by Sanjay Madhav (chmmravatar) on 2007-01-23 02:09:06 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Sanjay Madhav (chmmravatar)

Created: 2007-01-23 02:09:06 PST

Size: 8.98 KB

patch

obsolete

>Index: JavaScriptCore/ChangeLog
>===================================================================
>--- JavaScriptCore/ChangeLog	(revision 19038)
>+++ JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,19 @@
>+2007-01-23  Sanjay Madhav  <sanjay12@gmail.com>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Fix for: http://bugs.webkit.org/show_bug.cgi?id=9508
>+        JS comparisons between the same form group now are correct.
>+
>+        Test: LayoutTests/fast/form/form-comp-test.html
>+
>+        * kjs/object.h:
>+        (KJS::JSObject::needsDeepEqual):
>+        (KJS::JSObject::deepEqual):
>+        * kjs/operations.cpp:
>+        (KJS::equal):
>+        (KJS::strictEqual):
>+
> 2007-01-18  Sanjay Madhav  <sanjay12@gmail.com>
> 
>         Reviewed by Darin.
>Index: JavaScriptCore/kjs/object.h
>===================================================================
>--- JavaScriptCore/kjs/object.h	(revision 19038)
>+++ JavaScriptCore/kjs/object.h	(working copy)
>@@ -454,6 +454,13 @@ namespace KJS {
>     void restoreProperties(const SavedProperties &p) { _prop.restore(p); }
> 
>     virtual bool isActivation() { return false; }
>+    
>+    // Derived objects which need to do deep equality comparisons in operations.cpp
>+    // should overload these functions.
>+    // For the most part, this would only be used for situations where you're creating
>+    // temporary objects, and the default address comparison would be invalid (such as with forms).
>+    virtual bool needsDeepEqual() const { return false; }
>+    virtual bool deepEqual(const JSObject*) const { return false; }
>   protected:
>     PropertyMap _prop;
>   private:
>Index: JavaScriptCore/kjs/operations.cpp
>===================================================================
>--- JavaScriptCore/kjs/operations.cpp	(revision 19038)
>+++ JavaScriptCore/kjs/operations.cpp	(working copy)
>@@ -158,6 +158,10 @@ bool equal(ExecState *exec, JSValue *v1,
>     if (t1 == BooleanType)
>         return v1->toBoolean(exec) == v2->toBoolean(exec);
>     
>+    JSObject* o1 = v1->getObject();
>+    if (o1->needsDeepEqual())
>+        return o1->deepEqual(v2->getObject());
>+
>     // types are Object
>     return v1 == v2;
> }
>@@ -182,6 +186,10 @@ bool strictEqual(ExecState *exec, JSValu
>     else if (t2 == BooleanType)
>         return v1->toBoolean(exec) == v2->toBoolean(exec);
>     
>+    JSObject* o1 = v1->getObject();
>+    if (o1->needsDeepEqual())
>+        return o1->deepEqual(v2->getObject());
>+    
>     if (v1 == v2)
>         return true;
>     /* TODO: joined objects */
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 19038)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,13 @@
>+2007-01-23  Sanjay Madhav  <sanjay12@gmail.com>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Test for: http://bugs.webkit.org/show_bug.cgi?id=9508
>+        JS comparisons between the same form group now are correct.
>+
>+        * fast/forms/form-comp-test-expected.txt: Added.
>+        * fast/forms/form-comp-test.html: Added.
>+
> 2007-01-22  Mitz Pettel  <mitz@webkit.org>
> 
>         Reviewed by Darin.
>Index: LayoutTests/fast/forms/form-comp-test-expected.txt
>===================================================================
>--- LayoutTests/fast/forms/form-comp-test-expected.txt	(revision 0)
>+++ LayoutTests/fast/forms/form-comp-test-expected.txt	(revision 0)
>@@ -0,0 +1,31 @@
>+layer at (0,0) size 800x600
>+  RenderView at (0,0) size 800x600
>+layer at (0,0) size 800x600
>+  RenderBlock {HTML} at (0,0) size 800x600
>+    RenderBody {BODY} at (8,8) size 784x584
>+      RenderBlock {P} at (0,0) size 784x18
>+        RenderText {#text} at (0,0) size 606x18
>+          text run at (0,0) width 606: "This page tests to see that the self-comparison of form element groups gives the expected results."
>+      RenderBlock {P} at (0,34) size 784x18
>+        RenderText {#text} at (0,0) size 403x18
>+          text run at (0,0) width 403: "If the test passes, you'll see three 'PASS' messages below the hr."
>+      RenderBlock {FORM} at (0,68) size 784x38
>+        RenderBlock {P} at (0,0) size 784x38
>+          RenderBlock {INPUT} at (3,3) size 12x13
>+          RenderText {#text} at (18,0) size 79x18
>+            text run at (18,0) width 79: " theRadio[0]"
>+          RenderBR {BR} at (97,14) size 0x0
>+          RenderBlock {INPUT} at (3,22) size 12x13
>+          RenderText {#text} at (18,19) size 79x18
>+            text run at (18,19) width 79: " theRadio[1]"
>+      RenderBlock {HR} at (0,122) size 784x2 [border: (1px inset #000000)]
>+      RenderBlock {DIV} at (0,132) size 784x45
>+        RenderBlock {DIV} at (0,0) size 784x15 [color=#008000]
>+          RenderText {#text} at (0,0) size 64x15
>+            text run at (0,0) width 64: "PASS: =="
>+        RenderBlock {DIV} at (0,15) size 784x15 [color=#008000]
>+          RenderText {#text} at (0,0) size 72x15
>+            text run at (0,0) width 72: "PASS: ==="
>+        RenderBlock {DIV} at (0,30) size 784x15 [color=#008000]
>+          RenderText {#text} at (0,0) size 64x15
>+            text run at (0,0) width 64: "PASS: !="
>Index: LayoutTests/fast/forms/form-comp-test.html
>===================================================================
>--- LayoutTests/fast/forms/form-comp-test.html	(revision 0)
>+++ LayoutTests/fast/forms/form-comp-test.html	(revision 0)
>@@ -0,0 +1,47 @@
>+<html>
>+<head>
>+<title>Form Comparison test</title>
>+<script>
>+function print(message, color) 
>+{
>+    var paragraph = document.createElement("div");
>+    paragraph.appendChild(document.createTextNode(message));
>+    paragraph.style.fontFamily = "monospace";
>+    if (color)
>+        paragraph.style.color = color;
>+    document.getElementById("console").appendChild(paragraph);
>+}
>+
>+function test()
>+{
>+    if (document.theForm.theRadio == document.theForm.theRadio)
>+        print("PASS: ==", "green");
>+    else
>+        print("FAIL: ==", "red");
>+    
>+    if (document.theForm.theRadio === document.theForm.theRadio)
>+        print("PASS: ===", "green");
>+    else
>+        print("FAIL: ===", "red");
>+        
>+    if (document.theForm.theRadio != document.theForm.theRadio)
>+        print("FAIL: !=", "red");
>+    else
>+        print("PASS: !=", "green");
>+}
>+</script>
>+</head>
>+
>+<body style="color: black" onload="test()">
>+<p>This page tests to see that the self-comparison of form element groups gives the expected results.</p>
>+<p>If the test passes, you'll see three 'PASS' messages below the hr.</p>
>+<form name='theForm'>
>+<p><input type='radio' name='theRadio' value='0' /> theRadio[0]<br />
>+<input type='radio' name='theRadio' value='1' /> theRadio[1]</p>
>+</form>
>+<hr>
>+
>+<div id='console'></div>
>+
>+</body>
>+</html>
>Index: WebCore/ChangeLog
>===================================================================
>--- WebCore/ChangeLog	(revision 19038)
>+++ WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,17 @@
>+2007-01-23  Sanjay Madhav  <sanjay12@gmail.com>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Fix for: http://bugs.webkit.org/show_bug.cgi?id=9508
>+        JS comparisons between the same form group now are correct.
>+
>+        Test: LayoutTests/fast/form/form-comp-test.html
>+
>+        * bindings/js/kjs_dom.cpp:
>+        (KJS::DOMNamedNodesCollection::deepEqual):
>+        * bindings/js/kjs_dom.h:
>+        (KJS::DOMNamedNodesCollection::needsDeepEqual):
>+
> 2007-01-22  Mark Rowe  <mrowe@apple.com>
> 
>         Reviewed by Maciej.
>Index: WebCore/bindings/js/kjs_dom.cpp
>===================================================================
>--- WebCore/bindings/js/kjs_dom.cpp	(revision 19038)
>+++ WebCore/bindings/js/kjs_dom.cpp	(working copy)
>@@ -1172,4 +1172,21 @@ bool DOMNamedNodesCollection::getOwnProp
>   return DOMObject::getOwnPropertySlot(exec, propertyName, slot);
> }
> 
>+bool DOMNamedNodesCollection::deepEqual(const JSObject* rhs) const
>+{
>+    if (!rhs->isObject(&DOMNamedNodesCollection::info))
>+        return false;
>+    
>+    const DOMNamedNodesCollection* d = reinterpret_cast<const DOMNamedNodesCollection*>(rhs);    
>+    if (m_nodes.size() != d->m_nodes.size())
>+        return false;
>+    
>+    // For performance, we can assume that two collections which have the same size and start with the
>+    // same nodes are equivalent, as opposed to iterating through the entire vectors.
>+    if (m_nodes.size() > 0 && m_nodes[0].get() != d->m_nodes[0].get())
>+        return false;
>+    
>+    return true;
>+}
>+
> } // namespace
>Index: WebCore/bindings/js/kjs_dom.h
>===================================================================
>--- WebCore/bindings/js/kjs_dom.h	(revision 19038)
>+++ WebCore/bindings/js/kjs_dom.h	(working copy)
>@@ -151,6 +151,8 @@ namespace KJS {
>     virtual bool getOwnPropertySlot(ExecState *, const Identifier&, PropertySlot&);
>     virtual const ClassInfo* classInfo() const { return &info; }
>     static const ClassInfo info;
>+    virtual bool needsDeepEqual() const { return true; }
>+    virtual bool deepEqual(const JSObject*) const;
> private:
>     static JSValue *lengthGetter(ExecState* exec, JSObject *, const Identifier&, const PropertySlot& slot);
>     static JSValue *indexGetter(ExecState* exec, JSObject *, const Identifier&, const PropertySlot& slot);

Flags:

darin: review-

Actions: View | Formatted Diff | Diff

Attachments on bug 9508: 8922 | 12621 | 12780