diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog index e474fd23e51c..bb7103370085 100644 --- a/Source/WebKit/ChangeLog +++ b/Source/WebKit/ChangeLog @@ -1,3 +1,39 @@ +2017-11-30 Brian Burg + + Web Automation: computeElementLayout does not correctly translate iframe client coordinates to main frame coordinates + https://bugs.webkit.org/show_bug.cgi?id=180213 + + + Reviewed by Simon Fraser. + + The current implementation computes points in terms of the frame in which the element is located. + However, WebDriver expects coordinates to be relative to the top-level document since + these coordinates are used for generating click events, among other things. + + To convert from frame client coordinates to main frame client coordinates, round-trip + both inViewCenterPoint and elementBounds to root view coordinates and back + to the main frame's contents/client coordinates. Then convert this to page coordinates if needed. + + This progresses several tests in the Selenium Python test suite: + + - event_firing_webdriver_tests.py::test_should_fire_navigation_events + - frame_switching_tests.py::testShouldBeAbleToClickInAFrameThatRewritesTopWindowLocation + - frame_switching_tests.py::testShouldBeAbleToSwitchToTheTopIfTheFrameIsDeletedFromUnderUs + - frame_switching_tests.py::testShouldBeAbleToSwitchToTheTopIfTheFrameIsDeletedFromUnderUsWithFrameIndex + - frame_switching_tests.py::testShouldBeAbleToSwitchToTheTopIfTheFrameIsDeletedFromUnderUsWithWebelement + - frame_switching_tests.py::testShouldNotBeAbleToDoAnythingTheFrameIsDeletedFromUnderUs + - position_and_size_tests.py::testShouldGetCoordinatesOfAnInvisibleElement + + * WebProcess/Automation/WebAutomationSessionProxy.cpp: + (WebKit::WebAutomationSessionProxy::computeElementLayout): + Get both the frame and main frame FrameViews and convert coordinates to the root view. + This is somewhat lossy as clientToDocument* deals with FloatPoints but contentsToRootView + deals with IntPoints. For the purposes of WebDriver, lossiness is not a problem since + integer values are expected anyway. + + The imperative nature of the coordinate calculations is difficult to debug, so I converted + this function to only assign to each variable once. + 2017-11-14 Carlos Garcia Campos Move JSONValues to WTF and convert uses of InspectorValues.h to JSONValues.h diff --git a/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp b/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp index e47374a6800c..070a8caba215 100644 --- a/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp +++ b/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp @@ -572,32 +572,37 @@ void WebAutomationSessionProxy::computeElementLayout(uint64_t pageID, uint64_t f return; } - WebCore::FloatRect elementBounds = coreElement->boundingClientRect(); - - auto* coreFrameView = frame->coreFrame()->view(); + 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; switch (coordinateSystem) { case CoordinateSystem::Page: - elementBounds = coreFrameView->clientToDocumentRect(elementBounds); + resultElementBounds = WebCore::IntRect(mainView->clientToDocumentRect(WebCore::FloatRect(rootElementBounds))); break; case CoordinateSystem::LayoutViewport: // The element bounds are already in client coordinates. + resultElementBounds = rootElementBounds; break; case CoordinateSystem::VisualViewport: ASSERT_NOT_REACHED(); break; } - std::optional inViewCenterPoint; + std::optional resultInViewCenterPoint; bool isObscured = false; if (containerElement) { - inViewCenterPoint = elementInViewClientCenterPoint(*containerElement, isObscured); - if (inViewCenterPoint.has_value()) { + std::optional frameInViewCenterPoint = elementInViewClientCenterPoint(*containerElement, isObscured); + if (frameInViewCenterPoint.has_value()) { + WebCore::IntPoint rootInViewCenterPoint = mainView->rootViewToContents(frameView->contentsToRootView(WebCore::IntPoint(frameInViewCenterPoint.value()))); switch (coordinateSystem) { case CoordinateSystem::Page: - inViewCenterPoint = coreFrameView->clientToDocumentPoint(inViewCenterPoint.value()); + resultInViewCenterPoint = WebCore::IntPoint(mainView->clientToDocumentPoint(rootInViewCenterPoint)); break; case CoordinateSystem::LayoutViewport: // The point is already in client coordinates. + resultInViewCenterPoint = rootInViewCenterPoint; break; case CoordinateSystem::VisualViewport: ASSERT_NOT_REACHED(); @@ -606,7 +611,7 @@ void WebAutomationSessionProxy::computeElementLayout(uint64_t pageID, uint64_t f } } - WebProcess::singleton().parentProcessConnection()->send(Messages::WebAutomationSession::DidComputeElementLayout(callbackID, enclosingIntRect(elementBounds), WebCore::IntPoint(inViewCenterPoint.value()), isObscured, String()), 0); + WebProcess::singleton().parentProcessConnection()->send(Messages::WebAutomationSession::DidComputeElementLayout(callbackID, resultElementBounds, resultInViewCenterPoint.value(), isObscured, String()), 0); } void WebAutomationSessionProxy::selectOptionElement(uint64_t pageID, uint64_t frameID, String nodeHandle, uint64_t callbackID)