1493115 - WebDriver:ElementClick: Test scrolling into view of partially displayed elements (was: WebDriver:ElementClick doesn't scroll partly visible element into view)

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect, P1)

Description

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

Originally filed as: https://github.com/mozilla/geckodriver/issues/1346

With `moz:webdriverClick` enabled an only partially visible element which center point is outside of the screen by eg. 1px, is not getting scrolled into view. 

Marionette test:

> def test(self):
>     self.marionette.navigate(inline("""
>       <div style="height: 99vh"></div>
>       <div id="foo" style="background-color: red;" onclick="alert('clicked');">foo</div>
>     """))
>     self.marionette.find_element(By.ID, "foo").click()
>     alert = self.marionette.switch_to_alert()
>     alert.accept()

With `moz:webdriverClick` set to `False` it works.

Comment 1

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

I wonder if that might also be related in how we calculate the center point and emit the click. Maybe bug 1499360 can make that work.

Comment 2

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

This is indeed fixed by Andreas' patch on bug 1499360. Andreas, do you think that a testcase like the above should be added? If yes, could you please do that?

Comment 3

[Andreas Tolfsen ❲:ato❳]

Right, this is another one of those ”literal” edge cases.  Before
https://bugzilla.mozilla.org/show_bug.cgi?id=1499360 we calculated
the target click coordinate of a 1px square to be 1px beyond the
square’s area.  In CSS pixels the square occupies exactly 1px, and
does not span _from_, say, (1,1) to (2,2), but is exactly _at_
(1,1).

My patch would’ve fixed the problem of this bug because we use the
in-view centre point to determine whether the element is already
in view, so that we don’t call DOMElement.scrollIntoView pre-emptively:
https://searchfox.org/mozilla-central/rev/fcfb479e6ff63aea017d063faa17877ff750b4e5/testing/marionette/element.js#1069

We should probably add a testcase for that, but I can’t promise I
will get around to it.

Comment 4

[Andreas Tolfsen ❲:ato❳]

Actually, thinking more carefully about this it might be an easy job.

Comment 5

[Andreas Tolfsen ❲:ato❳]

Attachment: bug 1493115: webdriver: move center_point into helpers;

Comment 6

[Andreas Tolfsen ❲:ato❳]

Attachment: bug 1493115: webdriver: test scroll behaviour on partially visible elements;

Partially visible elements should according to WebDriver not be
scrolled into view.  Drivers first run the in-view centre point
algorithm to determine if any portion of the targetted element is
in view, and if it is not, the element is scrolled into view.

As little as a 1 CSS pixel is sufficient for the element to deemed
in view.  Implementations have had problems with deeming elements
out of view when a single pixel border has in fact been in view due
to faulty rounding of the target point, which is why we test a range
from 10 to 1 pixels.

Depends on D10821

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d3bf4055a519
webdriver: move center_point into helpers; r=whimboo
https://hg.mozilla.org/integration/autoland/rev/d1a6af0172d9
webdriver: test scroll behaviour on partially visible elements; r=whimboo

Comment 8

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/d3bf4055a519
https://hg.mozilla.org/mozilla-central/rev/d1a6af0172d9

Comment 9

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

There is no indication yet for an upstream sync. James, can you please have a look? Thanks.

Comment 10

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/13967 for changes under testing/web-platform/tests

Comment 11

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Can't merge web-platform-tests PR due to failing upstream checks:
Github PR https://github.com/web-platform-tests/wpt/pull/13967
* continuous-integration/travis-ci/pr (https://travis-ci.org/web-platform-tests/wpt/builds/452015781?utm_source=github_status&utm_medium=notification)
* Taskcluster (pull_request) (https://tools.taskcluster.net/task-group-inspector/#/Vww6CwgXTqm-Styj8nRhnw)

Comment 12

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged