Attachment 369429 Details for Bug 197696 – Patch

[patch] Patch

bug-197696-20190508151134.patch (text/plain), 8.22 KB, created by Chris Dumez on 2019-05-08 15:11:34 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2019-05-08 15:11:34 PDT

Size: 8.22 KB

patch

obsolete

>Subversion Revision: 245064
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index e7cc55d888e65666fc2953457157137052d6d88c..ab8622af6368799b854193528eadd29dfb4b6282 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,26 @@
>+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 NOBODY (OOPS!).
>+
>+        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  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         [iOS] Add a quirk to synthesize mouse events when modifying the selection
>diff --git a/Source/WebCore/dom/Node.cpp b/Source/WebCore/dom/Node.cpp
>index 4aa02cfe3366e2c743cca0642cbc3bfbe529ffd9..d6f434b32c689632c9f78f523247514be702744f 100644
>--- a/Source/WebCore/dom/Node.cpp
>+++ b/Source/WebCore/dom/Node.cpp
>@@ -2118,13 +2118,6 @@ static inline bool tryAddEventListener(Node* targetNode, const AtomicString& eve
>     if (targetNode == &targetNode->document() && eventType == eventNames().scrollEvent)
>         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))
>         targetNode->document().addTouchEventListener(*targetNode);
>@@ -2160,12 +2153,6 @@ static inline bool tryRemoveEventListener(Node* targetNode, const AtomicString&
>     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)
>     if (eventNames().isTouchRelatedEventType(targetNode->document(), eventType))
>         targetNode->document().removeTouchEventListener(*targetNode);
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index f32b5a20b8f25d234408b9c7c43bb5db7fe46b9d..32808c578400e243af65a2a7c193d6d77bca47b0 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,16 @@
>+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 NOBODY (OOPS!).
>+
>+        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  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         [iOS] Add a quirk to synthesize mouse events when modifying the selection
>diff --git a/LayoutTests/fast/events/ios/rotation/orientationchange-event-listener-on.body-expected.txt b/LayoutTests/fast/events/ios/rotation/orientationchange-event-listener-on.body-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..85c5edbfd2e13acaa8521be8a2f038b6224f69ce
>--- /dev/null
>+++ b/LayoutTests/fast/events/ios/rotation/orientationchange-event-listener-on.body-expected.txt
>@@ -0,0 +1,15 @@
>+Tests the behavior of the resize / orientationchange event listeners on both the body and the window.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+Before rotation
>+PASS In body's onorientationchange event handler
>+PASS In window's orientationchange event listener
>+PASS In body's onresize event handler
>+PASS In window's resize event listener
>+After rotation
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+Rotation test.
>diff --git a/LayoutTests/fast/events/ios/rotation/orientationchange-event-listener-on.body.html b/LayoutTests/fast/events/ios/rotation/orientationchange-event-listener-on.body.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..f5b2d66f33e4fb62e013e6df2140d91c5d71579a
>--- /dev/null
>+++ b/LayoutTests/fast/events/ios/rotation/orientationchange-event-listener-on.body.html
>@@ -0,0 +1,67 @@
>+<!DOCTYPE html> 
>+
>+<html>
>+<head>
>+    <meta name="viewport" content="initial-scale=1.0">
>+    <script src="../../../../resources/js-test.js"></script>
>+</head>
>+<body onorientationchange="bodyOrientationChange()" onresize="bodyResize()">
>+    <script>
>+        description("Tests the behavior of the resize / orientationchange event listeners on both the body and the window.");
>+        jsTestIsAsync = true;
>+
>+        function getRotationUIScript()
>+        {
>+            return `
>+            (function() {
>+                uiController.simulateRotation('landscape-right', function() {
>+                    uiController.doAfterVisibleContentRectUpdate(function () {
>+                        uiController.uiScriptComplete();
>+                    })
>+                });
>+            })();`
>+        }
>+
>+        function doTest()
>+        {
>+            debug('Before rotation');
>+            if (!window.testRunner)
>+                return;
>+
>+            testRunner.runUIScript(getRotationUIScript(), function(result) {
>+                debug('After rotation');
>+
>+                finishJSTest();
>+            });
>+        }
>+        window.addEventListener('resize', function() {
>+            testPassed("In window's resize event listener");
>+        }, false);
>+
>+        window.addEventListener('orientationchange', function() {
>+            testPassed("In window's orientationchange event listener");
>+        }, false);
>+
>+        document.body.addEventListener('resize', function() {
>+            testFailed("In body's resize event listener");
>+        }, false);
>+
>+        document.body.addEventListener('orientationchange', function() {
>+            testFailed("In body's orientationchange event listener");
>+        }, false);
>+
>+        function bodyOrientationChange()
>+        {
>+            testPassed("In body's onorientationchange event handler");
>+        }
>+
>+        function bodyResize()
>+        {
>+            testPassed("In body's onresize event handler");
>+        }
>+
>+        window.addEventListener('load', doTest, false);
>+    </script>
>+    Rotation test.
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 197696: 369429 | 369448 | 369993