- Revision
- 216360
- Author
- carlo...@webkit.org
- Date
- 2017-05-08 01:02:08 -0700 (Mon, 08 May 2017)
Log Message
Merge r215404 - 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.
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.
Modified Paths
Added Paths
Diff
Modified: releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog (216359 => 216360)
--- releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog 2017-05-08 07:56:02 UTC (rev 216359)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog 2017-05-08 08:02:08 UTC (rev 216360)
@@ -1,3 +1,18 @@
+2017-04-16 Chris Dumez <cdu...@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-14 Zalan Bujtas <za...@apple.com>
text-align start / end failure in table cells
Added: releases/WebKitGTK/webkit-2.16/LayoutTests/fast/events/beforeunload-alert-handled-keydown-expected.txt (0 => 216360)
--- releases/WebKitGTK/webkit-2.16/LayoutTests/fast/events/beforeunload-alert-handled-keydown-expected.txt (rev 0)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/fast/events/beforeunload-alert-handled-keydown-expected.txt 2017-05-08 08:02:08 UTC (rev 216360)
@@ -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
+
Added: releases/WebKitGTK/webkit-2.16/LayoutTests/fast/events/beforeunload-alert-handled-keydown.html (0 => 216360)
--- releases/WebKitGTK/webkit-2.16/LayoutTests/fast/events/beforeunload-alert-handled-keydown.html (rev 0)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/fast/events/beforeunload-alert-handled-keydown.html 2017-05-08 08:02:08 UTC (rev 216360)
@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<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 = ""
+ setTimeout(finishJSTest, 0);
+ }, 0);
+};
+</script>
+<iframe id="testFrame" src=""
+<input id="testInput" type="text"></input>
+<script src=""
+</body>
+</html>
Added: releases/WebKitGTK/webkit-2.16/LayoutTests/fast/events/beforeunload-alert-unhandled-keydown-expected.txt (0 => 216360)
--- releases/WebKitGTK/webkit-2.16/LayoutTests/fast/events/beforeunload-alert-unhandled-keydown-expected.txt (rev 0)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/fast/events/beforeunload-alert-unhandled-keydown-expected.txt 2017-05-08 08:02:08 UTC (rev 216360)
@@ -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
+
Added: releases/WebKitGTK/webkit-2.16/LayoutTests/fast/events/beforeunload-alert-unhandled-keydown.html (0 => 216360)
--- releases/WebKitGTK/webkit-2.16/LayoutTests/fast/events/beforeunload-alert-unhandled-keydown.html (rev 0)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/fast/events/beforeunload-alert-unhandled-keydown.html 2017-05-08 08:02:08 UTC (rev 216360)
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<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 = ""
+ setTimeout(finishJSTest, 0);
+ }, 0);
+};
+</script>
+<iframe id="testFrame" src=""
+<script src=""
+</body>
+</html>
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog (216359 => 216360)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog 2017-05-08 07:56:02 UTC (rev 216359)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog 2017-05-08 08:02:08 UTC (rev 216360)
@@ -1,3 +1,33 @@
+2017-04-16 Chris Dumez <cdu...@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-05 Miguel Gomez <mago...@igalia.com>
[GTK+] PNG animations that should run once are not played at all
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/Document.h (216359 => 216360)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/Document.h 2017-05-08 07:56:02 UTC (rev 216359)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/Document.h 2017-05-08 08:02:08 UTC (rev 216360)
@@ -1154,6 +1154,9 @@
bool hasTouchEventHandlers() const { return false; }
#endif
+ 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;
@@ -1735,6 +1738,7 @@
RefPtr<MediaSession> m_defaultMediaSession;
#endif
bool m_areDeviceMotionAndOrientationUpdatesSuspended { false };
+ bool m_userDidInteractWithPage { false };
#if ENABLE(MEDIA_STREAM)
bool m_hasHadActiveMediaStreamTrack { false };
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/UserGestureIndicator.cpp (216359 => 216360)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/UserGestureIndicator.cpp 2017-05-08 07:56:02 UTC (rev 216359)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/UserGestureIndicator.cpp 2017-05-08 08:02:08 UTC (rev 216360)
@@ -54,8 +54,10 @@
if (state)
currentToken() = UserGestureToken::create(state.value());
- if (document && currentToken()->processingUserGesture())
+ if (document && currentToken()->processingUserGesture()) {
document->topDocument().updateLastHandledUserGestureTimestamp();
+ document->topDocument().setUserDidInteractWithPage(true);
+ }
}
UserGestureIndicator::UserGestureIndicator(RefPtr<UserGestureToken> token)
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/FrameLoader.cpp (216359 => 216360)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/FrameLoader.cpp 2017-05-08 07:56:02 UTC (rev 216359)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/FrameLoader.cpp 2017-05-08 08:02:08 UTC (rev 216360)
@@ -2988,7 +2988,7 @@
static bool shouldAskForNavigationConfirmation(Document& document, const BeforeUnloadEvent& event)
{
- bool userDidInteractWithPage = document.topDocument().lastHandledUserGestureTimestamp() > 0;
+ 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.
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/page/EventHandler.cpp (216359 => 216360)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/page/EventHandler.cpp 2017-05-08 07:56:02 UTC (rev 216359)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/page/EventHandler.cpp 2017-05-08 08:02:08 UTC (rev 216360)
@@ -3093,8 +3093,21 @@
}
#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());
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/page/EventHandler.h (216359 => 216360)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/page/EventHandler.h 2017-05-08 07:56:02 UTC (rev 216359)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/page/EventHandler.h 2017-05-08 08:02:08 UTC (rev 216360)
@@ -347,6 +347,8 @@
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);
