Skip to content

Commit

Permalink
Fix up bugs to with Actions when using JSON or W3C commands
Browse files Browse the repository at this point in the history 
There were a number of bugs in the recently-landed updates
to `Actions`. The major ones were:

 * If the `WebDriver` instance implemented both `Interactive`
   and `HasInputDevices` the wrong set of W3C actions would
   be constructed.

 * It was occasionally possible for multiple `Action`
   instances to be added where only one was needed.

The solution makes the implementation of `Actions` uglier
but does resolve the problem. What we do is ensure that each
public method correctly populates both the `CompositeAction`
and the action sequences required for the W3C implementation.
To make it trivially easy to determine that each method is
doing this, we avoid delegation to other methods, even when
this would make life easier (eg. "click()" -> "click(null)")
as this would require additional logic in each of the methods
to figure out what the intended behaviour (already given by
the method call!) would be.

This isn't my neatest work, but I think it works.
  • Loading branch information
@shs96c
shs96c committed Feb 20, 2017
1 parent 5d7ab06 commit 30012e4
Showing 1 changed file with 75 additions and 50 deletions.
125 changes: 75 additions & 50 deletions java/client/src/org/openqa/selenium/interactions/Actions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@

import static org.openqa.selenium.interactions.PointerInput.Kind.MOUSE;
import static org.openqa.selenium.interactions.PointerInput.MouseButton.LEFT;
import static org.openqa.selenium.interactions.PointerInput.MouseButton.RIGHT;

import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;

import org.openqa.selenium.Keys;
import org.openqa.selenium.UnsupportedCommandException;
Expand All @@ -38,6 +40,7 @@
import java.util.Optional;
import java.util.Set;
import java.util.function.IntConsumer;
import java.util.logging.Logger;

/**
* The user-facing API for emulating complex user gestures. Use this class rather than using the
Expand All @@ -48,6 +51,7 @@
*/
public class Actions {

private final static Logger LOG = Logger.getLogger(Actions.class.getName());
private final WebDriver driver;

// W3C
Expand Down Expand Up @@ -114,7 +118,7 @@ public Actions(Keyboard keyboard) {
*/
public Actions keyDown(CharSequence key) {
if (isBuildingActions()) {
return keyDown(null, key);
action.addAction(new KeyDownAction(jsonKeyboard, jsonMouse, asKeys(key)));
}
return addKeyAction(key, codePoint -> tick(defaultKeyboard.createKeyDown(codePoint)));
}
Expand All @@ -132,9 +136,9 @@ public Actions keyDown(CharSequence key) {
public Actions keyDown(WebElement target, CharSequence key) {
if (isBuildingActions()) {
action.addAction(new KeyDownAction(jsonKeyboard, jsonMouse, (Locatable) target, asKeys(key)));
return this;
}
return click(target).keyDown(key);
return focusInTicks(target)
.addKeyAction(key, codepoint -> tick(defaultKeyboard.createKeyDown(codepoint)));
}

/**
Expand All @@ -146,7 +150,7 @@ public Actions keyDown(WebElement target, CharSequence key) {
*/
public Actions keyUp(CharSequence key) {
if (isBuildingActions()) {
return keyUp(null, key);
action.addAction(new KeyUpAction(jsonKeyboard, jsonMouse, asKeys(key)));
}

return addKeyAction(key, codePoint -> tick(defaultKeyboard.createKeyUp(codePoint)));
Expand All @@ -164,9 +168,10 @@ public Actions keyUp(CharSequence key) {
public Actions keyUp(WebElement target, CharSequence key) {
if (isBuildingActions()) {
action.addAction(new KeyUpAction(jsonKeyboard, jsonMouse, (Locatable) target, asKeys(key)));
return this;
}
return click(target).keyUp(key);

return focusInTicks(target)
.addKeyAction(key, codePoint -> tick(defaultKeyboard.createKeyUp(codePoint)));
}

/**
Expand All @@ -186,17 +191,9 @@ public Actions keyUp(WebElement target, CharSequence key) {
public Actions sendKeys(CharSequence... keys) {
if (isBuildingActions()) {
action.addAction(new SendKeysAction(jsonKeyboard, jsonMouse, null, keys));
return this;
}

for (CharSequence key : keys) {
key.codePoints().forEach(codePoint -> {
tick(defaultKeyboard.createKeyDown(codePoint));
tick(defaultKeyboard.createKeyUp(codePoint));
});
}

return this;
return sendKeysInTicks(keys);
}

/**
Expand All @@ -214,9 +211,9 @@ public Actions sendKeys(CharSequence... keys) {
public Actions sendKeys(WebElement target, CharSequence... keys) {
if (isBuildingActions()) {
action.addAction(new SendKeysAction(jsonKeyboard, jsonMouse, (Locatable) target, keys));
return this;
}
return click(target).sendKeys(keys);

return focusInTicks(target).sendKeysInTicks(keys);
}

private Keys asKeys(CharSequence key) {
Expand All @@ -228,6 +225,16 @@ private Keys asKeys(CharSequence key) {
return (Keys) key;
}

private Actions sendKeysInTicks(CharSequence... keys) {
for (CharSequence key : keys) {
key.codePoints().forEach(codePoint -> {
tick(defaultKeyboard.createKeyDown(codePoint));
tick(defaultKeyboard.createKeyUp(codePoint));
});
}
return this;
}

private Actions addKeyAction(CharSequence key, IntConsumer consumer) {
// Verify that we only have a single character to type.
Preconditions.checkState(
Expand All @@ -249,9 +256,9 @@ private Actions addKeyAction(CharSequence key, IntConsumer consumer) {
public Actions clickAndHold(WebElement target) {
if (isBuildingActions()) {
action.addAction(new ClickAndHoldAction(jsonMouse, (Locatable) target));
return this;
}
return moveToElement(target).clickAndHold();
return moveInTicks(target, 0, 0)
.tick(defaultMouse.createPointerDown(LEFT.asArg()));
}

/**
Expand All @@ -260,11 +267,10 @@ public Actions clickAndHold(WebElement target) {
*/
public Actions clickAndHold() {
if (isBuildingActions()) {
return clickAndHold(null);
action.addAction(new ClickAndHoldAction(jsonMouse, null));
}

tick(defaultMouse.createPointerDown(LEFT.asArg()));
return this;
return tick(defaultMouse.createPointerDown(LEFT.asArg()));
}

/**
Expand All @@ -281,9 +287,9 @@ public Actions clickAndHold() {
public Actions release(WebElement target) {
if (isBuildingActions()) {
action.addAction(new ButtonReleaseAction(jsonMouse, (Locatable) target));
return this;
}
return moveToElement(target).release();

return moveInTicks(target, 0, 0).tick(defaultMouse.createPointerUp(LEFT.asArg()));
}

/**
Expand All @@ -293,7 +299,7 @@ public Actions release(WebElement target) {
*/
public Actions release() {
if (isBuildingActions()) {
this.release(null);
action.addAction(new ButtonReleaseAction(jsonMouse, null));
}

return tick(defaultMouse.createPointerUp(Button.LEFT.asArg()));
Expand All @@ -309,9 +315,9 @@ public Actions release() {
public Actions click(WebElement target) {
if (isBuildingActions()) {
action.addAction(new ClickAction(jsonMouse, (Locatable) target));
return this;
}
return moveToElement(target).click();

return moveInTicks(target, 0, 0).clickInTicks(LEFT);
}

/**
Expand All @@ -322,14 +328,22 @@ public Actions click(WebElement target) {
*/
public Actions click() {
if (isBuildingActions()) {
return click(null);
action.addAction(new ClickAction(jsonMouse, null));
}
tick(defaultMouse.createPointerDown(0));
tick(defaultMouse.createPointerUp(0));

return clickInTicks(LEFT);
}

private Actions clickInTicks(PointerInput.MouseButton button) {
tick(defaultMouse.createPointerDown(button.asArg()));
tick(defaultMouse.createPointerUp(button.asArg()));
return this;
}

private Actions focusInTicks(WebElement target) {
return moveInTicks(target, 0, 0).clickInTicks(LEFT);
}

/**
* Performs a double-click at middle of the given element. Equivalent to:
* <i>Actions.moveToElement(element).doubleClick()</i>
Expand All @@ -340,9 +354,11 @@ public Actions click() {
public Actions doubleClick(WebElement target) {
if (isBuildingActions()) {
action.addAction(new DoubleClickAction(jsonMouse, (Locatable) target));
return this;
}
return moveToElement(target).doubleClick();

return moveInTicks(target, 0, 0)
.clickInTicks(LEFT)
.clickInTicks(LEFT);
}

/**
Expand All @@ -351,9 +367,10 @@ public Actions doubleClick(WebElement target) {
*/
public Actions doubleClick() {
if (isBuildingActions()) {
return doubleClick(null);
action.addAction(new DoubleClickAction(jsonMouse, null));
}
return click().click();

return clickInTicks(LEFT).clickInTicks(LEFT);
}

/**
Expand All @@ -365,9 +382,9 @@ public Actions doubleClick() {
public Actions moveToElement(WebElement target) {
if (isBuildingActions()) {
action.addAction(new MoveMouseAction(jsonMouse, (Locatable) target));
return this;
}
return moveToElement(target, 0, 0);

return moveInTicks(target, 0, 0);
}

/**
Expand All @@ -383,11 +400,17 @@ public Actions moveToElement(WebElement target) {
public Actions moveToElement(WebElement target, int xOffset, int yOffset) {
if (isBuildingActions()) {
action.addAction(new MoveToOffsetAction(jsonMouse, (Locatable) target, xOffset, yOffset));
return this;
}

// Of course, this is the offset from the centre of the element. We have no idea what the width
// and height are once we execute this method.
LOG.info("When using the W3C Action commands, offsets are from the center of element");
return moveInTicks(target, xOffset, yOffset);
}

private Actions moveInTicks(WebElement target, int xOffset, int yOffset) {
return tick(defaultMouse.createPointerMove(
Duration.ofMillis(250),
Duration.ofMillis(100),
Origin.fromElement(target),
xOffset,
yOffset));
Expand All @@ -406,7 +429,6 @@ public Actions moveToElement(WebElement target, int xOffset, int yOffset) {
public Actions moveByOffset(int xOffset, int yOffset) {
if (isBuildingActions()) {
action.addAction(new MoveToOffsetAction(jsonMouse, null, xOffset, yOffset));
return this;
}

return tick(
Expand All @@ -423,9 +445,8 @@ public Actions moveByOffset(int xOffset, int yOffset) {
public Actions contextClick(WebElement target) {
if (isBuildingActions()) {
action.addAction(new ContextClickAction(jsonMouse, (Locatable) target));
return this;
}
return moveToElement(target).contextClick();
return moveInTicks(target, 0, 0).clickInTicks(RIGHT);
}

/**
Expand All @@ -434,11 +455,10 @@ public Actions contextClick(WebElement target) {
*/
public Actions contextClick() {
if (isBuildingActions()) {
return contextClick(null);
action.addAction(new ContextClickAction(jsonMouse, null));
}

return tick(defaultMouse.createPointerDown(Button.RIGHT.asArg()))
.tick(defaultMouse.createPointerUp(Button.RIGHT.asArg()));
return clickInTicks(RIGHT);
}

/**
Expand All @@ -454,9 +474,12 @@ public Actions dragAndDrop(WebElement source, WebElement target) {
action.addAction(new ClickAndHoldAction(jsonMouse, (Locatable) source));
action.addAction(new MoveMouseAction(jsonMouse, (Locatable) target));
action.addAction(new ButtonReleaseAction(jsonMouse, (Locatable) target));
return this;
}
return clickAndHold(source).moveToElement(target).release();

return moveInTicks(source, 0, 0)
.tick(defaultMouse.createPointerDown(LEFT.asArg()))
.moveInTicks(source, 0, 0)
.tick(defaultMouse.createPointerUp(LEFT.asArg()));
}

/**
Expand All @@ -473,9 +496,12 @@ public Actions dragAndDropBy(WebElement source, int xOffset, int yOffset) {
action.addAction(new ClickAndHoldAction(jsonMouse, (Locatable) source));
action.addAction(new MoveToOffsetAction(jsonMouse, null, xOffset, yOffset));
action.addAction(new ButtonReleaseAction(jsonMouse, null));
return this;
}
return clickAndHold(source).moveByOffset(xOffset, yOffset).release();

return moveInTicks(source, 0, 0)
.tick(defaultMouse.createPointerDown(LEFT.asArg()))
.tick(defaultMouse.createPointerMove(Duration.ofMillis(250), Origin.pointer(), xOffset, yOffset))
.tick(defaultMouse.createPointerUp(LEFT.asArg()));
}

/**
Expand All @@ -490,7 +516,6 @@ public Actions dragAndDropBy(WebElement source, int xOffset, int yOffset) {
public Actions pause(long pause) {
if (isBuildingActions()) {
action.addAction(new PauseAction(pause));
return this;
}

return tick(new Pause(defaultMouse, Duration.ofMillis(pause)));
Expand Down Expand Up @@ -551,7 +576,7 @@ public Actions tick(Action action) {
* @return the composite action
*/
public Action build() {
Action toReturn = new BuiltAction(driver, sequences, action);
Action toReturn = new BuiltAction(driver, ImmutableMap.copyOf(sequences), action);
action = new CompositeAction();
sequences.clear();
return toReturn;
Expand Down

0 comments on commit 30012e4

Please sign in to comment.