Oops.
Created attachment 376290 [details] Patch - for review
Created attachment 376291 [details] Patch - for EWS
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.
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()); ```
Created attachment 376335 [details] For EWS (with GTK fix)
Committed r248715: <https://trac.webkit.org/changeset/248715>
<rdar://problem/54348165>