Attachment 305101 Details for Bug 169936 – Patch

[patch] Patch

bug-169936-20170322091334.patch (text/plain), 20.06 KB, created by Chris Dumez on 2017-03-22 09:13:37 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2017-03-22 09:13:37 PDT

Size: 20.06 KB

patch

obsolete

>Subversion Revision: 214230
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c7457c7098461186519b4c3aaee6d42ff1f5b71e..77ae2a85477469917f2f5cf5f15f254401b84656 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,32 @@
>+2017-03-21  Chris Dumez  <cdumez@apple.com>
>+
>+        WebKit should disallow beforeunload alerts from web pages users have never interacted with
>+        https://bugs.webkit.org/show_bug.cgi?id=169936
>+        <rdar://problem/23798897>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        WebKit should disallow beforeunload alerts from web pages users have never interacted with.
>+        This reduces the risk of annoyance to the user and is allowed by the specification:
>+        - https://html.spec.whatwg.org/multipage/browsers.html#prompt-to-unload-a-document (Step 8):
>+       which says:
>+       """
>+       The user agent is encouraged to avoid asking the user for confirmation if it judges that doing
>+       so would be annoying, deceptive, or pointless. A simple heuristic might be that if the user
>+       has not interacted with the document, the user agent would not ask for confirmation before
>+       unloading it.
>+       """
>+
>+       Firefox already implements this, Chrome does not.
>+
>+        Tests: fast/events/beforeunload-alert-no-user-interaction.html
>+               fast/events/beforeunload-alert-user-interaction.html
>+               fast/events/beforeunload-alert-user-interaction2.html
>+
>+        * loader/FrameLoader.cpp:
>+        (WebCore::shouldAskForNavigationConfirmation):
>+        (WebCore::FrameLoader::dispatchBeforeUnloadEvent):
>+
> 2017-03-20  Jer Noble  <jer.noble@apple.com>
> 
>         Crash in WebCore: WebCore::CARingBuffer::getCurrentFrameBounds + 28
>diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp
>index 86a740871956f81922706ad0924398903d5bc546..ec7785a825fccf084a87c39c1e085694bf32e17e 100644
>--- a/Source/WebCore/loader/FrameLoader.cpp
>+++ b/Source/WebCore/loader/FrameLoader.cpp
>@@ -3068,14 +3068,15 @@ void FrameLoader::dispatchUnloadEvents(UnloadEventPolicy unloadEventPolicy)
>         m_frame.document()->removeAllEventListeners();
> }
> 
>-static bool shouldAskForNavigationConfirmation(const BeforeUnloadEvent& event)
>+static bool shouldAskForNavigationConfirmation(Document& document, const BeforeUnloadEvent& event)
> {
>+    bool userDidInteractWithPage = document.topDocument().lastHandledUserGestureTimestamp() > 0;
>     // Web pages can request we ask for confirmation before navigating by:
>     // - Cancelling the BeforeUnloadEvent (modern way)
>     // - Setting the returnValue attribute on the BeforeUnloadEvent to a non-empty string.
>     // - Returning a non-empty string from the event handler, which is then set as returnValue
>     //   attribute on the BeforeUnloadEvent.
>-    return event.defaultPrevented() || !event.returnValue().isEmpty();
>+    return userDidInteractWithPage && (event.defaultPrevented() || !event.returnValue().isEmpty());
> }
> 
> bool FrameLoader::dispatchBeforeUnloadEvent(Chrome& chrome, FrameLoader* frameLoaderBeingNavigated)
>@@ -3102,7 +3103,7 @@ bool FrameLoader::dispatchBeforeUnloadEvent(Chrome& chrome, FrameLoader* frameLo
>     if (!beforeUnloadEvent->defaultPrevented())
>         document->defaultEventHandler(beforeUnloadEvent.get());
> 
>-    if (!shouldAskForNavigationConfirmation(beforeUnloadEvent))
>+    if (!shouldAskForNavigationConfirmation(*document, beforeUnloadEvent))
>         return true;
> 
>     // If the navigating FrameLoader has already shown a beforeunload confirmation panel for the current navigation attempt,
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index a62dea538131dd93265f8b37164fce84f506dc7c..de1f11fa0087f34c019d718c88a92c90ee60538e 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,31 @@
>+2017-03-21  Chris Dumez  <cdumez@apple.com>
>+
>+        WebKit should disallow beforeunload alerts from web pages users have never interacted with
>+        https://bugs.webkit.org/show_bug.cgi?id=169936
>+        <rdar://problem/23798897>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * fast/events/before-unload-return-string-conversion-expected.txt:
>+        * fast/events/before-unload-returnValue-expected.txt:
>+        Rebaseline now that the CONFIRM MESSAGE is no longer shown. This is because there is
>+        no user interaction with the page.
>+
>+        * fast/events/beforeunload-alert-no-user-interaction-expected.txt: Added.
>+        * fast/events/beforeunload-alert-no-user-interaction.html: Added.
>+        * fast/events/beforeunload-alert-user-interaction-expected.txt: Added.
>+        * fast/events/beforeunload-alert-user-interaction.html: Added.
>+        * fast/events/beforeunload-alert-user-interaction2-expected.txt: Added.
>+        * fast/events/beforeunload-alert-user-interaction2.html: Added.
>+        Add layout test coverage.
>+
>+        * fast/loader/form-submission-after-beforeunload-cancel.html:
>+        * fast/loader/show-only-one-beforeunload-dialog.html:
>+        * http/tests/misc/iframe-beforeunload-dialog-matching-ancestor-securityorigin.html:
>+        * http/tests/misc/iframe-beforeunload-dialog-not-matching-ancestor-securityorigin.html:
>+        Simulate user interaction with the page so that the CONFIRM MESSAGE log lines are still
>+        shown.
>+
> 2017-03-21  Carlos Alberto Lopez Perez  <clopez@igalia.com>
> 
>         [GTK][Wayland] Enable WebGL, 3D and compositing layout tests
>diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
>index ee3d6db299a87d7627fda1e46ad7a1a2d2966fc5..c529009d6101c4a781b6889a3f6caf9a257432ba 100644
>--- a/LayoutTests/imported/w3c/ChangeLog
>+++ b/LayoutTests/imported/w3c/ChangeLog
>@@ -1,3 +1,16 @@
>+2017-03-21  Chris Dumez  <cdumez@apple.com>
>+
>+        WebKit should disallow beforeunload alerts from web pages users have never interacted with
>+        https://bugs.webkit.org/show_bug.cgi?id=169936
>+        <rdar://problem/23798897>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * web-platform-tests/html/browsers/browsing-the-web/unloading-documents/beforeunload-canceling-expected.txt:
>+        * web-platform-tests/html/webappapis/scripting/events/compile-event-handler-settings-objects-expected.txt:
>+        Rebaseline now that the CONFIRM MESSAGE lines are now longer shown. This is because there is no user interaction
>+        with the page.
>+
> 2017-03-20  Jon Lee  <jonlee@apple.com>
> 
>         Update progression in wpt test for WebRTC.
>diff --git a/LayoutTests/fast/events/before-unload-return-string-conversion-expected.txt b/LayoutTests/fast/events/before-unload-return-string-conversion-expected.txt
>index 24a14e784b3ee317ca18f76620e964c171e6af3d..ea450b8fdefe1f2868d57e8fd514121000004811 100644
>--- a/LayoutTests/fast/events/before-unload-return-string-conversion-expected.txt
>+++ b/LayoutTests/fast/events/before-unload-return-string-conversion-expected.txt
>@@ -1,4 +1,3 @@
>-CONFIRM NAVIGATION: PASS
> Tests that the value returned from a beforeunload event handler gets converted to a string, even if the returnValue property was already set.
> 
> On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>diff --git a/LayoutTests/fast/events/before-unload-returnValue-expected.txt b/LayoutTests/fast/events/before-unload-returnValue-expected.txt
>index 6dd0a557f0e7e55ff59591e8d00fefb0b7e680ac..595d0ee93c710e301b665af31e830b2f5ac527d7 100644
>--- a/LayoutTests/fast/events/before-unload-returnValue-expected.txt
>+++ b/LayoutTests/fast/events/before-unload-returnValue-expected.txt
>@@ -1,4 +1,3 @@
>-CONFIRM NAVIGATION: This is beforeunload from the top level frame.
> Tests the returnValue attribute of the BeforeUnloadEvent.
> 
> On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>diff --git a/LayoutTests/fast/events/beforeunload-alert-no-user-interaction-expected.txt b/LayoutTests/fast/events/beforeunload-alert-no-user-interaction-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..92ce52112dac448ca3b377f6cd4a1616524ce02e
>--- /dev/null
>+++ b/LayoutTests/fast/events/beforeunload-alert-no-user-interaction-expected.txt
>@@ -0,0 +1,9 @@
>+Tests that the beforeunload alert is not shown when the user did not interact with the page. You should NOT see a 'CONFIRM NAVIGATION' message at the top.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/fast/events/beforeunload-alert-no-user-interaction.html b/LayoutTests/fast/events/beforeunload-alert-no-user-interaction.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..a6b913665f6ddc85cade8eab837e117f9aff26d2
>--- /dev/null
>+++ b/LayoutTests/fast/events/beforeunload-alert-no-user-interaction.html
>@@ -0,0 +1,23 @@
>+<!DOCTYPE html>
>+<html>
>+<body>
>+<script src="../../resources/js-test-pre.js"></script>
>+<script>
>+description("Tests that the beforeunload alert is not shown when the user did not interact with the page. You should NOT see a 'CONFIRM NAVIGATION' message at the top.");
>+jsTestIsAsync = true;
>+
>+onload = function() {
>+    const testFrame = document.getElementById("testFrame");
>+    testFrame.contentWindow.onbeforeunload = function(e) {
>+        return "FAIL: a beforeunload alert was shown even though the user did not interact with the page.";
>+    };
>+    setTimeout(function() {
>+        testFrame.src = "about:blank";
>+        setTimeout(finishJSTest, 0);
>+    }, 0);
>+};
>+</script>
>+<iframe id="testFrame" src="resources/onclick.html"></iframe>
>+<script src="../../resources/js-test-post.js"></script>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/events/beforeunload-alert-user-interaction-expected.txt b/LayoutTests/fast/events/beforeunload-alert-user-interaction-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..0f6fa9599addca5241a9c911f2483b0f84c570bf
>--- /dev/null
>+++ b/LayoutTests/fast/events/beforeunload-alert-user-interaction-expected.txt
>@@ -0,0 +1,10 @@
>+CONFIRM NAVIGATION: PASS: a beforeunload alert was shown.
>+Tests that the beforeunload alert is shown when the user interacted with the page. You should see a 'CONFIRM NAVIGATION' message at the top.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+ 
>diff --git a/LayoutTests/fast/events/beforeunload-alert-user-interaction.html b/LayoutTests/fast/events/beforeunload-alert-user-interaction.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..66a7c70680ecf79276f26d0f72af8a23a8cd8388
>--- /dev/null
>+++ b/LayoutTests/fast/events/beforeunload-alert-user-interaction.html
>@@ -0,0 +1,29 @@
>+<!DOCTYPE html>
>+<html>
>+<body>
>+<script src="../../resources/js-test-pre.js"></script>
>+<script>
>+description("Tests that the beforeunload alert is shown when the user interacted with the page. You should see a 'CONFIRM NAVIGATION' message at the top.");
>+jsTestIsAsync = true;
>+
>+onload = function() {
>+    const testFrame = document.getElementById("testFrame");
>+    testFrame.contentWindow.onbeforeunload = function(e) {
>+        return "PASS: a beforeunload alert was shown.";
>+    };
>+    // Simulate a user interaction.
>+    const testInput = document.getElementById("testInput");
>+    testInput.focus();
>+    if (window.eventSender)
>+        eventSender.keyDown("a");
>+    setTimeout(function() {
>+        testFrame.src = "about:blank";
>+        setTimeout(finishJSTest, 0);
>+    }, 0);
>+};
>+</script>
>+<iframe id="testFrame" src="resources/onclick.html"></iframe>
>+<input id="testInput" type="text"></input>
>+<script src="../../resources/js-test-post.js"></script>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/events/beforeunload-alert-user-interaction2-expected.txt b/LayoutTests/fast/events/beforeunload-alert-user-interaction2-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..0f6fa9599addca5241a9c911f2483b0f84c570bf
>--- /dev/null
>+++ b/LayoutTests/fast/events/beforeunload-alert-user-interaction2-expected.txt
>@@ -0,0 +1,10 @@
>+CONFIRM NAVIGATION: PASS: a beforeunload alert was shown.
>+Tests that the beforeunload alert is shown when the user interacted with the page. You should see a 'CONFIRM NAVIGATION' message at the top.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+ 
>diff --git a/LayoutTests/fast/events/beforeunload-alert-user-interaction2.html b/LayoutTests/fast/events/beforeunload-alert-user-interaction2.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..19b56ac5e0ffc25d874a2cecfd6e367ac75618b3
>--- /dev/null
>+++ b/LayoutTests/fast/events/beforeunload-alert-user-interaction2.html
>@@ -0,0 +1,27 @@
>+<!DOCTYPE html>
>+<html>
>+<body>
>+<script src="../../resources/js-test-pre.js"></script>
>+<script src="../../resources/ui-helper.js"></script>
>+<script>
>+description("Tests that the beforeunload alert is shown when the user interacted with the page. You should see a 'CONFIRM NAVIGATION' message at the top.");
>+jsTestIsAsync = true;
>+
>+onload = function() {
>+    const testFrame = document.getElementById("testFrame");
>+    testFrame.contentWindow.onbeforeunload = function(e) {
>+        return "PASS: a beforeunload alert was shown.";
>+    };
>+    // Simulate a user interaction.
>+    const testInput = document.getElementById("testInput");
>+    UIHelper.activateAt(testInput.offsetLeft + 5, testInput.offsetTop + 5).then(function() {
>+        testFrame.src = "about:blank";
>+        setTimeout(finishJSTest, 0);
>+    });
>+};
>+</script>
>+<iframe id="testFrame" src="resources/onclick.html"></iframe>
>+<input id="testInput" type="text"></input>
>+<script src="../../resources/js-test-post.js"></script>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/loader/form-submission-after-beforeunload-cancel.html b/LayoutTests/fast/loader/form-submission-after-beforeunload-cancel.html
>index 48d56e10922c3a6d7ce3daf2638c6225ecb7aedb..e1f8d721ba6278ba5457041646231b2f357bc546 100644
>--- a/LayoutTests/fast/loader/form-submission-after-beforeunload-cancel.html
>+++ b/LayoutTests/fast/loader/form-submission-after-beforeunload-cancel.html
>@@ -1,3 +1,4 @@
>+<script src="../../resources/ui-helper.js"></script>
> <script>
> if (window.testRunner) {
>     testRunner.waitUntilDone();
>@@ -39,5 +40,8 @@ window.addEventListener("beforeunload", function() {
> </form>
> 
> <script>
>-document.forms[0].submit();
>-</script>
>\ No newline at end of file
>+// Simulate a user interaction with the page so that the beforeunload alert shows.
>+UIHelper.activateAt(1, 1).then(function() {
>+    document.forms[0].submit();
>+});
>+</script>
>diff --git a/LayoutTests/fast/loader/show-only-one-beforeunload-dialog.html b/LayoutTests/fast/loader/show-only-one-beforeunload-dialog.html
>index c5d1b284b815fe69474ef5ea6502b738fb90fe6b..c81ee96b485aa9240cb05aea4e07de879a089338 100644
>--- a/LayoutTests/fast/loader/show-only-one-beforeunload-dialog.html
>+++ b/LayoutTests/fast/loader/show-only-one-beforeunload-dialog.html
>@@ -1,3 +1,4 @@
>+<script src="../../resources/ui-helper.js"></script>
> <script>
> 
> if (window.testRunner) {
>@@ -7,7 +8,10 @@ if (window.testRunner) {
> 
> window.onload = function()
> {
>-	window.setTimeout("window.location.href = 'resources/notify-done.html';", 0);
>+	// Simulate a user interaction with the page so that the beforeunload alert shows.
>+	UIHelper.activateAt(1, 1).then(function() {
>+		window.location.href = 'resources/notify-done.html';
>+	});
> }
> 
> window.onbeforeunload = function()
>diff --git a/LayoutTests/http/tests/misc/iframe-beforeunload-dialog-matching-ancestor-securityorigin.html b/LayoutTests/http/tests/misc/iframe-beforeunload-dialog-matching-ancestor-securityorigin.html
>index 200222fc3365fe8e43d1ad3343a515a6e304525d..c8ffc1c5b105f349b77eecc945efc46b3b143e3d 100644
>--- a/LayoutTests/http/tests/misc/iframe-beforeunload-dialog-matching-ancestor-securityorigin.html
>+++ b/LayoutTests/http/tests/misc/iframe-beforeunload-dialog-matching-ancestor-securityorigin.html
>@@ -1,3 +1,4 @@
>+<script src="/js-test-resources/ui-helper.js"></script>
> <script>
> 
> if (window.testRunner) {
>@@ -7,7 +8,10 @@ if (window.testRunner) {
> 
> window.onload = function()
> {
>-	window.setTimeout("window.location.href = 'resources/notify-done.html';", 0);
>+	// Simulate a user interaction with the page so that the beforeunload alert shows.
>+	UIHelper.activateAt(1, 1).then(function() {
>+		window.location.href = 'resources/notify-done.html';
>+	});
> }
> 
> </script>
>diff --git a/LayoutTests/http/tests/misc/iframe-beforeunload-dialog-not-matching-ancestor-securityorigin.html b/LayoutTests/http/tests/misc/iframe-beforeunload-dialog-not-matching-ancestor-securityorigin.html
>index 374072b4805b1c02f23d21ba4b778bf74b643d82..1e491810c456b4a87ecc81720db0ddc38ad69028 100644
>--- a/LayoutTests/http/tests/misc/iframe-beforeunload-dialog-not-matching-ancestor-securityorigin.html
>+++ b/LayoutTests/http/tests/misc/iframe-beforeunload-dialog-not-matching-ancestor-securityorigin.html
>@@ -1,3 +1,4 @@
>+<script src="/js-test-resources/ui-helper.js"></script>
> <script>
> 
> if (window.testRunner) {
>@@ -7,7 +8,10 @@ if (window.testRunner) {
> 
> window.onload = function()
> {
>-	window.setTimeout("window.location.href = 'resources/notify-done.html';", 0);
>+        // Simulate a user interaction with the page so that the beforeunload alert shows.
>+	UIHelper.activateAt(1, 1).then(function() {
>+		window.location.href = 'resources/notify-done.html';
>+	});
> }
> 
> </script>
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/html/browsers/browsing-the-web/unloading-documents/beforeunload-canceling-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/html/browsers/browsing-the-web/unloading-documents/beforeunload-canceling-expected.txt
>index 296af1ab41b2faabb3ad3366567f33fffbc5b46f..5076ed48a7fbe0753c199019b642c910bc040446 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/html/browsers/browsing-the-web/unloading-documents/beforeunload-canceling-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/html/browsers/browsing-the-web/unloading-documents/beforeunload-canceling-expected.txt
>@@ -1,13 +1,3 @@
>-CONFIRM NAVIGATION: 
>-CONFIRM NAVIGATION: false
>-CONFIRM NAVIGATION: true
>-CONFIRM NAVIGATION: 0
>-CONFIRM NAVIGATION: foo
>-CONFIRM NAVIGATION: foo
>-CONFIRM NAVIGATION: foo
>-CONFIRM NAVIGATION: foo
>-CONFIRM NAVIGATION: foo
>-CONFIRM NAVIGATION: foo
> 
> PASS Returning a string must not cancel the event: CustomEvent, non-cancelable 
> PASS Returning a string must not cancel the event: CustomEvent, cancelable 
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/html/webappapis/scripting/events/compile-event-handler-settings-objects-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/html/webappapis/scripting/events/compile-event-handler-settings-objects-expected.txt
>index 93dd2f9c7993502eeed7eb20714747f54cd9011b..300042be11ba67630cb53f5c80ee7f6f62f7eae2 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/html/webappapis/scripting/events/compile-event-handler-settings-objects-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/html/webappapis/scripting/events/compile-event-handler-settings-objects-expected.txt
>@@ -1,4 +1,3 @@
>-CONFIRM NAVIGATION: undefined
> 
> PASS The Function instance must be created in the Realm of the node document 
> FAIL The entry settings object while executing the compiled callback via Web IDL's invoke must be that of the node document assert_equals: expected "/html/webappapis/scripting/events/resources/open-window.html" but got "/html/webappapis/scripting/events/open-window.html"
>diff --git a/LayoutTests/platform/ios-simulator/TestExpectations b/LayoutTests/platform/ios-simulator/TestExpectations
>index 7565f8651735e1dfcd0ca2bf74e9fd3931dfa589..f7b8c9bda4489bc4f135bac69fcd0292c6d0bc20 100644
>--- a/LayoutTests/platform/ios-simulator/TestExpectations
>+++ b/LayoutTests/platform/ios-simulator/TestExpectations
>@@ -363,6 +363,7 @@ fast/text/all-small-caps-whitespace.html [ Skip ]
> # This test relies on EventSender.keydown(), which is not supported on iOS
> editing/caret/emoji.html [ Skip ]
> webkit.org/b/155233 fast/events/max-tabindex-focus.html [ Skip ]
>+fast/events/beforeunload-alert-user-interaction.html [ Skip ]
> fast/forms/validation-bubble-escape-key-dismiss.html [ Skip ]
> fast/forms/validation-message-maxLength.html [ Skip ]
> fast/shadow-dom/shadow-host-removal-crash.html [ Skip ]

Actions: View | Formatted Diff | Diff

Attachments on bug 169936: 305048 | 305057 | 305063 | 305078 | 305101 | 305108 | 305116 | 305118 | 305121