1154525 - Make HTMLElement.size and HTMLElement.location to use marionette.rect internally

Status: RESOLVED FIXED

(Testing :: Marionette Client and Harness, defect)

Description

[David Burns :automatedtester]

size and location have been deprecated and are due for removal. We need to update the python tests to use HTMLElement.rect instead

Comment 1

[Andreas Tolfsen ❲:ato❳]

Attachment: MozReview Request: bz://1154525/ato

/r/7207 - Bug 1154525: Make HTMLElement#location and #size use #rect internally

Pull down this commit:

hg pull -r d0f18163b3bcf7e3a9d5698953e244b718013258 https://reviewboard-hg.mozilla.org/gecko/

Comment 2

[Andreas Tolfsen ❲:ato❳]

dburns: I'm sorry I misread this bug as making them use rect internally, but I only realised after I'd pushed my patch.  Refiled your original bug as bug 1155746.

Comment 3

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8594010 [details]
MozReview Request: bz://1154525/ato

/r/7207 - Bug 1154525: Make HTMLElement#location and #size use #rect internally

Pull down this commit:

hg pull -r d0f18163b3bcf7e3a9d5698953e244b718013258 https://reviewboard-hg.mozilla.org/gecko/

Comment 4

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8594010 [details]
MozReview Request: bz://1154525/ato

https://reviewboard.mozilla.org/r/7205/#review6145

::: testing/marionette/driver/marionette_driver/marionette.py:153
(Diff revision 1)
> +        return {"x": rect.x, "y": rect.y}

In the listener's getElementLocation I see:

    let el = elementManager.getKnownElement(msg.json.id, curFrame);
    let rect = el.getBoundingClientRect();

    let location = {};
    location.x = rect.left;
    location.y = rect.top;
    
but getElementRect has:

    let el = elementManager.getKnownElement(id, curFrame);
    let clientRect = el.getBoundingClientRect();
    return {
        x: clientRect.x + curFrame.pageXOffset,
        y: clientRect.y  + curFrame.pageYOffset,
        width: clientRect.width,
        height: clientRect.height
    };

from documentation of getBoundingClientRect, this means location used to be relative to the viewport but is now relative to the document. Is that intended?

Comment 5

[Andreas Tolfsen ❲:ato❳]

https://reviewboard.mozilla.org/r/7205/#review6195

> In the listener's getElementLocation I see:
> 
>     let el = elementManager.getKnownElement(msg.json.id, curFrame);
>     let rect = el.getBoundingClientRect();
> 
>     let location = {};
>     location.x = rect.left;
>     location.y = rect.top;
>     
> but getElementRect has:
> 
>     let el = elementManager.getKnownElement(id, curFrame);
>     let clientRect = el.getBoundingClientRect();
>     return {
>         x: clientRect.x + curFrame.pageXOffset,
>         y: clientRect.y  + curFrame.pageYOffset,
>         width: clientRect.width,
>         height: clientRect.height
>     };
> 
> from documentation of getBoundingClientRect, this means location used to be relative to the viewport but is now relative to the document. Is that intended?

Well spotted, sir!  I didn't catch this.

Although the [specification isn't entirely clear on this](https://w3c.github.io/webdriver/webdriver-spec.html#elementrect-getelementrect) I think `getElementLocation` has been wrong all along.  An element's position should be relative to the document, and not the viewport.

I also think this is what Selenium does, but that's also not quite clear from the API documentation.

The question then is if we have test code which relies on this for calculations.  I'll do a full try run to verify.

Thanks again for the thorough review!

Comment 6

[Andreas Tolfsen ❲:ato❳]

try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7875e6a70828

Comment 7

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8594010 [details]
MozReview Request: bz://1154525/ato

https://reviewboard.mozilla.org/r/7205/#review6217

::: testing/marionette/driver/marionette_driver/marionette.py:158
(Diff revision 1)
> +        

Whitespace added.

Comment 8

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8594010 [details]
MozReview Request: bz://1154525/ato

https://reviewboard.mozilla.org/r/7205/#review6219

Ship It!

Comment 9

[Andreas Tolfsen ❲:ato❳]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6f1ca5412ad1

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/768d5fb8b227

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/768d5fb8b227

Comment 13

[Andreas Tolfsen ❲:ato❳]

Attachment: MozReview Request: Bug 1154525: Make HTMLElement#location and #size use #rect internally

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Moving bugs for Marionette client due to component changes.