Bug 213139 – Automation.computeElementLayout should return iframe-relative element rects (when coordinate system is 'Page')

• Status: RESOLVED
• Resolution: FIXED
• Component: WebDriver

[Blaze Burg]

Currently it's relative to top-level browsing context, which does not match the non-normative note (https://w3c.github.io/webdriver/#get-element-rect):

"""
The Get Element Rect command returns the dimensions and coordinates of the given web element. The returned value is a dictionary with the following members:

x
X axis position of the top-left corner of the web element relative to the current browsing context’s document element in CSS pixels.
y
Y axis position of the top-left corner of the web element relative to the current browsing context’s document element in CSS pixels.
"""

However the remote end says to "let coordinates be the result of the 'calculate the absolute position' algorithm, which does not seem to align with the note in the case where the main frame is scrolled:

"""
To calculate the absolute position of element:
1. Let rect be the value returned by calling getBoundingClientRect.
2. Let window be the associated window of current top-level browsing context.
3. Let x be (scrollX of window + rect’s x coordinate).
4. Let y be (scrollY of window + rect’s y coordinate).
5. Return a pair of (x, y).
"""

It seems to me that adding the scrollX of the top window is unnecessary to achieve the desired result.

In any case other browsers return iframe-relative coordinates from Get Element Rect.

[Blaze Burg]

<rdar://problem/55042445>

[Blaze Burg]

I filed a spec bug to clarify here: https://github.com/w3c/webdriver/issues/1533

[Blaze Burg]

I think the correct fix is to alter the semantics of the 'Page' coordinate system to provide iframe-relative coordinates. The LayoutViewport coordinate system is designed to provide top-level browsing context client coordinates that would be usable for Element Click / Actions chains.

[Blaze Burg]

(In reply to Brian Burg from comment #3)
> I think the correct fix is to alter the semantics of the 'Page' coordinate
> system to provide iframe-relative coordinates. The LayoutViewport coordinate
> system is designed to provide top-level browsing context client coordinates
> that would be usable for Element Click / Actions chains.

Based on an audit of uses in webkitgtkdriver and safaridriver, this change should not regress any other endpoints.

[Blaze Burg]

The obvious fix to use the equivalent of elem.getBoundingClientRect() seems to do the trick so far, running some more tests to check for fallout.

[Blaze Burg]

Created attachment 401794 [details]
Patch

[Devin Rousso]

Comment on attachment 401794 [details]
Patch

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

r=me, nice fix :)

> Source/WebKit/ChangeLog:14
> +        Covered by existing WPT test suite. The semantics of this command are under review, as

I always hated "WPT test" ... it's so redundant 😞

> Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp:689
>      case CoordinateSystem::LayoutViewport:

NIT: we should wrap this `case` in `{` and `}` so `elementBoundsInRootCoordinates` is local to just this `case` (i'd also be fine if you inlined it)

> Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp:747
> +        auto inViewCenterPointInRootCoordinates = convertPointFromFrameClientToRootView(frameView, elementInViewCenterPoint);

Ditto (:689)

[Blaze Burg]

Committed r262997: <https://trac.webkit.org/changeset/262997>

[Lauro Moura]

Two selenium test cases from `position_and_size_tests.py` started failing after this change, but I think we may add them as failures while this spec ambiguity is sorted out.

The first one, testShouldScrollPageAndGetCoordinatesOfAnElementThatIsOutOfViewPort, uses `element.location` and `element.location_once_scrolled_into_view`, and seems to assume the former to reflect the non-normative behavior of returning the location on the page instead on the current browsing context. Similarly, the Java version of the tests finishes with:

assertThat(getLocationOnPage(By.id("box"))).isEqualTo(new Point(10, 5010));

The second one, testShouldGetCoordinatesOfAnElementWithFixedPosition, uses a fixed element originally at y=0, scrolls, and tries to see if its `elem.location` changed (it does not, with the current behavior, as the position in the viewport does not change).

I'll mark them as Fail, linking to this bug for reference.