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

[bugzilla-daemon at webkit.org]

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

--- Comment #6 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to Brian Burg from comment #5)
> Comment on attachment 338723 [details]
> 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?

Thanks for the review. I started to move the origin handling to the WebKit side yesterday, but it's more complicated than I expected. I'm not sure I'll be able to continue working on it today. If I see it's going to take long, I'll land this patch and leave the origin for a follow up.

> > 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.

But useless in the end :-(

> > Source/WebDriver/Session.cpp:2187
> > +                        state->setDouble(ASCIILiteral("duration"), action.duration.value());
> 
> Is duration guaranteed to have a value given these preconditions?

Yes.

> > 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.

Indeed.

> > 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.

Ok, it makes sense.

-- 
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/20180504/69b563ea/attachment-0001.html>