From 941a6cf143bd8923da10d75435fb48017019c735 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Tue, 5 Nov 2024 01:14:15 -0500 Subject: [PATCH 1/8] [dotnet] Gracefully handle clashing device names --- dotnet/src/webdriver/Interactions/Actions.cs | 90 +++++++------------ .../Interactions/CombinedInputActionsTest.cs | 29 ++++++ 2 files changed, 62 insertions(+), 57 deletions(-) diff --git a/dotnet/src/webdriver/Interactions/Actions.cs b/dotnet/src/webdriver/Interactions/Actions.cs index dddde06781b7a..0064d76f3d31c 100644 --- a/dotnet/src/webdriver/Interactions/Actions.cs +++ b/dotnet/src/webdriver/Interactions/Actions.cs @@ -31,16 +31,15 @@ public class Actions : IAction private PointerInputDevice activePointer; private KeyInputDevice activeKeyboard; private WheelInputDevice activeWheel; - private IActionExecutor actionExecutor; /// /// Initializes a new instance of the class. /// /// The object on which the actions built will be performed. + /// If does not implement IActionExecutor. public Actions(IWebDriver driver) : this(driver, TimeSpan.FromMilliseconds(250)) { - } /// @@ -48,6 +47,7 @@ public Actions(IWebDriver driver) /// /// The object on which the actions built will be performed. /// How long durable action is expected to take. + /// If does not implement IActionExecutor. public Actions(IWebDriver driver, TimeSpan duration) { IActionExecutor actionExecutor = GetDriverAs(driver); @@ -56,7 +56,7 @@ public Actions(IWebDriver driver, TimeSpan duration) throw new ArgumentException("The IWebDriver object must implement or wrap a driver that implements IActionExecutor.", nameof(driver)); } - this.actionExecutor = actionExecutor; + this.ActionExecutor = actionExecutor; this.duration = duration; } @@ -64,10 +64,7 @@ public Actions(IWebDriver driver, TimeSpan duration) /// /// Returns the for the driver. /// - protected IActionExecutor ActionExecutor - { - get { return this.actionExecutor; } - } + protected IActionExecutor ActionExecutor { get; } /// /// Sets the active pointer device for this Actions class. @@ -75,24 +72,10 @@ protected IActionExecutor ActionExecutor /// The kind of pointer device to set as active. /// The name of the pointer device to set as active. /// A self-reference to this Actions class. + /// If a device with this name exists but is not a pointer. public Actions SetActivePointer(PointerKind kind, string name) { - IList sequences = this.actionBuilder.ToActionSequenceList(); - - InputDevice device = null; - - foreach (var sequence in sequences) - { - Dictionary actions = sequence.ToDictionary(); - - string id = (string)actions["id"]; - - if (id == name) - { - device = sequence.inputDevice; - break; - } - } + InputDevice device = FindDeviceById(name); if (device == null) { @@ -100,7 +83,8 @@ public Actions SetActivePointer(PointerKind kind, string name) } else { - this.activePointer = (PointerInputDevice)device; + this.activePointer = device as PointerInputDevice + ?? throw new InvalidOperationException($"Device under the name \"{name}\" is not a pointer. Actual input type: {device.DeviceKind}"); } return this; @@ -111,24 +95,10 @@ public Actions SetActivePointer(PointerKind kind, string name) /// /// The name of the keyboard device to set as active. /// A self-reference to this Actions class. + /// If a device with this name exists but is not a keyboard. public Actions SetActiveKeyboard(string name) { - IList sequences = this.actionBuilder.ToActionSequenceList(); - - InputDevice device = null; - - foreach (var sequence in sequences) - { - Dictionary actions = sequence.ToDictionary(); - - string id = (string)actions["id"]; - - if (id == name) - { - device = sequence.inputDevice; - break; - } - } + InputDevice device = FindDeviceById(name); if (device == null) { @@ -136,7 +106,9 @@ public Actions SetActiveKeyboard(string name) } else { - this.activeKeyboard = (KeyInputDevice)device; + this.activeKeyboard = device as KeyInputDevice + ?? throw new InvalidOperationException($"Device under the name \"{name}\" is not a keyboard. Actual input type: {device.DeviceKind}"); + } return this; @@ -147,13 +119,27 @@ public Actions SetActiveKeyboard(string name) /// /// The name of the wheel device to set as active. /// A self-reference to this Actions class. + /// If a device with this name exists but is not a wheel. public Actions SetActiveWheel(string name) { - IList sequences = this.actionBuilder.ToActionSequenceList(); + InputDevice device = FindDeviceById(name); - InputDevice device = null; + if (device == null) + { + this.activeWheel = new WheelInputDevice(name); + } + else + { + this.activeWheel = device as WheelInputDevice + ?? throw new InvalidOperationException($"Device under the name \"{name}\" is not a wheel. Actual input type: {device.DeviceKind}"); + } - foreach (var sequence in sequences) + return this; + } + + private InputDevice FindDeviceById(string name) + { + foreach (var sequence in this.actionBuilder.ToActionSequenceList()) { Dictionary actions = sequence.ToDictionary(); @@ -161,21 +147,11 @@ public Actions SetActiveWheel(string name) if (id == name) { - device = sequence.inputDevice; - break; + return sequence.inputDevice; } } - if (device == null) - { - this.activeWheel = new WheelInputDevice(name); - } - else - { - this.activeWheel = (WheelInputDevice)device; - } - - return this; + return null; } /// @@ -619,7 +595,7 @@ public IAction Build() /// public void Perform() { - this.actionExecutor.PerformActions(this.actionBuilder.ToActionSequenceList()); + this.ActionExecutor.PerformActions(this.actionBuilder.ToActionSequenceList()); this.actionBuilder.ClearSequences(); } diff --git a/dotnet/test/common/Interactions/CombinedInputActionsTest.cs b/dotnet/test/common/Interactions/CombinedInputActionsTest.cs index 483f1204b7ca8..77e443afec25d 100644 --- a/dotnet/test/common/Interactions/CombinedInputActionsTest.cs +++ b/dotnet/test/common/Interactions/CombinedInputActionsTest.cs @@ -397,6 +397,35 @@ public void PerformsPause() Assert.IsTrue(DateTime.Now - start > TimeSpan.FromMilliseconds(1200)); } + + [Test] + public void ShouldHandleClashingDeviceNamesGracefully() + { + var actionsWithKeyboard = new Actions(driver) + .SetActiveKeyboard("test") + .KeyDown(Keys.Shift).KeyUp(Keys.Shift); + + Assert.That(() => + { + actionsWithKeyboard.SetActiveWheel("test"); + }, Throws.InvalidOperationException.With.Message.EqualTo("Device under the name \"test\" is not a wheel. Actual input type: Key")); + + Assert.That(() => + { + actionsWithKeyboard.SetActivePointer(PointerKind.Pen, "test"); + }, Throws.InvalidOperationException.With.Message.EqualTo("Device under the name \"test\" is not a pointer. Actual input type: Key")); + + var actionsWithWheel = new Actions(driver) + .SetActiveWheel("test") + .ScrollByAmount(0, 0); + + Assert.That(() => + { + actionsWithWheel.SetActiveKeyboard("test"); + }, Throws.InvalidOperationException.With.Message.EqualTo("Device under the name \"test\" is not a keyboard. Actual input type: Wheel")); + } + + private bool FuzzyPositionMatching(int expectedX, int expectedY, string locationTuple) { string[] splitString = locationTuple.Split(','); From 3e5bd16648ea25b04b9a575a9797d189eb923e83 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Tue, 5 Nov 2024 01:20:55 -0500 Subject: [PATCH 2/8] improve test a bit --- .../Interactions/CombinedInputActionsTest.cs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/dotnet/test/common/Interactions/CombinedInputActionsTest.cs b/dotnet/test/common/Interactions/CombinedInputActionsTest.cs index 77e443afec25d..49b0a7a767d37 100644 --- a/dotnet/test/common/Interactions/CombinedInputActionsTest.cs +++ b/dotnet/test/common/Interactions/CombinedInputActionsTest.cs @@ -401,14 +401,18 @@ public void PerformsPause() [Test] public void ShouldHandleClashingDeviceNamesGracefully() { - var actionsWithKeyboard = new Actions(driver) - .SetActiveKeyboard("test") - .KeyDown(Keys.Shift).KeyUp(Keys.Shift); + var actionsWithPointer = new Actions(driver) + .SetActivePointer(PointerKind.Mouse, "test") + .Click(); Assert.That(() => { - actionsWithKeyboard.SetActiveWheel("test"); - }, Throws.InvalidOperationException.With.Message.EqualTo("Device under the name \"test\" is not a wheel. Actual input type: Key")); + actionsWithPointer.SetActiveWheel("test"); + }, Throws.InvalidOperationException.With.Message.EqualTo("Device under the name \"test\" is not a wheel. Actual input type: Pointer")); + + var actionsWithKeyboard = new Actions(driver) + .SetActiveKeyboard("test") + .KeyDown(Keys.Shift).KeyUp(Keys.Shift); Assert.That(() => { From 05856d0221c53bcf21c9ca7adecf029ec68af466 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Tue, 5 Nov 2024 09:52:21 -0500 Subject: [PATCH 3/8] Add XML link to Actions constructor --- dotnet/src/webdriver/Interactions/Actions.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dotnet/src/webdriver/Interactions/Actions.cs b/dotnet/src/webdriver/Interactions/Actions.cs index 0064d76f3d31c..5bc6cd7e6025a 100644 --- a/dotnet/src/webdriver/Interactions/Actions.cs +++ b/dotnet/src/webdriver/Interactions/Actions.cs @@ -36,7 +36,7 @@ public class Actions : IAction /// Initializes a new instance of the class. /// /// The object on which the actions built will be performed. - /// If does not implement IActionExecutor. + /// If does not implement . public Actions(IWebDriver driver) : this(driver, TimeSpan.FromMilliseconds(250)) { @@ -47,7 +47,7 @@ public Actions(IWebDriver driver) /// /// The object on which the actions built will be performed. /// How long durable action is expected to take. - /// If does not implement IActionExecutor. + /// If does not implement . public Actions(IWebDriver driver, TimeSpan duration) { IActionExecutor actionExecutor = GetDriverAs(driver); From 0db1b1f04aecc1c8b1c8e82883a3fea3a87cb04d Mon Sep 17 00:00:00 2001 From: Michael Render Date: Tue, 5 Nov 2024 13:21:03 -0500 Subject: [PATCH 4/8] Simplify active device assignment --- dotnet/src/webdriver/Interactions/Actions.cs | 40 ++++++++------------ 1 file changed, 15 insertions(+), 25 deletions(-) diff --git a/dotnet/src/webdriver/Interactions/Actions.cs b/dotnet/src/webdriver/Interactions/Actions.cs index 5bc6cd7e6025a..36c58a925b78b 100644 --- a/dotnet/src/webdriver/Interactions/Actions.cs +++ b/dotnet/src/webdriver/Interactions/Actions.cs @@ -77,15 +77,12 @@ public Actions SetActivePointer(PointerKind kind, string name) { InputDevice device = FindDeviceById(name); - if (device == null) + this.activePointer = device switch { - this.activePointer = new PointerInputDevice(kind, name); - } - else - { - this.activePointer = device as PointerInputDevice - ?? throw new InvalidOperationException($"Device under the name \"{name}\" is not a pointer. Actual input type: {device.DeviceKind}"); - } + null => new PointerInputDevice(kind, name), + PointerInputDevice pointerDevice => pointerDevice, + _ => throw new InvalidOperationException($"Device under the name \"{name}\" is not a pointer. Actual input type: {device.DeviceKind}"), + }; return this; } @@ -100,16 +97,12 @@ public Actions SetActiveKeyboard(string name) { InputDevice device = FindDeviceById(name); - if (device == null) - { - this.activeKeyboard = new KeyInputDevice(name); - } - else + this.activeKeyboard = device switch { - this.activeKeyboard = device as KeyInputDevice - ?? throw new InvalidOperationException($"Device under the name \"{name}\" is not a keyboard. Actual input type: {device.DeviceKind}"); - - } + null => new KeyInputDevice(name), + KeyInputDevice keyDevice => keyDevice, + _ => throw new InvalidOperationException($"Device under the name \"{name}\" is not a keyboard. Actual input type: {device.DeviceKind}"), + }; return this; } @@ -124,15 +117,12 @@ public Actions SetActiveWheel(string name) { InputDevice device = FindDeviceById(name); - if (device == null) - { - this.activeWheel = new WheelInputDevice(name); - } - else + this.activeWheel = device switch { - this.activeWheel = device as WheelInputDevice - ?? throw new InvalidOperationException($"Device under the name \"{name}\" is not a wheel. Actual input type: {device.DeviceKind}"); - } + null => new WheelInputDevice(name), + WheelInputDevice wheelDevice => wheelDevice, + _ => throw new InvalidOperationException($"Device under the name \"{name}\" is not a wheel. Actual input type: {device.DeviceKind}"), + }; return this; } From 70cded9c4840c3ddb1d97001424bfbb41429ebe9 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Tue, 5 Nov 2024 13:28:19 -0500 Subject: [PATCH 5/8] Update dotnet/test/common/Interactions/CombinedInputActionsTest.cs --- dotnet/test/common/Interactions/CombinedInputActionsTest.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/dotnet/test/common/Interactions/CombinedInputActionsTest.cs b/dotnet/test/common/Interactions/CombinedInputActionsTest.cs index 49b0a7a767d37..16cb913497f3b 100644 --- a/dotnet/test/common/Interactions/CombinedInputActionsTest.cs +++ b/dotnet/test/common/Interactions/CombinedInputActionsTest.cs @@ -397,7 +397,6 @@ public void PerformsPause() Assert.IsTrue(DateTime.Now - start > TimeSpan.FromMilliseconds(1200)); } - [Test] public void ShouldHandleClashingDeviceNamesGracefully() { From 0561bf9f31952915e324b4d585d04b1be60298ef Mon Sep 17 00:00:00 2001 From: Michael Render Date: Tue, 5 Nov 2024 13:28:59 -0500 Subject: [PATCH 6/8] Update dotnet/test/common/Interactions/CombinedInputActionsTest.cs --- dotnet/test/common/Interactions/CombinedInputActionsTest.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/dotnet/test/common/Interactions/CombinedInputActionsTest.cs b/dotnet/test/common/Interactions/CombinedInputActionsTest.cs index 16cb913497f3b..711259b785aa5 100644 --- a/dotnet/test/common/Interactions/CombinedInputActionsTest.cs +++ b/dotnet/test/common/Interactions/CombinedInputActionsTest.cs @@ -428,7 +428,6 @@ public void ShouldHandleClashingDeviceNamesGracefully() }, Throws.InvalidOperationException.With.Message.EqualTo("Device under the name \"test\" is not a keyboard. Actual input type: Wheel")); } - private bool FuzzyPositionMatching(int expectedX, int expectedY, string locationTuple) { string[] splitString = locationTuple.Split(','); From 05b4cb42c115b3923cb51fde68f162fb11d8f2e9 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Tue, 5 Nov 2024 14:14:16 -0500 Subject: [PATCH 7/8] Fix infinite recursion when calling `ActionSequence.inputDevice` --- dotnet/src/webdriver/Interactions/ActionSequence.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotnet/src/webdriver/Interactions/ActionSequence.cs b/dotnet/src/webdriver/Interactions/ActionSequence.cs index 68eabbea00f2b..b46b7f50f06d4 100644 --- a/dotnet/src/webdriver/Interactions/ActionSequence.cs +++ b/dotnet/src/webdriver/Interactions/ActionSequence.cs @@ -73,7 +73,7 @@ public int Count /// public InputDevice inputDevice { - get { return this.inputDevice; } + get { return this.device; } } /// From d2b565033d02392c38188718ece8ec9b55dc46c8 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Tue, 5 Nov 2024 14:39:51 -0500 Subject: [PATCH 8/8] Rename `ActionSequence.inputDevice` to `ActionSequence.InputDevice` --- .../webdriver/Interactions/ActionSequence.cs | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/dotnet/src/webdriver/Interactions/ActionSequence.cs b/dotnet/src/webdriver/Interactions/ActionSequence.cs index b46b7f50f06d4..7aebebd8a3b35 100644 --- a/dotnet/src/webdriver/Interactions/ActionSequence.cs +++ b/dotnet/src/webdriver/Interactions/ActionSequence.cs @@ -29,7 +29,6 @@ namespace OpenQA.Selenium.Interactions public class ActionSequence { private List interactions = new List(); - private InputDevice device; /// /// Initializes a new instance of the class. @@ -52,7 +51,7 @@ public ActionSequence(InputDevice device, int initialSize) throw new ArgumentNullException(nameof(device), "Input device cannot be null."); } - this.device = device; + this.InputDevice = device; for (int i = 0; i < initialSize; i++) { @@ -71,10 +70,13 @@ public int Count /// /// Gets the input device for this Action sequence. /// - public InputDevice inputDevice - { - get { return this.device; } - } + [Obsolete("This property has been renamed to InputDevice and will be removed in a future version")] + public InputDevice inputDevice => InputDevice; + + /// + /// Gets the input device for this Action sequence. + /// + public InputDevice InputDevice { get; } /// /// Adds an action to the sequence. @@ -88,9 +90,9 @@ public ActionSequence AddAction(Interaction interactionToAdd) throw new ArgumentNullException(nameof(interactionToAdd), "Interaction to add to sequence must not be null"); } - if (!interactionToAdd.IsValidFor(this.device.DeviceKind)) + if (!interactionToAdd.IsValidFor(this.InputDevice.DeviceKind)) { - throw new ArgumentException(string.Format(CultureInfo.InvariantCulture, "Interaction {0} is invalid for device type {1}.", interactionToAdd.GetType(), this.device.DeviceKind), nameof(interactionToAdd)); + throw new ArgumentException(string.Format(CultureInfo.InvariantCulture, "Interaction {0} is invalid for device type {1}.", interactionToAdd.GetType(), this.InputDevice.DeviceKind), nameof(interactionToAdd)); } this.interactions.Add(interactionToAdd); @@ -103,7 +105,7 @@ public ActionSequence AddAction(Interaction interactionToAdd) /// A containing the actions in this sequence. public Dictionary ToDictionary() { - Dictionary toReturn = this.device.ToDictionary(); + Dictionary toReturn = this.InputDevice.ToDictionary(); List encodedActions = new List(); foreach (Interaction action in this.interactions) @@ -122,7 +124,7 @@ public Dictionary ToDictionary() /// A string that represents the current . public override string ToString() { - StringBuilder builder = new StringBuilder("Action sequence - ").Append(this.device.ToString()); + StringBuilder builder = new StringBuilder("Action sequence - ").Append(this.InputDevice.ToString()); foreach (Interaction action in this.interactions) { builder.AppendLine();