Fix tap by element coordinates

[mykola-mokhnach]

The same fix as for appium/appium-uiautomator2-driver#163

[imurchie]

I'm confused. Why is the ability to set an element and the offset being removed?

[mykola-mokhnach]

Because such feature is not supported by the bootstrap server itself:

  @Override
  public AndroidCommandResult execute(final AndroidCommand command)
      throws JSONException {
    if (command.isElementCommand()) {
      try {
        final AndroidElement el = command.getElement();
        el.click();
        return getSuccessResult(true);
      } catch (final UiObjectNotFoundException e) {
        return new AndroidCommandResult(WDStatus.NO_SUCH_ELEMENT,
            e.getMessage());
      } catch (final Exception e) { // handle NullPointerException
        return getErrorResult("Unknown error");
      }
    } else {
      final Hashtable<String, Object> params = command.params();
      Point coords = new Point(Double.parseDouble(params.get("x").toString()),
          Double.parseDouble(params.get("y").toString()) );

      try {
        coords = PositionHelper.getDeviceAbsPos(coords);
      } catch (final UiObjectNotFoundException e) {
        return new AndroidCommandResult(WDStatus.NO_SUCH_ELEMENT,
            e.getMessage());
      } catch (final InvalidCoordinatesException e) {
        return new AndroidCommandResult(WDStatus.INVALID_ELEMENT_COORDINATES,
            e.getMessage());
      }

      final boolean res = UiDevice.getInstance().click(coords.x.intValue(), coords.y.intValue());
      return getSuccessResult(res);
    }
  }

As you can see here it can only perform click on element or click by absolute coordinates.

[imurchie]

Ok. That makes sense, then.

[imurchie]

It's still possible to do what we purport to do (element with offset) by converting to absolute coordinates (i.e., finding the coordinates of the element and then adding the offset). Since this is what is purported to be done.

[mykola-mokhnach]

Yes, it is possible. However, in order to achieve this one has to rewrite the logic for touch actions parser, for the bootstrap lib and for this tap command. It could be more error-prone, since many components will be affected (including UIA2 driver) and will take much more time.

[imurchie]

Really? We take the correct arguments now. We could just go, in the driver,

if (elementId && areCoordinatesDefined) {
  const loc = await this.getLocation(elementId);
  const newX = loc.x + x;
  const newY = loc.y + y;
  await this.bootstrap.sendAction("click", {x: newX, y: newY});
} else ...

And, of course, this could be done on the bootstrap side more efficiently.

[mykola-mokhnach]

The coordinates passed to tap are already absolute. Although, I don't mind if you can implement a better fix

[mykola-mokhnach]

@imurchie I think we could merge this solution for now, since it is not ideal, but fixes the original problem, so clients could properly run their tests with Appium@beta. In the meantime one could refactor the code and present something better. Same for appium/appium-uiautomator2-driver#163

[mykola-mokhnach]

@imurchie the code above is already applied in

appium-android-driver/lib/commands/touch.js

Line 196 in 20c9e4a

helpers.parseTouch = async function (gestures, multi) {

method.

[jsf-clabot]

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ mykola-mokhnach
❌ Mykola Mokhnach

---

Mykola Mokhnach seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.