Bug 200729 – Web Automation: mouse buttons are not correctly printed in SimulatedInputDispatcher log spew

• Component: WebDriver
• Status: RESOLVED FIXED

[Blaze Burg]

Oops.

[Blaze Burg]

Created attachment 376290 [details]
Patch - for review

[Blaze Burg]

Created attachment 376291 [details]
Patch - for EWS

[Blaze Burg]

Comment on attachment 376291 [details]
Patch - for EWS

This patch changes the same signature as the patch in https://bugs.webkit.org/show_bug.cgi?id=200728, but I don't want to land these changes together because Reasons. I combined both of them for EWS purposes.

[Devin Rousso]

Comment on attachment 376291 [details]
Patch - for EWS

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

r=me, nice fixes

> Source/WebKit/UIProcess/Automation/WebAutomationSession.h:147
> +    void simulateMouseInteraction(WebPageProxy&, MouseInteraction, MouseButton, const WebCore::IntPoint& locationInView, AutomationCompletionHandler&&) final;

Should this also be `locationInViewport` to match?

> Source/WebKit/UIProcess/Automation/mac/WebAutomationSessionMac.mm:125
> +static WebMouseEvent::Button automationMouseButtonToPlatformMouseButton(MouseButton button)

Is this function really necessary?  The only place it's used is inside yet another switch-case, so couldn't you just have it switch on `MouseButton` there instead?

> Source/WebKit/UIProcess/Automation/mac/WebAutomationSessionMac.mm:132
> +    case MouseButton::Left:   return WebMouseEvent::LeftButton;
> +    case MouseButton::Middle: return WebMouseEvent::MiddleButton;
> +    case MouseButton::Right:  return WebMouseEvent::RightButton;
> +    case MouseButton::None:   return WebMouseEvent::NoButton;
> +    default: ASSERT_NOT_REACHED();

Style: this doesn't look like WebKit style, so please split onto separate lines (like how `protocolMouseButtonToWebMouseEventButton` used to be).

> Source/WebKit/UIProcess/Automation/mac/WebAutomationSessionMac.mm:140
> +    IntPoint locationInView = WebCore::IntPoint(locationInViewport.x(), locationInViewport.y() + page.topContentInset());

NIT: I usually either use `auto` as the type, or don't do an assignment constructor so that you only have to type the type once.
```
    auto locationInView = IntPoint(locationInViewport.x(), locationInViewport.y() + page.topContentInset());
```
or
```
    IntPoint locationInView(locationInViewport.x(), locationInViewport.y() + page.topContentInset());
```

[Blaze Burg]

Created attachment 376335 [details]
For EWS (with GTK fix)

[Blaze Burg]

Committed r248715: <https://trac.webkit.org/changeset/248715>

[Radar WebKit Bug Importer]

<rdar://problem/54348165>