Web Automation: elements larger than the viewport have incorrect in-view center point
authordrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 15 May 2019 08:37:02 +0000 (08:37 +0000)
committerdrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 15 May 2019 08:37:02 +0000 (08:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195696
<rdar://problem/48737122>

Reviewed by Simon Fraser.

Original patch by Brian Burg <bburg@apple.com>.

Source/WebCore:

Some conversion methods do not exist for `FloatRect`/`FloatPoint`. Fill them in as needed,
and export some symbols used by WebDriver code to compute an element's in-view center point
in various coordinate systems.

* dom/TreeScope.h:
* dom/TreeScope.cpp:
(WebCore::TreeScope::elementsFromPoint): Added.
* page/FrameView.h:
* page/FrameView.cpp:
(WebCore::FrameView::absoluteToLayoutViewportPoint const): Added.
(WebCore::FrameView::layoutViewportToAbsoluteRect const): Added.
(WebCore::FrameView::absoluteToLayoutViewportRect const): Added.
* platform/ScrollView.h:
* platform/ScrollView.cpp:
(WebCore::ScrollView::viewToContents const): Added.
(WebCore::ScrollView::contentsToView const): Added.
(WebCore::ScrollView::contentsToRootView const): Added.
* platform/Widget.h:
* platform/Widget.cpp:
(WebCore::Widget::convertToRootView const): Added.
(WebCore::Widget::convertFromRootView const): Added.
(WebCore::Widget::convertToContainingView const): Added.
(WebCore::Widget::convertFromContainingView const): Added.

Source/WebKit:

This seems to be an omission in the specification. While it does mention that the in-view
center point (IVCP) must be within the viewport, the algorithm never intersects the element
bounding box with the viewport rect.

* WebProcess/Automation/WebAutomationSessionProxy.cpp:
(WebKit::WebAutomationSessionProxy::computeElementLayout):
This code is incorrect. For `CoordinateSystem::LayoutViewport`, coordinates should be in
root view coordinates so that it can be later converted to screen and synthesized as a HID
event in screen coordinates. Intersect the element rect and the viewport rect before finding
the center point of the part of the element that's visible in the viewport.

(WebKit::convertRectFromFrameClientToRootView): Added.
(WebKit::convertPointFromFrameClientToRootView): Added.
Added helpers to properly account for scroll contents position on iOS.

* UIProcess/Automation/WebAutomationSession.cpp:
(WebKit::WebAutomationSession::viewportInViewCenterPointOfElement):
Now that we determine whether the element is inside the viewport much earlier, if the
element has no `inViewCenterPoint`, we can return a `TargetOutOfBounds` instead of a more
"generic" `ElementNotInteractable`.

(WebKit::WebAutomationSession::simulateMouseInteraction):
Rename `locationInView` -> `locationInViewport`.

(WebKit::WebAutomationSession::simulateTouchInteraction):
This code is incorrect. The `unobscuredContentRect` is in screen coordinates, but
we are trying to see if (x, y) is outside the size of the viewport assumed to be at (0, 0).
Grab the visual viewport rect and see if the location exceeds the viewport size.

* UIProcess/Automation/ios/WebAutomationSessionIOS.mm:
(WebKit::operator<<):
Add logging helper for `TouchInteraction` enum.

(WebKit::WebAutomationSession::platformSimulateTouchInteraction):
Move local variable.

* UIProcess/Automation/SimulatedInputDispatcher.cpp:
(WebKit::SimulatedInputDispatcher::transitionInputSourceToState):
Fix a typo in logging.

* UIProcess/Automation/Automation.json:
Simplify enum name.

* Platform/Logging.h:
Add logging channel to dump fully resolved interaction details.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@245320 268f45cc-cd09-0410-ab3c-d52691b4dbfc

16 files changed:
Source/WebCore/ChangeLog
Source/WebCore/dom/TreeScope.cpp
Source/WebCore/dom/TreeScope.h
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/FrameView.h
Source/WebCore/platform/ScrollView.cpp
Source/WebCore/platform/ScrollView.h
Source/WebCore/platform/Widget.cpp
Source/WebCore/platform/Widget.h
Source/WebKit/ChangeLog
Source/WebKit/Platform/Logging.h
Source/WebKit/UIProcess/Automation/Automation.json
Source/WebKit/UIProcess/Automation/SimulatedInputDispatcher.cpp
Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp
Source/WebKit/UIProcess/Automation/ios/WebAutomationSessionIOS.mm
Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp

index 99be118..debe028 100644 (file)
@@ -1,3 +1,37 @@
+2019-05-15  Devin Rousso  <drousso@apple.com>
+
+        Web Automation: elements larger than the viewport have incorrect in-view center point
+        https://bugs.webkit.org/show_bug.cgi?id=195696
+        <rdar://problem/48737122>
+
+        Reviewed by Simon Fraser.
+
+        Original patch by Brian Burg <bburg@apple.com>.
+
+        Some conversion methods do not exist for `FloatRect`/`FloatPoint`. Fill them in as needed,
+        and export some symbols used by WebDriver code to compute an element's in-view center point
+        in various coordinate systems.
+
+        * dom/TreeScope.h:
+        * dom/TreeScope.cpp:
+        (WebCore::TreeScope::elementsFromPoint): Added.
+        * page/FrameView.h:
+        * page/FrameView.cpp:
+        (WebCore::FrameView::absoluteToLayoutViewportPoint const): Added.
+        (WebCore::FrameView::layoutViewportToAbsoluteRect const): Added.
+        (WebCore::FrameView::absoluteToLayoutViewportRect const): Added.
+        * platform/ScrollView.h:
+        * platform/ScrollView.cpp:
+        (WebCore::ScrollView::viewToContents const): Added.
+        (WebCore::ScrollView::contentsToView const): Added.
+        (WebCore::ScrollView::contentsToRootView const): Added.
+        * platform/Widget.h:
+        * platform/Widget.cpp:
+        (WebCore::Widget::convertToRootView const): Added.
+        (WebCore::Widget::convertFromRootView const): Added.
+        (WebCore::Widget::convertToContainingView const): Added.
+        (WebCore::Widget::convertFromContainingView const): Added.
+
 2019-05-14  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         Missing cursor/caret showing in search field on google.com
index 7ba3578..cf9276c 100644 (file)
@@ -441,6 +441,11 @@ Vector<RefPtr<Element>> TreeScope::elementsFromPoint(double clientX, double clie
     return elements;
 }
 
+Vector<RefPtr<Element>> TreeScope::elementsFromPoint(const FloatPoint& p)
+{
+    return elementsFromPoint(p.x(), p.y());
+}
+
 Element* TreeScope::findAnchor(const String& name)
 {
     if (name.isEmpty())
index b164bd9..efb4425 100644 (file)
@@ -37,6 +37,7 @@ namespace WebCore {
 class ContainerNode;
 class Document;
 class Element;
+class FloatPoint;
 class HTMLImageElement;
 class HTMLLabelElement;
 class HTMLMapElement;
@@ -95,6 +96,7 @@ public:
 
     WEBCORE_EXPORT RefPtr<Element> elementFromPoint(double clientX, double clientY);
     WEBCORE_EXPORT Vector<RefPtr<Element>> elementsFromPoint(double clientX, double clientY);
+    WEBCORE_EXPORT Vector<RefPtr<Element>> elementsFromPoint(const FloatPoint&);
 
     // Find first anchor with the given name.
     // First searches for an element with the given ID, but if that fails, then looks
index 50813d8..10016c2 100644 (file)
@@ -4791,6 +4791,14 @@ FloatPoint FrameView::clientToDocumentPoint(FloatPoint point) const
     return point;
 }
 
+FloatPoint FrameView::absoluteToLayoutViewportPoint(FloatPoint p) const
+{
+    ASSERT(frame().settings().visualViewportEnabled());
+    p.scale(1 / frame().frameScaleFactor());
+    p.moveBy(-layoutViewportRect().location());
+    return p;
+}
+
 FloatPoint FrameView::layoutViewportToAbsolutePoint(FloatPoint p) const
 {
     ASSERT(frame().settings().visualViewportEnabled());
@@ -4798,6 +4806,22 @@ FloatPoint FrameView::layoutViewportToAbsolutePoint(FloatPoint p) const
     return p.scaled(frame().frameScaleFactor());
 }
 
+FloatRect FrameView::layoutViewportToAbsoluteRect(FloatRect rect) const
+{
+    ASSERT(frame().settings().visualViewportEnabled());
+    rect.moveBy(layoutViewportRect().location());
+    rect.scale(frame().frameScaleFactor());
+    return rect;
+}
+
+FloatRect FrameView::absoluteToLayoutViewportRect(FloatRect rect) const
+{
+    ASSERT(frame().settings().visualViewportEnabled());
+    rect.scale(1 / frame().frameScaleFactor());
+    rect.moveBy(-layoutViewportRect().location());
+    return rect;
+}
+
 FloatRect FrameView::clientToLayoutViewportRect(FloatRect rect) const
 {
     ASSERT(frame().settings().visualViewportEnabled());
index 81d1cc6..d36e565 100644 (file)
@@ -475,19 +475,23 @@ public:
     float documentToAbsoluteScaleFactor(Optional<float> effectiveZoom = WTF::nullopt) const;
     float absoluteToDocumentScaleFactor(Optional<float> effectiveZoom = WTF::nullopt) const;
 
-    FloatRect absoluteToDocumentRect(FloatRect, Optional<float> effectiveZoom = WTF::nullopt) const;
-    FloatPoint absoluteToDocumentPoint(FloatPoint, Optional<float> effectiveZoom = WTF::nullopt) const;
+    WEBCORE_EXPORT FloatRect absoluteToDocumentRect(FloatRect, Optional<float> effectiveZoom = WTF::nullopt) const;
+    WEBCORE_EXPORT FloatPoint absoluteToDocumentPoint(FloatPoint, Optional<float> effectiveZoom = WTF::nullopt) const;
 
     FloatRect absoluteToClientRect(FloatRect, Optional<float> effectiveZoom = WTF::nullopt) const;
 
     FloatSize documentToClientOffset() const;
-    FloatRect documentToClientRect(FloatRect) const;
+    WEBCORE_EXPORT FloatRect documentToClientRect(FloatRect) const;
     FloatPoint documentToClientPoint(FloatPoint) const;
     WEBCORE_EXPORT FloatRect clientToDocumentRect(FloatRect) const;
     WEBCORE_EXPORT FloatPoint clientToDocumentPoint(FloatPoint) const;
 
+    WEBCORE_EXPORT FloatPoint absoluteToLayoutViewportPoint(FloatPoint) const;
     FloatPoint layoutViewportToAbsolutePoint(FloatPoint) const;
 
+    WEBCORE_EXPORT FloatRect absoluteToLayoutViewportRect(FloatRect) const;
+    FloatRect layoutViewportToAbsoluteRect(FloatRect) const;
+
     // Unlike client coordinates, layout viewport coordinates are affected by page zoom.
     WEBCORE_EXPORT FloatRect clientToLayoutViewportRect(FloatRect) const;
     WEBCORE_EXPORT FloatPoint clientToLayoutViewportPoint(FloatPoint) const;
index 255ff67..61bb518 100644 (file)
@@ -842,6 +842,22 @@ IntPoint ScrollView::contentsToView(const IntPoint& point) const
     return point - toIntSize(documentScrollPositionRelativeToViewOrigin());
 }
 
+FloatPoint ScrollView::viewToContents(const FloatPoint& point) const
+{
+    if (delegatesScrolling())
+        return point;
+
+    return viewToContents(IntPoint(point));
+}
+
+FloatPoint ScrollView::contentsToView(const FloatPoint& point) const
+{
+    if (delegatesScrolling())
+        return point;
+
+    return contentsToView(IntPoint(point));
+}
+
 IntRect ScrollView::viewToContents(IntRect rect) const
 {
     if (delegatesScrolling())
@@ -908,6 +924,11 @@ IntPoint ScrollView::contentsToRootView(const IntPoint& contentsPoint) const
     return convertToRootView(contentsToView(contentsPoint));
 }
 
+FloatPoint ScrollView::contentsToRootView(const FloatPoint& contentsPoint) const
+{
+    return convertToRootView(contentsToView(contentsPoint));
+}
+
 IntRect ScrollView::rootViewToContents(const IntRect& rootViewRect) const
 {
     return viewToContents(convertFromRootView(rootViewRect));
@@ -918,6 +939,11 @@ FloatRect ScrollView::rootViewToContents(const FloatRect& rootViewRect) const
     return viewToContents(convertFromRootView(rootViewRect));
 }
 
+FloatRect ScrollView::contentsToRootView(const FloatRect& contentsRect) const
+{
+    return convertToRootView(contentsToView(contentsRect));
+}
+
 IntPoint ScrollView::rootViewToTotalContents(const IntPoint& rootViewPoint) const
 {
     if (delegatesScrolling())
index 8eb4683..7d824c4 100644 (file)
@@ -230,7 +230,7 @@ public:
     int scrollY() const { return scrollPosition().y(); }
 
     // Scroll position used by web-exposed features (has legacy iOS behavior).
-    IntPoint contentsScrollPosition() const;
+    WEBCORE_EXPORT IntPoint contentsScrollPosition() const;
     void setContentsScrollPosition(const IntPoint&);
 
 #if PLATFORM(IOS_FAMILY)
@@ -279,13 +279,18 @@ public:
 
     WEBCORE_EXPORT IntPoint rootViewToContents(const IntPoint&) const;
     WEBCORE_EXPORT IntPoint contentsToRootView(const IntPoint&) const;
+    WEBCORE_EXPORT FloatPoint contentsToRootView(const FloatPoint&) const;
     WEBCORE_EXPORT IntRect rootViewToContents(const IntRect&) const;
     WEBCORE_EXPORT IntRect contentsToRootView(const IntRect&) const;
     WEBCORE_EXPORT FloatRect rootViewToContents(const FloatRect&) const;
+    WEBCORE_EXPORT FloatRect contentsToRootView(const FloatRect&) const;
 
     IntPoint viewToContents(const IntPoint&) const;
     IntPoint contentsToView(const IntPoint&) const;
 
+    FloatPoint viewToContents(const FloatPoint&) const;
+    FloatPoint contentsToView(const FloatPoint&) const;
+
     IntRect viewToContents(IntRect) const;
     IntRect contentsToView(IntRect) const;
 
index fc941f4..48570aa 100644 (file)
@@ -95,6 +95,15 @@ IntRect Widget::convertToRootView(const IntRect& localRect) const
     return localRect;
 }
 
+FloatRect Widget::convertToRootView(const FloatRect& localRect) const
+{
+    if (const ScrollView* parentScrollView = parent()) {
+        FloatRect parentRect = convertToContainingView(localRect);
+        return parentScrollView->convertToRootView(parentRect);
+    }
+    return localRect;
+}
+
 IntPoint Widget::convertFromRootView(const IntPoint& rootPoint) const
 {
     if (const ScrollView* parentScrollView = parent()) {
@@ -113,6 +122,25 @@ IntPoint Widget::convertToRootView(const IntPoint& localPoint) const
     return localPoint;
 }
 
+
+FloatPoint Widget::convertFromRootView(const FloatPoint& rootPoint) const
+{
+    if (const ScrollView* parentScrollView = parent()) {
+        FloatPoint parentPoint = parentScrollView->convertFromRootView(rootPoint);
+        return convertFromContainingView(parentPoint);
+    }
+    return rootPoint;
+}
+
+FloatPoint Widget::convertToRootView(const FloatPoint& localPoint) const
+{
+    if (const ScrollView* parentScrollView = parent()) {
+        FloatPoint parentPoint = convertToContainingView(localPoint);
+        return parentScrollView->convertToRootView(parentPoint);
+    }
+    return localPoint;
+}
+
 IntRect Widget::convertFromContainingWindow(const IntRect& windowRect) const
 {
     if (const ScrollView* parentScrollView = parent()) {
@@ -204,6 +232,11 @@ IntRect Widget::convertFromContainingView(const IntRect& parentRect) const
     return parentRect;
 }
 
+FloatRect Widget::convertToContainingView(const FloatRect& localRect) const
+{
+    return convertToContainingView(IntRect(localRect));
+}
+
 FloatRect Widget::convertFromContainingView(const FloatRect& parentRect) const
 {
     return convertFromContainingView(IntRect(parentRect));
@@ -225,6 +258,16 @@ IntPoint Widget::convertFromContainingView(const IntPoint& parentPoint) const
     return parentPoint;
 }
 
+FloatPoint Widget::convertToContainingView(const FloatPoint& localPoint) const
+{
+    return convertToContainingView(IntPoint(localPoint));
+}
+
+FloatPoint Widget::convertFromContainingView(const FloatPoint& parentPoint) const
+{
+    return convertFromContainingView(IntPoint(parentPoint));
+}
+
 #if !PLATFORM(COCOA) && !PLATFORM(GTK) && !PLATFORM(WIN)
 
 Widget::~Widget()
index 2111265..2aeaeab 100644 (file)
@@ -150,11 +150,15 @@ public:
     WEBCORE_EXPORT IntRect convertToRootView(const IntRect&) const;
     IntRect convertFromRootView(const IntRect&) const;
 
+    FloatRect convertToRootView(const FloatRect&) const;
     FloatRect convertFromRootView(const FloatRect&) const;
 
     IntPoint convertToRootView(const IntPoint&) const;
     IntPoint convertFromRootView(const IntPoint&) const;
 
+    FloatPoint convertToRootView(const FloatPoint&) const;
+    FloatPoint convertFromRootView(const FloatPoint&) const;
+
     // It is important for cross-platform code to realize that Mac has flipped coordinates.  Therefore any code
     // that tries to convert the location of a rect using the point-based convertFromContainingWindow will end
     // up with an inaccurate rect.  Always make sure to use the rect-based convertFromContainingWindow method
@@ -186,9 +190,12 @@ public:
     // Virtual methods to convert points to/from the containing ScrollView
     WEBCORE_EXPORT virtual IntRect convertToContainingView(const IntRect&) const;
     WEBCORE_EXPORT virtual IntRect convertFromContainingView(const IntRect&) const;
+    WEBCORE_EXPORT virtual FloatRect convertToContainingView(const FloatRect&) const;
     WEBCORE_EXPORT virtual FloatRect convertFromContainingView(const FloatRect&) const;
     WEBCORE_EXPORT virtual IntPoint convertToContainingView(const IntPoint&) const;
     WEBCORE_EXPORT virtual IntPoint convertFromContainingView(const IntPoint&) const;
+    WEBCORE_EXPORT virtual FloatPoint convertToContainingView(const FloatPoint&) const;
+    WEBCORE_EXPORT virtual FloatPoint convertFromContainingView(const FloatPoint&) const;
 
 private:
     void init(PlatformWidget); // Must be called by all Widget constructors to initialize cross-platform data.
index 44ebe14..5df2e3d 100644 (file)
@@ -1,3 +1,59 @@
+2019-05-15  Devin Rousso  <drousso@apple.com>
+
+        Web Automation: elements larger than the viewport have incorrect in-view center point
+        https://bugs.webkit.org/show_bug.cgi?id=195696
+        <rdar://problem/48737122>
+
+        Reviewed by Simon Fraser.
+
+        Original patch by Brian Burg <bburg@apple.com>.
+
+        This seems to be an omission in the specification. While it does mention that the in-view
+        center point (IVCP) must be within the viewport, the algorithm never intersects the element
+        bounding box with the viewport rect.
+
+        * WebProcess/Automation/WebAutomationSessionProxy.cpp:
+        (WebKit::WebAutomationSessionProxy::computeElementLayout):
+        This code is incorrect. For `CoordinateSystem::LayoutViewport`, coordinates should be in
+        root view coordinates so that it can be later converted to screen and synthesized as a HID
+        event in screen coordinates. Intersect the element rect and the viewport rect before finding
+        the center point of the part of the element that's visible in the viewport.
+
+        (WebKit::convertRectFromFrameClientToRootView): Added.
+        (WebKit::convertPointFromFrameClientToRootView): Added.
+        Added helpers to properly account for scroll contents position on iOS.
+
+        * UIProcess/Automation/WebAutomationSession.cpp:
+        (WebKit::WebAutomationSession::viewportInViewCenterPointOfElement):
+        Now that we determine whether the element is inside the viewport much earlier, if the
+        element has no `inViewCenterPoint`, we can return a `TargetOutOfBounds` instead of a more
+        "generic" `ElementNotInteractable`.
+
+        (WebKit::WebAutomationSession::simulateMouseInteraction):
+        Rename `locationInView` -> `locationInViewport`.
+
+        (WebKit::WebAutomationSession::simulateTouchInteraction):
+        This code is incorrect. The `unobscuredContentRect` is in screen coordinates, but
+        we are trying to see if (x, y) is outside the size of the viewport assumed to be at (0, 0).
+        Grab the visual viewport rect and see if the location exceeds the viewport size.
+
+        * UIProcess/Automation/ios/WebAutomationSessionIOS.mm:
+        (WebKit::operator<<):
+        Add logging helper for `TouchInteraction` enum.
+
+        (WebKit::WebAutomationSession::platformSimulateTouchInteraction):
+        Move local variable.
+
+        * UIProcess/Automation/SimulatedInputDispatcher.cpp:
+        (WebKit::SimulatedInputDispatcher::transitionInputSourceToState):
+        Fix a typo in logging.
+
+        * UIProcess/Automation/Automation.json:
+        Simplify enum name.
+
+        * Platform/Logging.h:
+        Add logging channel to dump fully resolved interaction details.
+
 2019-05-14  Ross Kirsling  <ross.kirsling@sony.com>
 
         Unreviewed restoration of non-unified build.
index 1f773a7..4b8a6d2 100644 (file)
@@ -42,6 +42,7 @@ extern "C" {
 #define WEBKIT2_LOG_CHANNELS(M) \
     M(AdClickAttribution) \
     M(Automation) \
+    M(AutomationInteractions) \
     M(ActivityState) \
     M(BackForward) \
     M(CacheStorage) \
index cad53c7..9130f0e 100644 (file)
@@ -32,7 +32,7 @@
             "description": "The coordinate system in which rects, points, and sizes are to be interpreted.",
             "enum": [
                 "Page",
-                "LayoutViewport"
+                "Viewport"
             ]
         },
         {
index 26a74e9..7acc68b 100644 (file)
@@ -290,7 +290,7 @@ void SimulatedInputDispatcher::transitionInputSourceToState(SimulatedInputSource
             } else if (a.pressedMouseButton && !b.pressedMouseButton) {
 #if !LOG_DISABLED
                 String mouseButtonName = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(a.pressedMouseButton.value());
-                LOG(Automation, "SimulatedInputDispatcher[%p]: simulating MouseUp[button=%s] @ (%d, %d) for transition to %d.%d", this, mouseButtonName.utf8().data(), a.location.value().x(), a.location.value().y(), m_keyframeIndex, m_inputSourceStateIndex);
+                LOG(Automation, "SimulatedInputDispatcher[%p]: simulating MouseUp[button=%s] @ (%d, %d) for transition to %d.%d", this, mouseButtonName.utf8().data(), b.location.value().x(), b.location.value().y(), m_keyframeIndex, m_inputSourceStateIndex);
 #endif
                 m_client.simulateMouseInteraction(m_page, MouseInteraction::Up, a.pressedMouseButton.value(), b.location.value(), WTFMove(eventDispatchFinished));
             } else if (a.location != b.location) {
@@ -324,7 +324,7 @@ void SimulatedInputDispatcher::transitionInputSourceToState(SimulatedInputSource
                 LOG(Automation, "SimulatedInputDispatcher[%p]: simulating TouchDown @ (%d, %d) for transition to %d.%d", this, b.location.value().x(), b.location.value().y(), m_keyframeIndex, m_inputSourceStateIndex);
                 m_client.simulateTouchInteraction(m_page, TouchInteraction::TouchDown, b.location.value(), WTF::nullopt, WTFMove(eventDispatchFinished));
             } else if (a.pressedMouseButton && !b.pressedMouseButton) {
-                LOG(Automation, "SimulatedInputDispatcher[%p]: simulating LiftUp @ (%d, %d) for transition to %d.%d", this, a.location.value().x(), a.location.value().y(), m_keyframeIndex, m_inputSourceStateIndex);
+                LOG(Automation, "SimulatedInputDispatcher[%p]: simulating LiftUp @ (%d, %d) for transition to %d.%d", this, b.location.value().x(), b.location.value().y(), m_keyframeIndex, m_inputSourceStateIndex);
                 m_client.simulateTouchInteraction(m_page, TouchInteraction::LiftUp, b.location.value(), WTF::nullopt, WTFMove(eventDispatchFinished));
             } else if (a.location != b.location) {
                 LOG(Automation, "SimulatedInputDispatcher[%p]: simulating MoveTo from (%d, %d) to (%d, %d) for transition to %d.%d", this, a.location.value().x(), a.location.value().y(), b.location.value().x(), b.location.value().y(), m_keyframeIndex, m_inputSourceStateIndex);
index c25b951..1075781 100644 (file)
@@ -1464,7 +1464,7 @@ void WebAutomationSession::viewportInViewCenterPointOfElement(WebPageProxy& page
         }
 
         if (!inViewCenterPoint) {
-            completionHandler(WTF::nullopt, AUTOMATION_COMMAND_ERROR_WITH_NAME(ElementNotInteractable));
+            completionHandler(WTF::nullopt, AUTOMATION_COMMAND_ERROR_WITH_NAME(TargetOutOfBounds));
             return;
         }
 
@@ -1477,11 +1477,10 @@ void WebAutomationSession::viewportInViewCenterPointOfElement(WebPageProxy& page
 #if ENABLE(WEBDRIVER_MOUSE_INTERACTIONS)
 void WebAutomationSession::simulateMouseInteraction(WebPageProxy& page, MouseInteraction interaction, WebMouseEvent::Button mouseButton, const WebCore::IntPoint& locationInViewport, CompletionHandler<void(Optional<AutomationCommandError>)>&& completionHandler)
 {
-    WebCore::IntPoint locationInView = WebCore::IntPoint(locationInViewport.x(), locationInViewport.y() + page.topContentInset());
-    page.getWindowFrameWithCallback([this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler), page = makeRef(page), interaction, mouseButton, locationInView](WebCore::FloatRect windowFrame) mutable {
-        auto clippedX = std::min(std::max(0.0f, (float)locationInView.x()), windowFrame.size().width());
-        auto clippedY = std::min(std::max(0.0f, (float)locationInView.y()), windowFrame.size().height());
-        if (clippedX != locationInView.x() || clippedY != locationInView.y()) {
+    page.getWindowFrameWithCallback([this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler), page = makeRef(page), interaction, mouseButton, locationInViewport](WebCore::FloatRect windowFrame) mutable {
+        auto clippedX = std::min(std::max(0.0f, (float)locationInViewport.x()), windowFrame.size().width());
+        auto clippedY = std::min(std::max(0.0f, (float)locationInViewport.y()), windowFrame.size().height());
+        if (clippedX != locationInViewport.x() || clippedY != locationInViewport.y()) {
             completionHandler(AUTOMATION_COMMAND_ERROR_WITH_NAME(TargetOutOfBounds));
             return;
         }
@@ -1496,7 +1495,7 @@ void WebAutomationSession::simulateMouseInteraction(WebPageProxy& page, MouseInt
             callbackInMap(AUTOMATION_COMMAND_ERROR_WITH_NAME(Timeout));
         callbackInMap = WTFMove(mouseEventsFlushedCallback);
 
-        platformSimulateMouseInteraction(page, interaction, mouseButton, locationInView, OptionSet<WebEvent::Modifier>::fromRaw(m_currentModifiers));
+        platformSimulateMouseInteraction(page, interaction, mouseButton, locationInViewport, OptionSet<WebEvent::Modifier>::fromRaw(m_currentModifiers));
 
         // If the event does not hit test anything in the window, then it may not have been delivered.
         if (callbackInMap && !page->isProcessingMouseEvents()) {
@@ -1513,7 +1512,8 @@ void WebAutomationSession::simulateMouseInteraction(WebPageProxy& page, MouseInt
 void WebAutomationSession::simulateTouchInteraction(WebPageProxy& page, TouchInteraction interaction, const WebCore::IntPoint& locationInViewport, Optional<Seconds> duration, CompletionHandler<void(Optional<AutomationCommandError>)>&& completionHandler)
 {
 #if PLATFORM(IOS_FAMILY)
-    if (!page.unobscuredContentRect().contains(locationInViewport)) {
+    WebCore::FloatRect visualViewportBounds = WebCore::FloatRect({ }, page.unobscuredContentRect().size());
+    if (!visualViewportBounds.contains(locationInViewport)) {
         completionHandler(AUTOMATION_COMMAND_ERROR_WITH_NAME(TargetOutOfBounds));
         return;
     }
index 3c5cad8..7228aff 100644 (file)
@@ -28,6 +28,7 @@
 
 #if PLATFORM(IOS_FAMILY)
 
+#import "Logging.h"
 #import "NativeWebKeyboardEvent.h"
 #import "WebAutomationSessionMacros.h"
 #import "WebPageProxy.h"
@@ -175,15 +176,34 @@ void WebAutomationSession::platformSimulateKeySequence(WebPageProxy& page, const
 #endif // ENABLE(WEBDRIVER_KEYBOARD_INTERACTIONS)
 
 #if ENABLE(WEBDRIVER_TOUCH_INTERACTIONS)
+#if !LOG_DISABLED
+static TextStream& operator<<(TextStream& ts, TouchInteraction interaction)
+{
+    switch (interaction) {
+    case TouchInteraction::TouchDown:
+        ts << "TouchDown";
+        break;
+    case TouchInteraction::MoveTo:
+        ts << "MoveTo";
+        break;
+    case TouchInteraction::LiftUp:
+        ts << "LiftUp";
+        break;
+    }
+    return ts;
+}
+#endif // !LOG_DISABLED
+
 void WebAutomationSession::platformSimulateTouchInteraction(WebPageProxy& page, TouchInteraction interaction, const WebCore::IntPoint& locationInViewport, Optional<Seconds> duration, AutomationCompletionHandler&& completionHandler)
 {
     WebCore::IntPoint locationOnScreen = page.syncRootViewToScreen(IntRect(locationInViewport, IntSize())).location();
-    _WKTouchEventGenerator *generator = [_WKTouchEventGenerator sharedTouchEventGenerator];
+    LOG_WITH_STREAM(AutomationInteractions, stream << "platformSimulateTouchInteraction: interaction=" << interaction << ", locationInViewport=" << locationInViewport << ", locationOnScreen=" << locationOnScreen << ", duration=" << duration.valueOr(0_s).seconds());
 
     auto interactionFinished = makeBlockPtr([completionHandler = WTFMove(completionHandler)] () mutable {
         completionHandler(WTF::nullopt);
     });
-    
+
+    _WKTouchEventGenerator *generator = [_WKTouchEventGenerator sharedTouchEventGenerator];
     switch (interaction) {
     case TouchInteraction::TouchDown:
         [generator touchDown:locationOnScreen completionBlock:interactionFinished.get()];
index 9c31b82..9b3b297 100644 (file)
@@ -477,37 +477,6 @@ void WebAutomationSessionProxy::focusFrame(uint64_t pageID, uint64_t frameID)
     coreDOMWindow->focus(true);
 }
 
-static Optional<WebCore::FloatPoint> elementInViewClientCenterPoint(WebCore::Element& element, bool& isObscured)
-{
-    // §11.1 Element Interactability.
-    // https://www.w3.org/TR/webdriver/#dfn-in-view-center-point
-    auto* clientRect = element.getClientRects()->item(0);
-    if (!clientRect)
-        return WTF::nullopt;
-
-    auto clientCenterPoint = WebCore::FloatPoint::narrowPrecision(0.5 * (clientRect->left() + clientRect->right()), 0.5 * (clientRect->top() + clientRect->bottom()));
-    auto elementList = element.treeScope().elementsFromPoint(clientCenterPoint.x(), clientCenterPoint.y());
-    if (elementList.isEmpty()) {
-        // An element is obscured if the pointer-interactable paint tree at its center point is empty,
-        // or the first element in this tree is not an inclusive descendant of itself.
-        // https://w3c.github.io/webdriver/webdriver-spec.html#dfn-obscured
-        isObscured = true;
-        return clientCenterPoint;
-    }
-
-    auto index = elementList.findMatching([&element] (auto& item) { return item.get() == &element; });
-    if (index == notFound)
-        return WTF::nullopt;
-
-    if (index) {
-        // Element is not the first one in the list.
-        auto firstElement = elementList[0];
-        isObscured = !firstElement->isDescendantOf(element);
-    }
-
-    return clientCenterPoint;
-}
-
 static WebCore::Element* containerElementForElement(WebCore::Element& element)
 {
     // §13. Element State.
@@ -534,6 +503,30 @@ static WebCore::Element* containerElementForElement(WebCore::Element& element)
     return &element;
 }
 
+static WebCore::FloatRect convertRectFromFrameClientToRootView(FrameView* frameView, WebCore::FloatRect clientRect)
+{
+    if (!frameView->delegatesScrolling())
+        return frameView->contentsToRootView(frameView->clientToDocumentRect(clientRect));
+
+    // If the frame delegates scrolling, contentsToRootView doesn't take into account scroll/zoom/scale.
+    auto& frame = frameView->frame();
+    clientRect.scale(frame.pageZoomFactor() * frame.frameScaleFactor());
+    clientRect.moveBy(frameView->contentsScrollPosition());
+    return clientRect;
+}
+
+static WebCore::FloatPoint convertPointFromFrameClientToRootView(FrameView* frameView, WebCore::FloatPoint clientPoint)
+{
+    if (!frameView->delegatesScrolling())
+        return frameView->contentsToRootView(frameView->clientToDocumentPoint(clientPoint));
+
+    // If the frame delegates scrolling, contentsToRootView doesn't take into account scroll/zoom/scale.
+    auto& frame = frameView->frame();
+    clientPoint.scale(frame.pageZoomFactor() * frame.frameScaleFactor());
+    clientPoint.moveBy(frameView->contentsScrollPosition());
+    return clientPoint;
+}
+
 void WebAutomationSessionProxy::computeElementLayout(uint64_t pageID, uint64_t frameID, String nodeHandle, bool scrollIntoViewIfNeeded, CoordinateSystem coordinateSystem, CompletionHandler<void(Optional<String>, WebCore::IntRect, Optional<WebCore::IntPoint>, bool)>&& completionHandler)
 {
     WebPage* page = WebProcess::singleton().webPage(pageID);
@@ -543,18 +536,16 @@ void WebAutomationSessionProxy::computeElementLayout(uint64_t pageID, uint64_t f
         return;
     }
 
-    String frameNotFoundErrorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::FrameNotFound);
-    String nodeNotFoundErrorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::NodeNotFound);
-    String notImplementedErrorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::NotImplemented);
-
     WebFrame* frame = frameID ? WebProcess::singleton().webFrame(frameID) : page->mainWebFrame();
     if (!frame || !frame->coreFrame() || !frame->coreFrame()->view()) {
+        String frameNotFoundErrorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::FrameNotFound);
         completionHandler(frameNotFoundErrorType, { }, WTF::nullopt, false);
         return;
     }
 
     WebCore::Element* coreElement = elementForNodeHandle(*frame, nodeHandle);
     if (!coreElement) {
+        String nodeNotFoundErrorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::NodeNotFound);
         completionHandler(nodeNotFoundErrorType, { }, WTF::nullopt, false);
         return;
     }
@@ -569,35 +560,76 @@ void WebAutomationSessionProxy::computeElementLayout(uint64_t pageID, uint64_t f
 
     WebCore::FrameView* frameView = frame->coreFrame()->view();
     WebCore::FrameView* mainView = frame->coreFrame()->mainFrame().view();
-    WebCore::IntRect frameElementBounds = roundedIntRect(coreElement->boundingClientRect());
-    WebCore::IntRect rootElementBounds = mainView->rootViewToContents(frameView->contentsToRootView(frameElementBounds));
+
     WebCore::IntRect resultElementBounds;
+    Optional<WebCore::IntPoint> resultInViewCenterPoint;
+    bool isObscured = false;
+
+    auto elementBoundsInRootCoordinates = convertRectFromFrameClientToRootView(frameView, coreElement->boundingClientRect());
     switch (coordinateSystem) {
     case CoordinateSystem::Page:
-        resultElementBounds = WebCore::IntRect(mainView->clientToDocumentRect(WebCore::FloatRect(rootElementBounds)));
+        resultElementBounds = enclosingIntRect(mainView->absoluteToDocumentRect(mainView->rootViewToContents(elementBoundsInRootCoordinates)));
         break;
     case CoordinateSystem::LayoutViewport:
-        // The element bounds are already in client coordinates.
-        resultElementBounds = WebCore::IntRect(mainView->clientToLayoutViewportRect(WebCore::FloatRect(rootElementBounds)));
+        resultElementBounds = enclosingIntRect(mainView->absoluteToLayoutViewportRect(elementBoundsInRootCoordinates));
         break;
     }
 
-    Optional<WebCore::IntPoint> resultInViewCenterPoint;
-    bool isObscured = false;
-    if (containerElement) {
-        Optional<WebCore::FloatPoint> frameInViewCenterPoint = elementInViewClientCenterPoint(*containerElement, isObscured);
-        if (frameInViewCenterPoint.hasValue()) {
-            WebCore::IntPoint rootInViewCenterPoint = mainView->rootViewToContents(frameView->contentsToRootView(WebCore::IntPoint(frameInViewCenterPoint.value())));
-            switch (coordinateSystem) {
-            case CoordinateSystem::Page:
-                resultInViewCenterPoint = WebCore::IntPoint(mainView->clientToDocumentPoint(rootInViewCenterPoint));
-                break;
-            case CoordinateSystem::LayoutViewport:
-                // The point is already in client coordinates.
-                resultInViewCenterPoint = WebCore::IntPoint(mainView->clientToLayoutViewportPoint(rootInViewCenterPoint));
-                break;
-            }
-        }
+    // If an <option> or <optgroup> does not have an associated <select> or <datalist> element, then give up.
+    if (!containerElement) {
+        String elementNotInteractableErrorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::ElementNotInteractable);
+        completionHandler(elementNotInteractableErrorType, resultElementBounds, resultInViewCenterPoint, isObscured);
+        return;
+    }
+
+    // §12.1 Element Interactability.
+    // https://www.w3.org/TR/webdriver/#dfn-in-view-center-point
+    auto* firstElementRect = containerElement->getClientRects()->item(0);
+    if (!firstElementRect) {
+        String elementNotInteractableErrorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::ElementNotInteractable);
+        completionHandler(elementNotInteractableErrorType, resultElementBounds, resultInViewCenterPoint, isObscured);
+        return;
+    }
+
+    // The W3C WebDriver specification does not explicitly intersect the element with the visual viewport.
+    // Do that here so that the IVCP for an element larger than the viewport is within the viewport.
+    // See spec bug here: https://github.com/w3c/webdriver/issues/1402
+    auto viewportRect = frameView->documentToClientRect(frameView->visualViewportRect());
+    auto elementRect = FloatRect(firstElementRect->x(), firstElementRect->y(), firstElementRect->width(), firstElementRect->height());
+    auto visiblePortionOfElementRect = intersection(viewportRect, elementRect);
+
+    // If the element is entirely outside the viewport, still calculate it's bounds.
+    if (visiblePortionOfElementRect.isEmpty()) {
+        completionHandler(WTF::nullopt, resultElementBounds, resultInViewCenterPoint, isObscured);
+        return;
+    }
+
+    auto elementInViewCenterPoint = visiblePortionOfElementRect.center();
+    auto elementList = containerElement->treeScope().elementsFromPoint(elementInViewCenterPoint);
+    auto index = elementList.findMatching([containerElement] (auto& item) { return item.get() == containerElement; });
+    if (elementList.isEmpty() || index == notFound) {
+        // We hit this case if the element is visibility:hidden or opacity:0, in which case it will not hit test
+        // at the calculated IVCP. An element is technically not "in view" if it is not within its own paint/hit test tree,
+        // so it cannot have an in-view center point either. And without an IVCP, the definition of 'obscured' makes no sense.
+        // See <https://w3c.github.io/webdriver/webdriver-spec.html#dfn-in-view>.
+        String elementNotInteractableErrorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::ElementNotInteractable);
+        completionHandler(elementNotInteractableErrorType, resultElementBounds, resultInViewCenterPoint, isObscured);
+        return;
+    }
+
+    // Check the case where a non-descendant element hit tests before the target element. For example, a child <option>
+    // of a <select> does not obscure the <select>, but two sibling <div> that overlap at the IVCP will obscure each other.
+    // Node::isDescendantOf() is not self-inclusive, so that is explicitly checked here.
+    isObscured = elementList[0] != containerElement && !elementList[0]->isDescendantOf(containerElement);
+
+    auto inViewCenterPointInRootCoordinates = convertPointFromFrameClientToRootView(frameView, elementInViewCenterPoint);
+    switch (coordinateSystem) {
+    case CoordinateSystem::Page:
+        resultInViewCenterPoint = roundedIntPoint(mainView->absoluteToDocumentPoint(inViewCenterPointInRootCoordinates));
+        break;
+    case CoordinateSystem::LayoutViewport:
+        resultInViewCenterPoint = roundedIntPoint(mainView->absoluteToLayoutViewportPoint(inViewCenterPointInRootCoordinates));
+        break;
     }
 
     completionHandler(WTF::nullopt, resultElementBounds, resultInViewCenterPoint, isObscured);