[Mac] Web Automation: key modifiers for synthesized NSEvents are inco…

…rrect

https://bugs.webkit.org/show_bug.cgi?id=178615

Reviewed by Joseph Pecoraro.

In both PLATFORM(MAC) platform methods for simulating keyboard interactions,
we errantly relied on +[NSEvent modifierFlags] to get the current state of
sticky modifiers when creating synthesized events. This is incorrect for two reasons:
modifierFlags is never updated when simulating a sequence of events (because
all the events are synthesized before any are delivered); and the NSEvent class
method only reflects the modifier state of the primary physical keyboard, which
is not affected by synthesized NSEvents that happen to have modifier flags.

Instead, just keep our own m_currentModifiers state in the session and compute
the necessary NSEventModifierFlags to put on each synthesized event. This aligns
the implementation with the treatment of sticky keys in the iOS and GTK platform methods.

* UIProcess/Automation/WebAutomationSession.h: Every port gets this variable now.
* UIProcess/Automation/mac/WebAutomationSessionMac.mm:
(WebKit::WebAutomationSession::platformSimulateKeyStroke):
(WebKit::WebAutomationSession::platformSimulateKeySequence):
Use and update m_currentModifiers.


Canonical link: https://commits.webkit.org/194865@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@223869 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog

@@ -1,3 +1,28 @@
+2017-10-23  Brian Burg  <bburg@apple.com>
+
+        [Mac] Web Automation: key modifiers for synthesized NSEvents are incorrect
+        https://bugs.webkit.org/show_bug.cgi?id=178615
+
+        Reviewed by Joseph Pecoraro.
+
+        In both PLATFORM(MAC) platform methods for simulating keyboard interactions,
+        we errantly relied on +[NSEvent modifierFlags] to get the current state of
+        sticky modifiers when creating synthesized events. This is incorrect for two reasons:
+        modifierFlags is never updated when simulating a sequence of events (because
+        all the events are synthesized before any are delivered); and the NSEvent class
+        method only reflects the modifier state of the primary physical keyboard, which
+        is not affected by synthesized NSEvents that happen to have modifier flags.
+
+        Instead, just keep our own m_currentModifiers state in the session and compute
+        the necessary NSEventModifierFlags to put on each synthesized event. This aligns
+        the implementation with the treatment of sticky keys in the iOS and GTK platform methods.
+
+        * UIProcess/Automation/WebAutomationSession.h: Every port gets this variable now.
+        * UIProcess/Automation/mac/WebAutomationSessionMac.mm:
+        (WebKit::WebAutomationSession::platformSimulateKeyStroke):
+        (WebKit::WebAutomationSession::platformSimulateKeySequence):
+        Use and update m_currentModifiers.
+
 2017-10-23  Alex Christensen  <achristensen@webkit.org>
 
         Fix ASAN test after r222824

Source/WebKit/UIProcess/Automation/WebAutomationSession.h

@@ -259,16 +259,15 @@ class WebAutomationSession final : public API::ObjectImpl<API::Object::Type::Aut
 
     bool m_permissionForGetUserMedia { true };
 
-#if ENABLE(REMOTE_INSPECTOR)
-    Inspector::FrontendChannel* m_remoteChannel { nullptr };
-#endif
 
-#if PLATFORM(IOS) || PLATFORM(GTK)
     // Keep track of currently active modifiers across multiple keystrokes.
-    // We don't synthesize platform keyboard events on iOS, so we need to track it ourselves.
-    // GTK+ doesn't keep track of the active modifiers when using synthesized events.
+    // Most platforms do not track current modifiers from synthesized events.
     unsigned m_currentModifiers { 0 };
+
+#if ENABLE(REMOTE_INSPECTOR)
+    Inspector::FrontendChannel* m_remoteChannel { nullptr };
 #endif
+
 };
 
 } // namespace WebKit

Source/WebKit/UIProcess/Automation/mac/WebAutomationSessionMac.mm

@@ -445,9 +445,6 @@ static NSEventModifierFlags eventModifierFlagsForVirtualKey(Inspector::Protocol:
 
     auto eventsToBeSent = adoptNS([[NSMutableArray alloc] init]);
 
-    NSEventModifierFlags existingModifiers = [NSEvent modifierFlags];
-    NSEventModifierFlags updatedModifiers = 0;
-
     // FIXME: this timestamp is not even close to matching native events. Find out how to get closer.
     NSTimeInterval timestamp = [NSDate timeIntervalSinceReferenceDate];
     NSWindow *window = page.platformWindow();
@@ -457,14 +454,14 @@ static NSEventModifierFlags eventModifierFlagsForVirtualKey(Inspector::Protocol:
     switch (interaction) {
     case Inspector::Protocol::Automation::KeyboardInteractionType::KeyPress: {
         NSEventType eventType = isStickyModifier ? NSEventTypeFlagsChanged : NSEventTypeKeyDown;
-        updatedModifiers = existingModifiers | changedModifiers;
-        [eventsToBeSent addObject:[NSEvent keyEventWithType:eventType location:eventPosition modifierFlags:updatedModifiers timestamp:timestamp windowNumber:windowNumber context:nil characters:characters charactersIgnoringModifiers:unmodifiedCharacters isARepeat:NO keyCode:keyCode]];
+        m_currentModifiers |= changedModifiers;
+        [eventsToBeSent addObject:[NSEvent keyEventWithType:eventType location:eventPosition modifierFlags:m_currentModifiers timestamp:timestamp windowNumber:windowNumber context:nil characters:characters charactersIgnoringModifiers:unmodifiedCharacters isARepeat:NO keyCode:keyCode]];
         break;
     }
     case Inspector::Protocol::Automation::KeyboardInteractionType::KeyRelease: {
         NSEventType eventType = isStickyModifier ? NSEventTypeFlagsChanged : NSEventTypeKeyUp;
-        updatedModifiers = existingModifiers & ~changedModifiers;
-        [eventsToBeSent addObject:[NSEvent keyEventWithType:eventType location:eventPosition modifierFlags:updatedModifiers timestamp:timestamp windowNumber:windowNumber context:nil characters:characters charactersIgnoringModifiers:unmodifiedCharacters isARepeat:NO keyCode:keyCode]];
+        m_currentModifiers &= ~changedModifiers;
+        [eventsToBeSent addObject:[NSEvent keyEventWithType:eventType location:eventPosition modifierFlags:m_currentModifiers timestamp:timestamp windowNumber:windowNumber context:nil characters:characters charactersIgnoringModifiers:unmodifiedCharacters isARepeat:NO keyCode:keyCode]];
         break;
     }
     case Inspector::Protocol::Automation::KeyboardInteractionType::InsertByKey: {
@@ -473,9 +470,9 @@ static NSEventModifierFlags eventModifierFlagsForVirtualKey(Inspector::Protocol:
         if (isStickyModifier)
             return;
 
-        updatedModifiers = existingModifiers | changedModifiers;
-        [eventsToBeSent addObject:[NSEvent keyEventWithType:NSEventTypeKeyDown location:eventPosition modifierFlags:updatedModifiers timestamp:timestamp windowNumber:windowNumber context:nil characters:characters charactersIgnoringModifiers:unmodifiedCharacters isARepeat:NO keyCode:keyCode]];
-        [eventsToBeSent addObject:[NSEvent keyEventWithType:NSEventTypeKeyUp location:eventPosition modifierFlags:updatedModifiers timestamp:timestamp windowNumber:windowNumber context:nil characters:characters charactersIgnoringModifiers:unmodifiedCharacters isARepeat:NO keyCode:keyCode]];
+        m_currentModifiers |= changedModifiers;
+        [eventsToBeSent addObject:[NSEvent keyEventWithType:NSEventTypeKeyDown location:eventPosition modifierFlags:m_currentModifiers timestamp:timestamp windowNumber:windowNumber context:nil characters:characters charactersIgnoringModifiers:unmodifiedCharacters isARepeat:NO keyCode:keyCode]];
+        [eventsToBeSent addObject:[NSEvent keyEventWithType:NSEventTypeKeyUp location:eventPosition modifierFlags:m_currentModifiers timestamp:timestamp windowNumber:windowNumber context:nil characters:characters charactersIgnoringModifiers:unmodifiedCharacters isARepeat:NO keyCode:keyCode]];
         break;
     }
     }
@@ -535,7 +532,6 @@ static BOOL characterIsProducedUsingShift(NSString *characters)
     // This API should move more towards that direction in the future.
     NSString *text = keySequence;
 
-    NSEventModifierFlags modifiers = [NSEvent modifierFlags];
     NSTimeInterval timestamp = [NSDate timeIntervalSinceReferenceDate];
     NSWindow *window = page.platformWindow();
     NSInteger windowNumber = window.windowNumber;
@@ -546,9 +542,9 @@ static BOOL characterIsProducedUsingShift(NSString *characters)
         // For ASCII characters that are produced using Shift on a US-108 key keyboard layout,
         // WebDriver expects these to be delivered as [shift-down, key-down, key-up, shift-up]
         // keystrokes unless the shift key is already pressed down from an earlier interaction.
-        BOOL shiftAlreadyPressed = modifiers & NSEventModifierFlagShift;
+        BOOL shiftAlreadyPressed = m_currentModifiers & NSEventModifierFlagShift;
         BOOL shouldPressShift = !shiftAlreadyPressed && substringRange.length == 1 && characterIsProducedUsingShift(substring);
-        NSEventModifierFlags modifiersForKeystroke = shouldPressShift ? modifiers | NSEventModifierFlagShift : modifiers;
+        NSEventModifierFlags modifiersForKeystroke = shouldPressShift ? m_currentModifiers | NSEventModifierFlagShift : m_currentModifiers;
 
         if (shouldPressShift)
             [eventsToBeSent addObject:[NSEvent keyEventWithType:NSEventTypeFlagsChanged location:eventPosition modifierFlags:modifiersForKeystroke timestamp:timestamp windowNumber:windowNumber context:nil characters:@"" charactersIgnoringModifiers:@"" isARepeat:NO keyCode:kVK_Shift]];
@@ -557,7 +553,7 @@ static BOOL characterIsProducedUsingShift(NSString *characters)
         [eventsToBeSent addObject:[NSEvent keyEventWithType:NSEventTypeKeyUp location:eventPosition modifierFlags:modifiersForKeystroke timestamp:timestamp windowNumber:windowNumber context:nil characters:substring charactersIgnoringModifiers:substring isARepeat:NO keyCode:0]];
 
         if (shouldPressShift)
-            [eventsToBeSent addObject:[NSEvent keyEventWithType:NSEventTypeFlagsChanged location:eventPosition modifierFlags:modifiers timestamp:timestamp windowNumber:windowNumber context:nil characters:@"" charactersIgnoringModifiers:@"" isARepeat:NO keyCode:kVK_Shift]];
+            [eventsToBeSent addObject:[NSEvent keyEventWithType:NSEventTypeFlagsChanged location:eventPosition modifierFlags:m_currentModifiers timestamp:timestamp windowNumber:windowNumber context:nil characters:@"" charactersIgnoringModifiers:@"" isARepeat:NO keyCode:kVK_Shift]];
     }];
 
     sendSynthesizedEventsToPage(page, eventsToBeSent.get());