Changeset 245086 in webkit

Timestamp:

May 8, 2019 6:34:05 PM (5 years ago)

Author:

Chris Dumez

Message:

[iOS Debug] ASSERTION FAILED: !m_originalNode in WebCore::JSLazyEventListener::checkValidityForEventTarget(WebCore::EventTarget &)
​https://bugs.webkit.org/show_bug.cgi?id=197696
<rdar://problem/50586956>

Reviewed by Simon Fraser.

Source/WebCore:

Setting the onorientationchange / onresize event handler on the body should set the event handler on the
window object, as per the HTML specification. However, calling body.addEventListener() with 'orientationchange'
or 'resize' should not set the event listener on the window object, only the body. Blink and Gecko seem to
behave as per specification but WebKit had a quirk for the addEventListener case. The quirk's implementation
is slightly wrong (because it is unsafe to take a JSLazyEventListener from a body element and add it to the
window, given that the JSLazyEventListener keeps a raw pointer to its element) and was causing crashes such
as <rdar://problem/24314027>. As a result, this patch simply drops the WebKit quirk, which will align our
behavior with other browsers and fix the crashes altogether.

Test: fast/events/ios/rotation/orientationchange-event-listener-on.body.html

• dom/Node.cpp:

(WebCore::tryAddEventListener):
(WebCore::tryRemoveEventListener):

LayoutTests:

Add layout test coverage.

• fast/events/ios/rotation/orientationchange-event-listener-on.body-expected.txt: Added.
• fast/events/ios/rotation/orientationchange-event-listener-on.body.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/events/ios/rotation/orientationchange-event-listener-on.body-expected.txt (added)
• LayoutTests/fast/events/ios/rotation/orientationchange-event-listener-on.body.html (added)
• LayoutTests/platform/ios/fast/dom/event-handler-attributes-expected.txt (deleted)
• LayoutTests/platform/ios/legacy-animation-engine/fast/dom (deleted)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/Node.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-05-08  Chris Dumez  <cdumez@apple.com>
+
+        [iOS Debug] ASSERTION FAILED: !m_originalNode in WebCore::JSLazyEventListener::checkValidityForEventTarget(WebCore::EventTarget &)
+        https://bugs.webkit.org/show_bug.cgi?id=197696
+        <rdar://problem/50586956>
+
+        Reviewed by Simon Fraser.
+
+        Add layout test coverage.
+
+        * fast/events/ios/rotation/orientationchange-event-listener-on.body-expected.txt: Added.
+        * fast/events/ios/rotation/orientationchange-event-listener-on.body.html: Added.
+
 2019-05-08  Ryan Haddad  <ryanhaddad@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-05-08  Chris Dumez  <cdumez@apple.com>
+
+        [iOS Debug] ASSERTION FAILED: !m_originalNode in WebCore::JSLazyEventListener::checkValidityForEventTarget(WebCore::EventTarget &)
+        https://bugs.webkit.org/show_bug.cgi?id=197696
+        <rdar://problem/50586956>
+
+        Reviewed by Simon Fraser.
+
+        Setting the onorientationchange / onresize event handler on the body should set the event handler on the
+        window object, as per the HTML specification. However, calling body.addEventListener() with 'orientationchange'
+        or 'resize' should not set the event listener on the window object, only the body. Blink and Gecko seem to
+        behave as per specification but WebKit had a quirk for the addEventListener case. The quirk's implementation
+        is slightly wrong (because it is unsafe to take a JSLazyEventListener from a body element and add it to the
+        window, given that the JSLazyEventListener keeps a raw pointer to its element) and was causing crashes such
+        as <rdar://problem/24314027>. As a result, this patch simply drops the WebKit quirk, which will align our
+        behavior with other browsers and fix the crashes altogether.
+
+        Test: fast/events/ios/rotation/orientationchange-event-listener-on.body.html
+
+        * dom/Node.cpp:
+        (WebCore::tryAddEventListener):
+        (WebCore::tryRemoveEventListener):
+
 2019-05-08  Ryan Haddad  <ryanhaddad@apple.com>
 

trunk/Source/WebCore/dom/Node.cpp

@@ -2119 +2119 @@
         targetNode->document().domWindow()->incrementScrollEventListenersCount();
 
-    // FIXME: Would it be sufficient to special-case this code for <body> and <frameset>?
-    //
-    // This code was added to address <rdar://problem/5846492> Onorientationchange event not working for document.body.
-    // Forward this call to addEventListener() to the window since these are window-only events.
-    if (eventType == eventNames().orientationchangeEvent || eventType == eventNames().resizeEvent)
-        targetNode->document().domWindow()->addEventListener(eventType, WTFMove(listener), options);
-
 #if ENABLE(TOUCH_EVENTS)
     if (eventNames().isTouchRelatedEventType(targetNode->document(), eventType))
@@ -2160 +2153 @@
     if (targetNode == &targetNode->document() && eventType == eventNames().scrollEvent)
         targetNode->document().domWindow()->decrementScrollEventListenersCount();
-
-    // FIXME: Would it be sufficient to special-case this code for <body> and <frameset>? See <rdar://problem/15647823>.
-    // This code was added to address <rdar://problem/5846492> Onorientationchange event not working for document.body.
-    // Forward this call to removeEventListener() to the window since these are window-only events.
-    if (eventType == eventNames().orientationchangeEvent || eventType == eventNames().resizeEvent)
-        targetNode->document().domWindow()->removeEventListener(eventType, listener, options);
 
 #if ENABLE(TOUCH_EVENTS)