[Webkit-unassigned] [Bug 174616] WebDriver: implement advance user interactions

[bugzilla-daemon at webkit.org]

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

Brian Burg <bburg at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #338723|review?                     |review+
              Flags|                            |

--- Comment #5 from Brian Burg <bburg at apple.com> ---
Comment on attachment 338723
  --> https://bugs.webkit.org/attachment.cgi?id=338723
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=338723&action=review

r=me Looks good, but I want to move element-based pointer origin into the protocol rather than doing it driver-side. Do you want to code that up now, or land this as-is and refactor it later?

> Source/WebDriver/ChangeLog:21
> +        (WebDriver::Session::virtualKeyForKeySequence): Add more kay codes includes in the spec.

Nit: key

> Source/WebDriver/Session.cpp:2114
> +        computeInViewCenterPointOfElements(WTFMove(elements), WTFMove(elementOrigins), WTFMove(completionHandler));

Nice.

> Source/WebDriver/Session.cpp:2187
> +                        state->setDouble(ASCIILiteral("duration"), action.duration.value());

Is duration guaranteed to have a value given these preconditions?

> Source/WebDriver/Session.cpp:2200
> +                                currentState.origin.x = action.x.value();

Per our discussion earlier, I think it is incorrect to resolve element locations on the driver side since they may change in the course of a multi-tick interaction. Instead, currentState.origin would have a symbolic x/y if the type is Element. On the WebKit side it would convert that into a layout computation.

> Source/WebDriver/Session.cpp:2203
> +                            case PointerOrigin::Type::Pointer:

This case can be merged with above case.

> Source/WebDriver/Session.cpp:2216
> +                                state->setDouble(ASCIILiteral("duration"), action.duration.value());

This seems like the correct way to do what I called out above re: Action::Type::None.

> Source/WebDriver/WebDriverService.cpp:1834
> +static std::optional<Vector<Action>> processInputActionSequence(Session& session, JSON::Value& actionSequenceValue, std::optional<String>& errorMessage)

I would use return type of

Expected<Vector<Action>, String>

then you can get rid of the errorMessage out parameter.

> Source/WebKit/UIProcess/Automation/SimulatedInputDispatcher.cpp:206
> +            m_client.simulateMouseInteraction(m_page, MouseInteraction::Move, b.pressedMouseButton.value_or(MouseButton::NoButton), b.location.value_or(WebCore::IntPoint()), WTFMove(eventDispatchFinished));

Let's land this separately since it affects safaridriver as well, and don't want to hold it up on the above questions.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180503/71ca86c6/attachment-0001.html>