CMD+R / CMD+Q is considered as user interaction and beforeunload alert is shown
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 17 Apr 2017 00:56:47 +0000 (00:56 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 17 Apr 2017 00:56:47 +0000 (00:56 +0000)
https://bugs.webkit.org/show_bug.cgi?id=169995
<rdar://problem/23798897>

Reviewed by Sam Weinig.

Source/WebCore:

Any key event was considered as user interaction with the page, which meant that they
would allow beforeunload alerts to be shown even when they do not represent actual
user interaction (e.g CMD+R / CMD+Q / CMD+T keyboard shortcuts).

To address the issue, we now only treat as user interaction with the page key events
that are actually handled by the page (i.e. handled by JS, typed into a field, ...).

Tests: fast/events/beforeunload-alert-handled-keydown.html
       fast/events/beforeunload-alert-unhandled-keydown.html

* dom/Document.h:
(WebCore::Document::setUserDidInteractWithPage):
(WebCore::Document::userDidInteractWithPage):
* dom/UserGestureIndicator.cpp:
(WebCore::UserGestureIndicator::UserGestureIndicator):
* loader/FrameLoader.cpp:
(WebCore::shouldAskForNavigationConfirmation):
* page/EventHandler.cpp:
(WebCore::EventHandler::keyEvent):
(WebCore::EventHandler::internalKeyEvent):
* page/EventHandler.h:

LayoutTests:

Add layout test coverage.

* fast/events/beforeunload-alert-handled-keydown-expected.txt: Added.
* fast/events/beforeunload-alert-handled-keydown.html: Added.
* fast/events/beforeunload-alert-unhandled-keydown-expected.txt: Added.
* fast/events/beforeunload-alert-unhandled-keydown.html: Added.

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

12 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/events/beforeunload-alert-handled-keydown-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/beforeunload-alert-handled-keydown.html [new file with mode: 0644]
LayoutTests/fast/events/beforeunload-alert-unhandled-keydown-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/beforeunload-alert-unhandled-keydown.html [new file with mode: 0644]
LayoutTests/platform/ios/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.h
Source/WebCore/dom/UserGestureIndicator.cpp
Source/WebCore/loader/FrameLoader.cpp
Source/WebCore/page/EventHandler.cpp
Source/WebCore/page/EventHandler.h

index 3474ded..cec41fe 100644 (file)
@@ -1,3 +1,18 @@
+2017-04-16  Chris Dumez  <cdumez@apple.com>
+
+        CMD+R / CMD+Q is considered as user interaction and beforeunload alert is shown
+        https://bugs.webkit.org/show_bug.cgi?id=169995
+        <rdar://problem/23798897>
+
+        Reviewed by Sam Weinig.
+
+        Add layout test coverage.
+
+        * fast/events/beforeunload-alert-handled-keydown-expected.txt: Added.
+        * fast/events/beforeunload-alert-handled-keydown.html: Added.
+        * fast/events/beforeunload-alert-unhandled-keydown-expected.txt: Added.
+        * fast/events/beforeunload-alert-unhandled-keydown.html: Added.
+
 2017-04-16  Joseph Pecoraro  <pecoraro@apple.com>
 
         test262: test262/test/built-ins/Object/getOwnPropertyNames/15.2.3.4-4-44.js
diff --git a/LayoutTests/fast/events/beforeunload-alert-handled-keydown-expected.txt b/LayoutTests/fast/events/beforeunload-alert-handled-keydown-expected.txt
new file mode 100644 (file)
index 0000000..a76f91e
--- /dev/null
@@ -0,0 +1,12 @@
+CONFIRM NAVIGATION: PASS: a beforeunload alert was shown.
+Tests that the beforeunload alert is shown when the user has typed a character into a field. This test passes if you see a 'CONFIRM NAVIGATION' message at the top.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Simulate user typing letter 'a' into the field.
+PASS testInput.value is "a"
+PASS successfullyParsed is true
+
+TEST COMPLETE
diff --git a/LayoutTests/fast/events/beforeunload-alert-handled-keydown.html b/LayoutTests/fast/events/beforeunload-alert-handled-keydown.html
new file mode 100644 (file)
index 0000000..8c8fceb
--- /dev/null
@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/js-test-pre.js"></script>
+<script>
+description("Tests that the beforeunload alert is shown when the user has typed a character into a field. This test passes if you 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.";
+    };
+
+    debug("Simulate user typing letter 'a' into the field.");
+    testInput = document.getElementById("testInput");
+    testInput.focus();
+    if (window.eventSender)
+        eventSender.keyDown("a");
+
+    setTimeout(function() {
+        shouldBeEqualToString("testInput.value", "a");
+        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-unhandled-keydown-expected.txt b/LayoutTests/fast/events/beforeunload-alert-unhandled-keydown-expected.txt
new file mode 100644 (file)
index 0000000..630d6d3
--- /dev/null
@@ -0,0 +1,10 @@
+Tests that the beforeunload alert is not shown in case of unhandled keypress. This test passes if you do NOT see a 'CONFIRM NAVIGATION' message at the top.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Simulate an unhandled user key press.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/events/beforeunload-alert-unhandled-keydown.html b/LayoutTests/fast/events/beforeunload-alert-unhandled-keydown.html
new file mode 100644 (file)
index 0000000..b6bb67e
--- /dev/null
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/js-test-pre.js"></script>
+<script>
+description("Tests that the beforeunload alert is not shown in case of unhandled keypress. This test passes if you do 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.";
+    };
+
+    debug("Simulate an unhandled user key press.");
+    if (window.eventSender)
+        eventSender.keyDown("a", ["metaKey"]);
+
+    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>
index 4c395f5..3260594 100644 (file)
@@ -365,6 +365,8 @@ fast/text/all-small-caps-whitespace.html [ Skip ]
 
 # This test relies on EventSender.keydown(), which is not supported on iOS
 webkit.org/b/155233 fast/events/max-tabindex-focus.html [ Skip ]
+fast/events/beforeunload-alert-handled-keydown.html [ Skip ]
+fast/events/beforeunload-alert-unhandled-keydown.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 ]
index e887d44..53e711f 100644 (file)
@@ -1,3 +1,33 @@
+2017-04-16  Chris Dumez  <cdumez@apple.com>
+
+        CMD+R / CMD+Q is considered as user interaction and beforeunload alert is shown
+        https://bugs.webkit.org/show_bug.cgi?id=169995
+        <rdar://problem/23798897>
+
+        Reviewed by Sam Weinig.
+
+        Any key event was considered as user interaction with the page, which meant that they
+        would allow beforeunload alerts to be shown even when they do not represent actual
+        user interaction (e.g CMD+R / CMD+Q / CMD+T keyboard shortcuts).
+
+        To address the issue, we now only treat as user interaction with the page key events
+        that are actually handled by the page (i.e. handled by JS, typed into a field, ...).
+
+        Tests: fast/events/beforeunload-alert-handled-keydown.html
+               fast/events/beforeunload-alert-unhandled-keydown.html
+
+        * dom/Document.h:
+        (WebCore::Document::setUserDidInteractWithPage):
+        (WebCore::Document::userDidInteractWithPage):
+        * dom/UserGestureIndicator.cpp:
+        (WebCore::UserGestureIndicator::UserGestureIndicator):
+        * loader/FrameLoader.cpp:
+        (WebCore::shouldAskForNavigationConfirmation):
+        * page/EventHandler.cpp:
+        (WebCore::EventHandler::keyEvent):
+        (WebCore::EventHandler::internalKeyEvent):
+        * page/EventHandler.h:
+
 2017-04-16  Sam Weinig  <sam@webkit.org>
 
         [WebIDL] Switch IDLAttributes.txt over to a more structured format so that more information can be added for each attribute
index ebb4d6b..1d2ef8f 100644 (file)
@@ -1147,6 +1147,9 @@ public:
     bool hasHadUserInteraction() const { return static_cast<bool>(m_lastHandledUserGestureTimestamp); }
     void updateLastHandledUserGestureTimestamp(MonotonicTime);
 
+    void setUserDidInteractWithPage(bool userDidInteractWithPage) { ASSERT(&topDocument() == this); m_userDidInteractWithPage = userDidInteractWithPage; }
+    bool userDidInteractWithPage() const { ASSERT(&topDocument() == this); return m_userDidInteractWithPage; }
+
     // Used for testing. Count handlers in the main document, and one per frame which contains handlers.
     WEBCORE_EXPORT unsigned wheelEventHandlerCount() const;
     WEBCORE_EXPORT unsigned touchEventHandlerCount() const;
@@ -1734,6 +1737,7 @@ private:
     bool m_visualUpdatesAllowed { true };
 
     bool m_areDeviceMotionAndOrientationUpdatesSuspended { false };
+    bool m_userDidInteractWithPage { false };
 
 #if ENABLE(TELEPHONE_NUMBER_DETECTION)
     bool m_isTelephoneNumberParsingAllowed { true };
index 0ac50fc..84c411d 100644 (file)
@@ -58,6 +58,7 @@ UserGestureIndicator::UserGestureIndicator(std::optional<ProcessingUserGestureSt
     if (document && currentToken()->processingUserGesture()) {
         document->updateLastHandledUserGestureTimestamp(MonotonicTime::now());
         ResourceLoadObserver::sharedObserver().logUserInteractionWithReducedTimeResolution(*document);
+        document->topDocument().setUserDidInteractWithPage(true);
     }
 }
 
index 0338c9e..9316d1f 100644 (file)
@@ -3036,7 +3036,7 @@ void FrameLoader::dispatchUnloadEvents(UnloadEventPolicy unloadEventPolicy)
 
 static bool shouldAskForNavigationConfirmation(Document& document, const BeforeUnloadEvent& event)
 {
-    bool userDidInteractWithPage = document.topDocument().hasHadUserInteraction();
+    bool userDidInteractWithPage = document.topDocument().userDidInteractWithPage();
     // 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.
index 845f3ae..6957c87 100644 (file)
@@ -3093,7 +3093,20 @@ bool EventHandler::isKeyEventAllowedInFullScreen(const PlatformKeyboardEvent& ke
 }
 #endif
 
-bool EventHandler::keyEvent(const PlatformKeyboardEvent& initialKeyEvent)
+bool EventHandler::keyEvent(const PlatformKeyboardEvent& keyEvent)
+{
+    Document* topDocument = m_frame.document() ? &m_frame.document()->topDocument() : nullptr;
+    bool savedUserDidInteractWithPage = topDocument ? topDocument->userDidInteractWithPage() : false;
+    bool wasHandled = internalKeyEvent(keyEvent);
+
+    // If the key event was not handled, do not treat it as user interaction with the page.
+    if (topDocument && !wasHandled)
+        topDocument->setUserDidInteractWithPage(savedUserDidInteractWithPage);
+
+    return wasHandled;
+}
+
+bool EventHandler::internalKeyEvent(const PlatformKeyboardEvent& initialKeyEvent)
 {
     Ref<Frame> protectedFrame(m_frame);
     RefPtr<FrameView> protector(m_frame.view());
index deb09c4..1ec5eed 100644 (file)
@@ -347,6 +347,8 @@ private:
 
     WEBCORE_EXPORT bool handleMouseReleaseEvent(const MouseEventWithHitTestResults&);
 
+    bool internalKeyEvent(const PlatformKeyboardEvent&);
+
     std::optional<Cursor> selectCursor(const HitTestResult&, bool shiftKey);
     void updateCursor(FrameView&, const HitTestResult&, bool shiftKey);