Attachment 307060 Details for Bug 169995 – Patch

[patch] Patch

bug-169995-20170413180329.patch (text/plain), 12.18 KB, created by Chris Dumez on 2017-04-13 18:03:30 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2017-04-13 18:03:30 PDT

Size: 12.18 KB

patch

obsolete

>Subversion Revision: 215309
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 4a7de121242a1eb0a968b4e9c2435585367c254c..b49e19790a530c4f47f26e96d2472d4f83052567 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,33 @@
>+2017-04-13  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 NOBODY (OOPS!).
>+
>+        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-12  Eric Carlson  <eric.carlson@apple.com>
> 
>         REGRESSION (r215242-215243): [ios-simulator] API test WebKit1.AudioSessionCategoryIOS is failing
>diff --git a/Source/WebCore/dom/Document.h b/Source/WebCore/dom/Document.h
>index ebb4d6b1d12758c6487c4f3b116c57c1cf1ca500..1d2ef8f6213e9d86f9b7c88265918da10d2beb9e 100644
>--- a/Source/WebCore/dom/Document.h
>+++ b/Source/WebCore/dom/Document.h
>@@ -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 };
>diff --git a/Source/WebCore/dom/UserGestureIndicator.cpp b/Source/WebCore/dom/UserGestureIndicator.cpp
>index 0ac50fcbafdb408064d7c414d08b679a93117a47..84c411d76d35562490ea0f79d0cae83f95cb2cb9 100644
>--- a/Source/WebCore/dom/UserGestureIndicator.cpp
>+++ b/Source/WebCore/dom/UserGestureIndicator.cpp
>@@ -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);
>     }
> }
> 
>diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp
>index 0338c9e48c37ec1a5d09bd9f67f596afa34610c9..9316d1fd2d05e12d679ba00840979768962d9629 100644
>--- a/Source/WebCore/loader/FrameLoader.cpp
>+++ b/Source/WebCore/loader/FrameLoader.cpp
>@@ -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.
>diff --git a/Source/WebCore/page/EventHandler.cpp b/Source/WebCore/page/EventHandler.cpp
>index 845f3aebef547e678f1d0c79d9e593fa0bc5d7fc..6957c8784fc5bc51d92b54fa264ecab8a5072a9d 100644
>--- a/Source/WebCore/page/EventHandler.cpp
>+++ b/Source/WebCore/page/EventHandler.cpp
>@@ -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());
>diff --git a/Source/WebCore/page/EventHandler.h b/Source/WebCore/page/EventHandler.h
>index deb09c4984b0e4760c7fc881a5eb95c5bfcd991c..1ec5eed7a708d85c21d3ee4f8aff2702c7e11dd3 100644
>--- a/Source/WebCore/page/EventHandler.h
>+++ b/Source/WebCore/page/EventHandler.h
>@@ -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);
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 8f941d1b01721953e91f25bee3c3e9f1df5250b3..70f0d4752621d8a2ed5199d8b7985ccc35d000e5 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,18 @@
>+2017-04-13  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 NOBODY (OOPS!).
>+
>+        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-12  Jonathan Bedard  <jbedard@apple.com>
> 
>         Move expectation for compositing/iframes/iframe-size-from-zero.htm to top level
>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
>index 0000000000000000000000000000000000000000..a76f91e77ab1247671d292e344ade6b8d2fd1b62
>--- /dev/null
>+++ b/LayoutTests/fast/events/beforeunload-alert-handled-keydown-expected.txt
>@@ -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
>index 0000000000000000000000000000000000000000..8c8fceb188e0064b29a5d47e2386e5ed576d9cca
>--- /dev/null
>+++ b/LayoutTests/fast/events/beforeunload-alert-handled-keydown.html
>@@ -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
>index 0000000000000000000000000000000000000000..630d6d33a771e513d23a12f9c0df25c31f4685bd
>--- /dev/null
>+++ b/LayoutTests/fast/events/beforeunload-alert-unhandled-keydown-expected.txt
>@@ -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
>index 0000000000000000000000000000000000000000..b6bb67e1d9cffd702631e25fd8e95b4ffb92a0d3
>--- /dev/null
>+++ b/LayoutTests/fast/events/beforeunload-alert-unhandled-keydown.html
>@@ -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>
>diff --git a/LayoutTests/platform/ios/TestExpectations b/LayoutTests/platform/ios/TestExpectations
>index 34b4caf0d21d1b781f0aacfc722980287c514da6..562c53d3a491a83a11a4816f314539002b7ebcdd 100644
>--- a/LayoutTests/platform/ios/TestExpectations
>+++ b/LayoutTests/platform/ios/TestExpectations
>@@ -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 ]

Actions: View | Formatted Diff | Diff

Attachments on bug 169995: 305166 | 305167 | 305170 | 305171 | 305172 | 305197 | 306093 | 306961 | 306995 | 307037 | 307057 | 307060