LayoutTests:
authordarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Jun 2006 01:33:34 +0000 (01:33 +0000)
committerdarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Jun 2006 01:33:34 +0000 (01:33 +0000)
        Reviewed by Geoff.

        - test for <rdar://problem/4585333> Changing location for weather on yahoo.com home page redirects to another page

        * fast/events/event-listener-html-non-html-confusion-expected.txt: Added.
        * fast/events/event-listener-html-non-html-confusion.html: Added.

WebCore:

        Reviewed by Geoff.

        - fix <rdar://problem/4585333> Changing location for weather on yahoo.com home page redirects to another page

        This patch fixes a bug where the event listener cache does not distinguish
        HTML and non-HTML listeners. Incorrect behavior where stopPropagation also
        prevented default masked a case of this bug on the yahoo.com home page until
        we fixed bug 5180 on 2005-10-03.

        Test: fast/events/event-listener-html-non-html-confusion.html

        * bindings/js/kjs_window.h: Add additional listener maps for HTML event listeners.

        * bindings/js/kjs_window.cpp:
        (KJS::Window::~Window): Go through the additional maps when clearing the window object
        pointer in event listeners.
        (KJS::Window::getJSEventListener): Look in the HTML or non-HTML map depending on the
        argument passed.
        (KJS::Window::getJSUnprotectedEventListener): Ditto.

        * bindings/js/kjs_events.cpp:
        (KJS::JSUnprotectedEventListener::JSUnprotectedEventListener): Add to either the HTML
        or non-HTML map depending on the argument passed.
        (KJS::JSUnprotectedEventListener::~JSUnprotectedEventListener): Remove from either the
        HTML or non-HTML map depending on whether the HTML flag is set.
        (KJS::JSEventListener::JSEventListener): More of the same.
        (KJS::JSEventListener::~JSEventListener): Ditto.
        (KJS::JSLazyEventListener::parseCode): Same thing here. In a lazy event listener there
        is not a listener at construction time, thus the code here to put the listener into a
        map needs the HTML vs. non-HTML logic.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@14856 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/events/event-listener-html-non-html-confusion-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/event-listener-html-non-html-confusion.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/bindings/js/kjs_events.cpp
WebCore/bindings/js/kjs_window.cpp
WebCore/bindings/js/kjs_window.h

index 3fda27d542ea170bc6eea2199b3f4649f96a4f70..fd16053f611281574aa4b333cd87b3424e9dc127 100644 (file)
@@ -1,3 +1,12 @@
+2006-06-13  Darin Adler  <darin@apple.com>
+
+        Reviewed by Geoff.
+
+        - test for <rdar://problem/4585333> Changing location for weather on yahoo.com home page redirects to another page
+
+        * fast/events/event-listener-html-non-html-confusion-expected.txt: Added.
+        * fast/events/event-listener-html-non-html-confusion.html: Added.
+
 2006-06-13  Maciej Stachowiak  <mjs@apple.com>
 
         Reviewed by Geoff.
diff --git a/LayoutTests/fast/events/event-listener-html-non-html-confusion-expected.txt b/LayoutTests/fast/events/event-listener-html-non-html-confusion-expected.txt
new file mode 100644 (file)
index 0000000..f236403
--- /dev/null
@@ -0,0 +1,7 @@
+This test checks for a particular problem in WebKit internals where adding the same function first as a non-HTML and then as an HTML event listener could leave the "is HTML" flag set wrong.
+
+If the test succeeds, you should see the word SUCCESS below. Otherwise, you'll see the word FAILURE or nothing at all.
+
+SUCCESS
+
+test anchor - script clicks this automatically
diff --git a/LayoutTests/fast/events/event-listener-html-non-html-confusion.html b/LayoutTests/fast/events/event-listener-html-non-html-confusion.html
new file mode 100644 (file)
index 0000000..d87496e
--- /dev/null
@@ -0,0 +1,30 @@
+<p>This test checks for a particular problem in WebKit internals where adding the same function
+first as a non-HTML and then as an HTML event listener could leave the "is HTML" flag set wrong.</p>
+
+<p>If the test succeeds, you should see the word SUCCESS below. Otherwise, you'll see the word FAILURE or nothing at all.</p>
+
+<p id="result"></p>
+
+<a href="javascript:document.getElementById('result').innerHTML = 'FAILURE'" id="anchor">test anchor - script clicks this automatically</a>
+
+<script>
+
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+
+function listener(event)
+{
+    document.getElementById('result').innerHTML = 'SUCCESS';
+    return false;
+}
+
+var anchor = document.getElementById("anchor");
+
+anchor.addEventListener("click", listener, false);
+anchor.onclick = listener;
+
+var clickEvent = document.createEvent("MouseEvents");
+clickEvent.initMouseEvent("click", true, true, null, 1, 1, 1, 1, 1, false, false, false, false, 0, document);
+anchor.dispatchEvent(clickEvent);
+
+</script>
index 080c081723bfb86a1575ce57c139faca2ad562cf..4f459305e10d7d20a8e96eb62f9feb5f76c931a3 100644 (file)
@@ -1,3 +1,36 @@
+2006-06-13  Darin Adler  <darin@apple.com>
+
+        Reviewed by Geoff.
+
+        - fix <rdar://problem/4585333> Changing location for weather on yahoo.com home page redirects to another page
+
+        This patch fixes a bug where the event listener cache does not distinguish
+        HTML and non-HTML listeners. Incorrect behavior where stopPropagation also
+        prevented default masked a case of this bug on the yahoo.com home page until
+        we fixed bug 5180 on 2005-10-03.
+
+        Test: fast/events/event-listener-html-non-html-confusion.html
+
+        * bindings/js/kjs_window.h: Add additional listener maps for HTML event listeners.
+
+        * bindings/js/kjs_window.cpp:
+        (KJS::Window::~Window): Go through the additional maps when clearing the window object
+        pointer in event listeners.
+        (KJS::Window::getJSEventListener): Look in the HTML or non-HTML map depending on the
+        argument passed.
+        (KJS::Window::getJSUnprotectedEventListener): Ditto.
+
+        * bindings/js/kjs_events.cpp:
+        (KJS::JSUnprotectedEventListener::JSUnprotectedEventListener): Add to either the HTML
+        or non-HTML map depending on the argument passed.
+        (KJS::JSUnprotectedEventListener::~JSUnprotectedEventListener): Remove from either the
+        HTML or non-HTML map depending on whether the HTML flag is set.
+        (KJS::JSEventListener::JSEventListener): More of the same.
+        (KJS::JSEventListener::~JSEventListener): Ditto.
+        (KJS::JSLazyEventListener::parseCode): Same thing here. In a lazy event listener there
+        is not a listener at construction time, thus the code here to put the listener into a
+        map needs the HTML vs. non-HTML logic.
+
 2006-06-13  Maciej Stachowiak  <mjs@apple.com>
 
         Reviewed by Darin.
index 6873b35ed4c3c31e2c4e87f73330939397253815..a8492e1a44075f21cb4d896bb67cdb838b52449b 100644 (file)
@@ -144,14 +144,20 @@ JSUnprotectedEventListener::JSUnprotectedEventListener(JSObject* _listener, Wind
   , listener(_listener)
   , win(_win)
 {
-    if (_listener)
-        _win->jsUnprotectedEventListeners.set(_listener, this);
+    if (_listener) {
+        Window::UnprotectedListenersMap& listeners = _html
+            ? _win->jsUnprotectedHTMLEventListeners : _win->jsUnprotectedEventListeners;
+        listeners.set(_listener, this);
+    }
 }
 
 JSUnprotectedEventListener::~JSUnprotectedEventListener()
 {
-    if (listener && win)
-        win->jsUnprotectedEventListeners.remove(listener);
+    if (listener && win) {
+        Window::UnprotectedListenersMap& listeners = isHTMLEventListener()
+            ? win->jsUnprotectedHTMLEventListeners : win->jsUnprotectedEventListeners;
+        listeners.remove(listener);
+    }
 }
 
 JSObject* JSUnprotectedEventListener::listenerObj() const
@@ -182,14 +188,20 @@ JSEventListener::JSEventListener(JSObject* _listener, Window* _win, bool _html)
     , listener(_listener)
     , win(_win)
 {
-    if (_listener)
-        _win->jsEventListeners.set(_listener, this);
+    if (_listener) {
+        Window::ListenersMap& listeners = _html
+            ? _win->jsHTMLEventListeners : _win->jsEventListeners;
+        listeners.set(_listener, this);
+    }
 }
 
 JSEventListener::~JSEventListener()
 {
-    if (listener && win)
-        win->jsEventListeners.remove(listener);
+    if (listener && win) {
+        Window::ListenersMap& listeners = isHTMLEventListener()
+            ? win->jsHTMLEventListeners : win->jsEventListeners;
+        listeners.remove(listener);
+    }
 }
 
 JSObject* JSEventListener::listenerObj() const
@@ -282,8 +294,11 @@ void JSLazyEventListener::parseCode() const
     m_functionName = String();
     code = String();
 
-    if (listener)
-        windowObj()->jsEventListeners.set(listener, const_cast<JSLazyEventListener*>(this));
+    if (listener) {
+        Window::ListenersMap& listeners = isHTMLEventListener()
+            ? windowObj()->jsHTMLEventListeners : windowObj()->jsEventListeners;
+        listeners.set(listener, const_cast<JSLazyEventListener*>(this));
+    }
 }
 
 JSValue* getNodeEventListener(EventTargetNode* n, const AtomicString& eventType)
index 4d5f601473801f0af1afe7d12d214a9212c44712..74b8a4b3d1f05d6bfbd1d971c7126be80c40ff6e 100644 (file)
@@ -333,15 +333,23 @@ Window::~Window()
 
     // Clear any backpointers to the window
 
-    UnprotectedListenersMap::iterator i1 = jsUnprotectedEventListeners.begin();
-    UnprotectedListenersMap::iterator e1 = jsUnprotectedEventListeners.end();
-    for (; i1 != e1; ++i1)
-        i1->second->clearWindowObj();
-
     ListenersMap::iterator i2 = jsEventListeners.begin();
     ListenersMap::iterator e2 = jsEventListeners.end();
     for (; i2 != e2; ++i2)
         i2->second->clearWindowObj();
+    i2 = jsHTMLEventListeners.begin();
+    e2 = jsHTMLEventListeners.end();
+    for (; i2 != e2; ++i2)
+        i2->second->clearWindowObj();
+
+    UnprotectedListenersMap::iterator i1 = jsUnprotectedEventListeners.begin();
+    UnprotectedListenersMap::iterator e1 = jsUnprotectedEventListeners.end();
+    for (; i1 != e1; ++i1)
+        i1->second->clearWindowObj();
+    i1 = jsUnprotectedHTMLEventListeners.begin();
+    e1 = jsUnprotectedHTMLEventListeners.end();
+    for (; i1 != e1; ++i1)
+        i1->second->clearWindowObj();
 }
 
 DOMWindow* Window::impl() const
@@ -1293,7 +1301,8 @@ JSEventListener *Window::getJSEventListener(JSValue *val, bool html)
     return 0;
   JSObject *object = static_cast<JSObject *>(val);
 
-  if (JSEventListener* listener = jsEventListeners.get(object))
+  ListenersMap& listeners = html ? jsHTMLEventListeners : jsEventListeners;
+  if (JSEventListener* listener = listeners.get(object))
     return listener;
 
   // Note that the JSEventListener constructor adds it to our jsEventListeners list
@@ -1306,7 +1315,8 @@ JSUnprotectedEventListener *Window::getJSUnprotectedEventListener(JSValue *val,
     return 0;
   JSObject* object = static_cast<JSObject *>(val);
 
-  if (JSUnprotectedEventListener* listener = jsUnprotectedEventListeners.get(object))
+  UnprotectedListenersMap& listeners = html ? jsUnprotectedHTMLEventListeners : jsUnprotectedEventListeners;
+  if (JSUnprotectedEventListener* listener = listeners.get(object))
     return listener;
 
   // The JSUnprotectedEventListener constructor adds it to our jsUnprotectedEventListeners map.
index ef102d40ca1990b5d2d122408d8ce1f18174f0cf..3619ed805a9f96e255254a617cd001c7eee208b3 100644 (file)
@@ -150,7 +150,9 @@ namespace KJS {
     typedef HashMap<JSObject*, JSEventListener*> ListenersMap;
     typedef HashMap<JSObject*, JSUnprotectedEventListener*> UnprotectedListenersMap;
     ListenersMap jsEventListeners;
+    ListenersMap jsHTMLEventListeners;
     UnprotectedListenersMap jsUnprotectedEventListeners;
+    UnprotectedListenersMap jsUnprotectedHTMLEventListeners;
     virtual const ClassInfo* classInfo() const { return &info; }
     static const ClassInfo info;
     enum { Closed, Crypto, DefaultStatus, Status, DOMException, Frames, History_, Event_, InnerHeight,