Attachment 305166 Details for Bug 169995 – Patch

[patch] Patch

bug-169995-20170322222906.patch (text/plain), 25.80 KB, created by Chris Dumez on 2017-03-22 22:29:07 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2017-03-22 22:29:07 PDT

Size: 25.80 KB

patch

obsolete

>Subversion Revision: 214277
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index a25728478373b70c42bf437fba646039468f6a9f..b203dd0a1ba44d5715cd3970bde5bb0e0616c84d 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,5 +1,35 @@
> 2017-03-22  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!).
>+
>+        CMD+R / CMD+Q / CMD+T keyboard shortcuts were considered as user interaction with the page,
>+        causing the beforeunload alert to be shown even though there was no real user interaction.
>+        This behavior seemed wrong and was inconsistent with Firefox.
>+
>+        To address the issue, we now delay setting the 'userDidInteractWithThePage' flag
>+        until a keyUp event is received, instead of setting it as soon as we get a keyUp
>+        event. This seems to be what Firefox is doing, based on local testing on:
>+        - https://www.researchgate.net/signup.SignUp.html
>+
>+        Test: fast/events/beforeunload-alert-user-interaction-keydown.html
>+
>+        * dom/Document.h:
>+        (WebCore::Document::markUserDidInteractWithPage):
>+        (WebCore::Document::userDidInteractWithPage):
>+        * dom/UserGestureIndicator.cpp:
>+        (WebCore::UserGestureIndicator::UserGestureIndicator):
>+        * dom/UserGestureIndicator.h:
>+        * loader/FrameLoader.cpp:
>+        (WebCore::shouldAskForNavigationConfirmation):
>+        * page/EventHandler.cpp:
>+        (WebCore::EventHandler::keyEvent):
>+
>+2017-03-22  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>
>diff --git a/Source/WebCore/dom/Document.h b/Source/WebCore/dom/Document.h
>index 85b6ca38cfc35c16e4f533b9a19f66c84dc87b1e..014e8a8e463a2c2b0015e14543f4f9e6a98d9629 100644
>--- a/Source/WebCore/dom/Document.h
>+++ b/Source/WebCore/dom/Document.h
>@@ -1143,6 +1143,9 @@ public:
>     double lastHandledUserGestureTimestamp() const { return m_lastHandledUserGestureTimestamp; }
>     void updateLastHandledUserGestureTimestamp();
> 
>+    void markUserDidInteractWithPage() { m_userDidInteractWithPage = true; }
>+    bool userDidInteractWithPage() const { return m_userDidInteractWithPage; }
>+
> #if ENABLE(TOUCH_EVENTS)
>     bool hasTouchEventHandlers() const { return (m_touchEventTargets.get()) ? m_touchEventTargets->size() : false; }
> #else
>@@ -1727,6 +1730,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 ece68f1d58bf798415dec512e3109b8cd2380ed8..c089ac8cb772437844f9845544f1cab8b9212982 100644
>--- a/Source/WebCore/dom/UserGestureIndicator.cpp
>+++ b/Source/WebCore/dom/UserGestureIndicator.cpp
>@@ -44,7 +44,7 @@ UserGestureToken::~UserGestureToken()
>         observer(*this);
> }
> 
>-UserGestureIndicator::UserGestureIndicator(std::optional<ProcessingUserGestureState> state, Document* document)
>+UserGestureIndicator::UserGestureIndicator(std::optional<ProcessingUserGestureState> state, Document* document, IgnoreInteractionWithPage ignoreInteractionWithPage)
>     : m_previousToken(currentToken())
> {
>     // Silently ignore UserGestureIndicators on non main threads.
>@@ -54,8 +54,11 @@ UserGestureIndicator::UserGestureIndicator(std::optional<ProcessingUserGestureSt
>     if (state)
>         currentToken() = UserGestureToken::create(state.value());
> 
>-    if (document && currentToken()->processingUserGesture())
>+    if (document && currentToken()->processingUserGesture()) {
>         document->topDocument().updateLastHandledUserGestureTimestamp();
>+        if (ignoreInteractionWithPage == IgnoreInteractionWithPage::No)
>+            document->topDocument().markUserDidInteractWithPage();
>+    }
> }
> 
> UserGestureIndicator::UserGestureIndicator(RefPtr<UserGestureToken> token)
>diff --git a/Source/WebCore/dom/UserGestureIndicator.h b/Source/WebCore/dom/UserGestureIndicator.h
>index f044f65aa16d1ea6d36a9bbed5730235b8417b1e..52bb52f3a20a291d8359cf461fb494f121538678 100644
>--- a/Source/WebCore/dom/UserGestureIndicator.h
>+++ b/Source/WebCore/dom/UserGestureIndicator.h
>@@ -42,6 +42,8 @@ enum ProcessingUserGestureState {
>     NotProcessingUserGesture
> };
> 
>+enum class IgnoreInteractionWithPage { No, Yes };
>+
> class UserGestureToken : public RefCounted<UserGestureToken> {
> public:
>     static RefPtr<UserGestureToken> create(ProcessingUserGestureState state)
>@@ -79,7 +81,7 @@ public:
>     WEBCORE_EXPORT static bool processingUserGestureForMedia();
> 
>     // If a document is provided, its last known user gesture timestamp is updated.
>-    WEBCORE_EXPORT explicit UserGestureIndicator(std::optional<ProcessingUserGestureState>, Document* = nullptr);
>+    WEBCORE_EXPORT explicit UserGestureIndicator(std::optional<ProcessingUserGestureState>, Document* = nullptr, IgnoreInteractionWithPage = IgnoreInteractionWithPage::No);
>     WEBCORE_EXPORT explicit UserGestureIndicator(RefPtr<UserGestureToken>);
>     WEBCORE_EXPORT ~UserGestureIndicator();
> 
>diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp
>index ec7785a825fccf084a87c39c1e085694bf32e17e..de46a963758e8c53f96c5458391915fb2b18d969 100644
>--- a/Source/WebCore/loader/FrameLoader.cpp
>+++ b/Source/WebCore/loader/FrameLoader.cpp
>@@ -3070,13 +3070,12 @@ void FrameLoader::dispatchUnloadEvents(UnloadEventPolicy unloadEventPolicy)
> 
> 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 userDidInteractWithPage && (event.defaultPrevented() || !event.returnValue().isEmpty());
>+    return document.topDocument().userDidInteractWithPage() && (event.defaultPrevented() || !event.returnValue().isEmpty());
> }
> 
> bool FrameLoader::dispatchBeforeUnloadEvent(Chrome& chrome, FrameLoader* frameLoaderBeingNavigated)
>diff --git a/Source/WebCore/page/EventHandler.cpp b/Source/WebCore/page/EventHandler.cpp
>index a01f6d3861164587ea3916315a5579ac4503d8ac..960ce5425858771999e66d00c9f59bc292e01444 100644
>--- a/Source/WebCore/page/EventHandler.cpp
>+++ b/Source/WebCore/page/EventHandler.cpp
>@@ -3149,7 +3149,10 @@ bool EventHandler::keyEvent(const PlatformKeyboardEvent& initialKeyEvent)
>     if (!element)
>         return false;
> 
>-    UserGestureIndicator gestureIndicator(ProcessingUserGesture, m_frame.document());
>+    // Delay updating document flag indicating the user has interacted with the page until the KeyUp so we do not
>+    // treat keyboard shortcuts such as CMD+R / CMD+Q as user interaction with the page.
>+    auto ignoreInteractionWithPage = initialKeyEvent.type() != PlatformEvent::KeyUp ? IgnoreInteractionWithPage::Yes : IgnoreInteractionWithPage::No;
>+    UserGestureIndicator gestureIndicator(ProcessingUserGesture, m_frame.document(), ignoreInteractionWithPage);
>     UserTypingGestureIndicator typingGestureIndicator(m_frame);
> 
>     if (FrameView* view = m_frame.view())
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index f7518d05c882d134b23b935748281d89e8484c51..2ad45ea46d8641a98eeb0634e74be0709c0519a6 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,35 @@
>+2017-03-22  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!).
>+
>+        eventSender.keyDown() previously send both keyDown and keyUp events. We now
>+        take in a boolean to indicate if a keyUp should be sent.
>+
>+        * DumpRenderTree/mac/EventSendingController.mm:
>+        (+[EventSendingController isSelectorExcludedFromWebScript:]):
>+        (+[EventSendingController webScriptNameForSelector:]):
>+        (-[EventSendingController keyDown:withModifiers:withLocation:withShouldSendKeyUp:]):
>+        (-[EventSendingController keyDownWrapper:withModifiers:withLocation:]):
>+        * WebKitTestRunner/EventSenderProxy.h:
>+        * WebKitTestRunner/InjectedBundle/Bindings/EventSendingController.idl:
>+        * WebKitTestRunner/InjectedBundle/EventSendingController.cpp:
>+        (WTR::createKeyDownMessageBody):
>+        (WTR::EventSendingController::keyDown):
>+        (WTR::EventSendingController::scheduleAsynchronousKeyDown):
>+        * WebKitTestRunner/InjectedBundle/EventSendingController.h:
>+        * WebKitTestRunner/InjectedBundle/ios/EventSenderProxyIOS.mm:
>+        (WTR::EventSenderProxy::keyDown):
>+        * WebKitTestRunner/TestController.cpp:
>+        (WTR::TestController::didReceiveKeyDownMessageFromInjectedBundle):
>+        * WebKitTestRunner/gtk/EventSenderProxyGtk.cpp:
>+        (WTR::EventSenderProxy::keyDown):
>+        * WebKitTestRunner/mac/EventSenderProxy.mm:
>+        (WTR::EventSenderProxy::keyDown):
>+
> 2017-03-22  Carlos Alberto Lopez Perez  <clopez@igalia.com>
> 
>         [Linux] determineArchitecture is not cross-compile aware
>diff --git a/Tools/DumpRenderTree/mac/EventSendingController.mm b/Tools/DumpRenderTree/mac/EventSendingController.mm
>index a55097035fb53e6b764a0917645a8a5cf92f58d1..87c8ff6bf6400355b7c016642e7ff061aa237580 100644
>--- a/Tools/DumpRenderTree/mac/EventSendingController.mm
>+++ b/Tools/DumpRenderTree/mac/EventSendingController.mm
>@@ -236,7 +236,7 @@ + (BOOL)isSelectorExcludedFromWebScript:(SEL)aSelector
>             || aSelector == @selector(contextClick)
>             || aSelector == @selector(enableDOMUIEventLogging:)
>             || aSelector == @selector(fireKeyboardEventsToElement:)
>-            || aSelector == @selector(keyDown:withModifiers:withLocation:)
>+            || aSelector == @selector(keyDown:withModifiers:withLocation:withShouldSendKeyUp:)
>             || aSelector == @selector(leapForward:)
>             || aSelector == @selector(mouseDown:withModifiers:)
>             || aSelector == @selector(mouseMoveToX:Y:)
>@@ -293,7 +293,7 @@ + (NSString *)webScriptNameForSelector:(SEL)aSelector
>         return @"enableDOMUIEventLogging";
>     if (aSelector == @selector(fireKeyboardEventsToElement:))
>         return @"fireKeyboardEventsToElement";
>-    if (aSelector == @selector(keyDown:withModifiers:withLocation:))
>+    if (aSelector == @selector(keyDown:withModifiers:withLocation:withShouldSendKeyUp:))
>         return @"keyDown";
>     if (aSelector == @selector(scheduleAsynchronousKeyDown:withModifiers:withLocation:))
>         return @"scheduleAsynchronousKeyDown";
>@@ -890,7 +890,7 @@ + (void)clearSavedEvents
>     savedMouseEvents = nil;
> }
> 
>-- (void)keyDown:(NSString *)character withModifiers:(WebScriptObject *)modifiers withLocation:(unsigned long)location
>+- (void)keyDown:(NSString *)character withModifiers:(WebScriptObject *)modifiers withLocation:(unsigned long)location withShouldSendKeyUp:(BOOL)shouldSendKeyUp
> {
>     NSString *eventCharacter = character;
>     unsigned short keyCode = 0;
>@@ -1105,6 +1105,9 @@ - (void)keyDown:(NSString *)character withModifiers:(WebScriptObject *)modifiers
>     [NSApp _setCurrentEvent:nil];
> #endif
> 
>+    if (!shouldSendKeyUp)
>+        return;
>+
> #if !PLATFORM(IOS)
>     event = [NSEvent keyEventWithType:NSEventTypeKeyUp
>                         location:NSMakePoint(5, 5)
>@@ -1145,7 +1148,7 @@ - (void)keyDown:(NSString *)character withModifiers:(WebScriptObject *)modifiers
> 
> - (void)keyDownWrapper:(NSString *)character withModifiers:(WebScriptObject *)modifiers withLocation:(unsigned long)location
> {
>-    [self keyDown:character withModifiers:modifiers withLocation:location];
>+    [self keyDown:character withModifiers:modifiers withLocation:location withShouldSendKeyUp:YES];
> }
> 
> - (void)scheduleAsynchronousKeyDown:(NSString *)character withModifiers:(WebScriptObject *)modifiers withLocation:(unsigned long)location
>diff --git a/Tools/WebKitTestRunner/EventSenderProxy.h b/Tools/WebKitTestRunner/EventSenderProxy.h
>index 70d94014c8d750cb7d6570804528e208cc2f4a54..5e51b6c52720aeb9856d8d57d03126ce8a4df1fc 100644
>--- a/Tools/WebKitTestRunner/EventSenderProxy.h
>+++ b/Tools/WebKitTestRunner/EventSenderProxy.h
>@@ -76,7 +76,7 @@ public:
> 
>     void leapForward(int milliseconds);
> 
>-    void keyDown(WKStringRef key, WKEventModifiers, unsigned location);
>+    void keyDown(WKStringRef key, WKEventModifiers, unsigned location, bool shouldSendKeyUp);
> 
> #if ENABLE(TOUCH_EVENTS)
>     // Touch events.
>diff --git a/Tools/WebKitTestRunner/InjectedBundle/Bindings/EventSendingController.idl b/Tools/WebKitTestRunner/InjectedBundle/Bindings/EventSendingController.idl
>index f3280bb94b054370243023e25336de1afae7939f..6beab97288def75c15bfc18e534d47e9167a0bb0 100644
>--- a/Tools/WebKitTestRunner/InjectedBundle/Bindings/EventSendingController.idl
>+++ b/Tools/WebKitTestRunner/InjectedBundle/Bindings/EventSendingController.idl
>@@ -41,7 +41,7 @@ interface EventSendingController {
> 
>     void leapForward(long milliseconds);
> 
>-    void keyDown(DOMString key, object modifierArray, long location);
>+    void keyDown(DOMString key, object modifierArray, long location, optional boolean shouldSendKeyUp = true);
>     void scheduleAsynchronousKeyDown(DOMString key);
> 
>     // Zoom functions.
>diff --git a/Tools/WebKitTestRunner/InjectedBundle/EventSendingController.cpp b/Tools/WebKitTestRunner/InjectedBundle/EventSendingController.cpp
>index f81982c6bf8d29a9b1261f3b2fcfab0b2bf0fc89..5a1e724b059cd20b0e0a8c9af70303bc1a195a3f 100644
>--- a/Tools/WebKitTestRunner/InjectedBundle/EventSendingController.cpp
>+++ b/Tools/WebKitTestRunner/InjectedBundle/EventSendingController.cpp
>@@ -353,7 +353,7 @@ void EventSendingController::scheduleAsynchronousClick()
>     WKBundlePagePostMessage(InjectedBundle::singleton().page()->page(), EventSenderMessageName.get(), mouseUpMessageBody.get());
> }
> 
>-static WKRetainPtr<WKMutableDictionaryRef> createKeyDownMessageBody(JSStringRef key, WKEventModifiers modifiers, int location)
>+static WKRetainPtr<WKMutableDictionaryRef> createKeyDownMessageBody(JSStringRef key, WKEventModifiers modifiers, int location, bool shouldSendKeyUp)
> {
>     WKRetainPtr<WKMutableDictionaryRef> EventSenderMessageBody(AdoptWK, WKMutableDictionaryCreate());
> 
>@@ -372,10 +372,14 @@ static WKRetainPtr<WKMutableDictionaryRef> createKeyDownMessageBody(JSStringRef
>     WKRetainPtr<WKUInt64Ref> locationRef(AdoptWK, WKUInt64Create(location));
>     WKDictionarySetItem(EventSenderMessageBody.get(), locationKey.get(), locationRef.get());
> 
>+    WKRetainPtr<WKStringRef> shouldSendKeyUpKey(AdoptWK, WKStringCreateWithUTF8CString("ShouldSendKeyUp"));
>+    WKRetainPtr<WKUInt64Ref> shouldSendKeyUpRef(AdoptWK, WKUInt64Create(shouldSendKeyUp));
>+    WKDictionarySetItem(EventSenderMessageBody.get(), shouldSendKeyUpKey.get(), shouldSendKeyUpRef.get());
>+
>     return EventSenderMessageBody;
> }
> 
>-void EventSendingController::keyDown(JSStringRef key, JSValueRef modifierArray, int location)
>+void EventSendingController::keyDown(JSStringRef key, JSValueRef modifierArray, int location, bool shouldSendKeyUp)
> {
>     WKBundlePageRef page = InjectedBundle::singleton().page()->page();
>     WKBundleFrameRef frame = WKBundlePageGetMainFrame(page);
>@@ -383,7 +387,7 @@ void EventSendingController::keyDown(JSStringRef key, JSValueRef modifierArray,
>     WKEventModifiers modifiers = parseModifierArray(context, modifierArray);
> 
>     WKRetainPtr<WKStringRef> EventSenderMessageName(AdoptWK, WKStringCreateWithUTF8CString("EventSender"));
>-    WKRetainPtr<WKMutableDictionaryRef> keyDownMessageBody = createKeyDownMessageBody(key, modifiers, location);
>+    WKRetainPtr<WKMutableDictionaryRef> keyDownMessageBody = createKeyDownMessageBody(key, modifiers, location, shouldSendKeyUp);
> 
>     WKBundlePagePostSynchronousMessageForTesting(InjectedBundle::singleton().page()->page(), EventSenderMessageName.get(), keyDownMessageBody.get(), 0);
> }
>@@ -391,7 +395,7 @@ void EventSendingController::keyDown(JSStringRef key, JSValueRef modifierArray,
> void EventSendingController::scheduleAsynchronousKeyDown(JSStringRef key)
> {
>     WKRetainPtr<WKStringRef> EventSenderMessageName(AdoptWK, WKStringCreateWithUTF8CString("EventSender"));
>-    WKRetainPtr<WKMutableDictionaryRef> keyDownMessageBody = createKeyDownMessageBody(key, 0 /* modifiers */, 0 /* location */);
>+    WKRetainPtr<WKMutableDictionaryRef> keyDownMessageBody = createKeyDownMessageBody(key, 0 /* modifiers */, 0 /* location */, true /* shouldSendKeyUp */);
> 
>     WKBundlePagePostMessage(InjectedBundle::singleton().page()->page(), EventSenderMessageName.get(), keyDownMessageBody.get());
> }
>diff --git a/Tools/WebKitTestRunner/InjectedBundle/EventSendingController.h b/Tools/WebKitTestRunner/InjectedBundle/EventSendingController.h
>index 3eb8129c7a38e3609d6685e5113ea57a7bfafeaa..fe74e0c12eb963685176c222b8fc87b0e4c5d0e4 100644
>--- a/Tools/WebKitTestRunner/InjectedBundle/EventSendingController.h
>+++ b/Tools/WebKitTestRunner/InjectedBundle/EventSendingController.h
>@@ -60,7 +60,7 @@ public:
>     void monitorWheelEvents();
>     void callAfterScrollingCompletes(JSValueRef functionCallback);
> 
>-    void keyDown(JSStringRef key, JSValueRef modifierArray, int location);
>+    void keyDown(JSStringRef key, JSValueRef modifierArray, int location, bool shouldSendKeyUp);
>     void scheduleAsynchronousKeyDown(JSStringRef key);
> 
>     // Zoom functions.
>diff --git a/Tools/WebKitTestRunner/InjectedBundle/ios/EventSenderProxyIOS.mm b/Tools/WebKitTestRunner/InjectedBundle/ios/EventSenderProxyIOS.mm
>index ac54a1c6d6d8c2c50e081db5bdb821d3e608ec2d..a602504e71e832eb51f4cecbea153d552ebdc6ac 100644
>--- a/Tools/WebKitTestRunner/InjectedBundle/ios/EventSenderProxyIOS.mm
>+++ b/Tools/WebKitTestRunner/InjectedBundle/ios/EventSenderProxyIOS.mm
>@@ -88,7 +88,7 @@ void EventSenderProxy::leapForward(int milliseconds)
>     m_time += milliseconds / 1000.0;
> }
> 
>-void EventSenderProxy::keyDown(WKStringRef key, WKEventModifiers modifiers, unsigned keyLocation)
>+void EventSenderProxy::keyDown(WKStringRef key, WKEventModifiers modifiers, unsigned keyLocation, bool shouldSendKeyUp)
> {
>     // Write me.
> }
>diff --git a/Tools/WebKitTestRunner/TestController.cpp b/Tools/WebKitTestRunner/TestController.cpp
>index 1eca1d284bde873010801d5379f09c8af9a93a06..30f4ff3f6e0ff2a29f0ee529d021c19e1b9363a7 100644
>--- a/Tools/WebKitTestRunner/TestController.cpp
>+++ b/Tools/WebKitTestRunner/TestController.cpp
>@@ -1243,7 +1243,10 @@ void TestController::didReceiveKeyDownMessageFromInjectedBundle(WKDictionaryRef
>     WKRetainPtr<WKStringRef> locationKey = adoptWK(WKStringCreateWithUTF8CString("Location"));
>     unsigned location = static_cast<unsigned>(WKUInt64GetValue(static_cast<WKUInt64Ref>(WKDictionaryGetItemForKey(messageBodyDictionary, locationKey.get()))));
> 
>-    m_eventSenderProxy->keyDown(key, modifiers, location);
>+    WKRetainPtr<WKStringRef> shouldSendKeyUpKey = adoptWK(WKStringCreateWithUTF8CString("ShouldSendKeyUp"));
>+    bool shouldSendKeyUp = static_cast<bool>(WKUInt64GetValue(static_cast<WKUInt64Ref>(WKDictionaryGetItemForKey(messageBodyDictionary, shouldSendKeyUpKey.get()))));
>+
>+    m_eventSenderProxy->keyDown(key, modifiers, location, shouldSendKeyUp);
> }
> 
> void TestController::didReceiveMessageFromInjectedBundle(WKStringRef messageName, WKTypeRef messageBody)
>diff --git a/Tools/WebKitTestRunner/gtk/EventSenderProxyGtk.cpp b/Tools/WebKitTestRunner/gtk/EventSenderProxyGtk.cpp
>index 3b1b41b0ce04369626db88706dea5057d45c263b..192b42334b8c616d1744f4fa4632cf05a24ddcf0 100644
>--- a/Tools/WebKitTestRunner/gtk/EventSenderProxyGtk.cpp
>+++ b/Tools/WebKitTestRunner/gtk/EventSenderProxyGtk.cpp
>@@ -309,7 +309,7 @@ int getGDKKeySymForKeyRef(WKStringRef keyRef, unsigned location, guint* modifier
>     return gdk_unicode_to_keyval(static_cast<guint32>(buffer.get()[0]));
> }
> 
>-void EventSenderProxy::keyDown(WKStringRef keyRef, WKEventModifiers wkModifiers, unsigned location)
>+void EventSenderProxy::keyDown(WKStringRef keyRef, WKEventModifiers wkModifiers, unsigned location, bool shouldSendKeyUp)
> {
>     guint modifiers = webkitModifiersToGDKModifiers(wkModifiers);
>     int gdkKeySym = getGDKKeySymForKeyRef(keyRef, location, &modifiers);
>@@ -328,6 +328,10 @@ void EventSenderProxy::keyDown(WKStringRef keyRef, WKEventModifiers wkModifiers,
> 
>     GdkEvent* releaseEvent = gdk_event_copy(pressEvent);
>     dispatchEvent(pressEvent);
>+
>+    if (!shouldSendKeyUp)
>+        return;
>+
>     releaseEvent->key.type = GDK_KEY_RELEASE;
>     dispatchEvent(releaseEvent);
> }
>diff --git a/Tools/WebKitTestRunner/mac/EventSenderProxy.mm b/Tools/WebKitTestRunner/mac/EventSenderProxy.mm
>index ee4eb0c1551dc52acb07782cef892234698ef48a..9612a1d45f4675148186b15e9e52e22ae746d5b7 100644
>--- a/Tools/WebKitTestRunner/mac/EventSenderProxy.mm
>+++ b/Tools/WebKitTestRunner/mac/EventSenderProxy.mm
>@@ -638,7 +638,7 @@ void EventSenderProxy::leapForward(int milliseconds)
>     m_time += milliseconds / 1000.0;
> }
> 
>-void EventSenderProxy::keyDown(WKStringRef key, WKEventModifiers modifiers, unsigned keyLocation)
>+void EventSenderProxy::keyDown(WKStringRef key, WKEventModifiers modifiers, unsigned keyLocation, bool shouldSendKeyUp)
> {
>     NSString* character = [NSString stringWithCString:toSTD(key).c_str()
>                                    encoding:[NSString defaultCStringEncoding]];
>@@ -837,6 +837,9 @@ void EventSenderProxy::keyDown(WKStringRef key, WKEventModifiers modifiers, unsi
>     [[m_testController->mainWebView()->platformWindow() firstResponder] keyDown:event];
>     [NSApp _setCurrentEvent:nil];
> 
>+    if (!shouldSendKeyUp)
>+        return;
>+
>     event = [NSEvent keyEventWithType:NSEventTypeKeyUp
>                         location:NSMakePoint(5, 5)
>                         modifierFlags:modifierFlags
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index d80daf02594090e4375ac9d168415a4d72662932..7e96c98f5983c487f6fb64ca2fa26052605b37c3 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,5 +1,19 @@
> 2017-03-22  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-user-interaction-keydown-expected.txt: Added.
>+        * fast/events/beforeunload-alert-user-interaction-keydown.html: Added.
>+        * platform/ios-simulator/TestExpectations:
>+
>+2017-03-22  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>
>diff --git a/LayoutTests/fast/events/beforeunload-alert-user-interaction-keydown-expected.txt b/LayoutTests/fast/events/beforeunload-alert-user-interaction-keydown-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..80e1fe206e01e5d674f88e0dbf15e43c15526e82
>--- /dev/null
>+++ b/LayoutTests/fast/events/beforeunload-alert-user-interaction-keydown-expected.txt
>@@ -0,0 +1,9 @@
>+Tests that the beforeunload alert is not shown when the page received a keyDown but no keyUp event. 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-user-interaction-keydown.html b/LayoutTests/fast/events/beforeunload-alert-user-interaction-keydown.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..ac2c3a7d6378375c3460e548a387ebda650b456f
>--- /dev/null
>+++ b/LayoutTests/fast/events/beforeunload-alert-user-interaction-keydown.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 not shown when the page received a keyDown but no keyUp event. 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.";
>+    };
>+    // Simulate a user interaction.
>+    const testInput = document.getElementById("testInput");
>+    testInput.focus();
>+    if (window.eventSender)
>+        eventSender.keyDown("a", false /* shouldSendKeyUp */);
>+    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/platform/ios-simulator/TestExpectations b/LayoutTests/platform/ios-simulator/TestExpectations
>index f7b8c9bda4489bc4f135bac69fcd0292c6d0bc20..1224b5e5e1a99ac0bc280c38a0239629ba7947d6 100644
>--- a/LayoutTests/platform/ios-simulator/TestExpectations
>+++ b/LayoutTests/platform/ios-simulator/TestExpectations
>@@ -364,6 +364,7 @@ fast/text/all-small-caps-whitespace.html [ Skip ]
> 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/events/beforeunload-alert-user-interaction-keyDown.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 169995: 305166 | 305167 | 305170 | 305171 | 305172 | 305197 | 306093 | 306961 | 306995 | 307037 | 307057 | 307060