Closed Bug 1493115 Opened Last year Closed Last year

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

Categories

(Testing :: Marionette, defect, P1)

defect

Tracking

(firefox65 fixed)

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: whimboo, Assigned: ato)

References

(Blocks 1 open bug, )

Details

Attachments

(2 files)

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.
Priority: -- → P3
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.
Depends on: 1499360
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?
Flags: needinfo?(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.
Flags: needinfo?(ato)
Actually, thinking more carefully about this it might be an easy job.
Assignee: nobody → ato
Status: NEW → ASSIGNED
Priority: P3 → P1
Summary: "WebDriver:ElementClick" doesn't scroll partly visible element into view → WebDriver:ElementClick: Test scrolling into view of partially displayed elements (was: WebDriver:ElementClick doesn't scroll partly visible element into view)
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
Pushed by atolfsen@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/d3bf4055a519
https://hg.mozilla.org/mozilla-central/rev/d1a6af0172d9
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Blocks: 1405967
No longer blocks: 1493112
Depends on: 1505142
There is no indication yet for an upstream sync. James, can you please have a look? Thanks.
Flags: needinfo?(james)
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/13967 for changes under testing/web-platform/tests
Flags: needinfo?(james)
You need to log in before you can comment on or make changes to this bug.