Open
Bug 1374283
Opened 7 years ago
Updated 2 years ago
Clicking an element like a link fails if child fully overlays area due to: "ElementNotInteractableException: Element could not be scrolled into view"
Categories
(Remote Protocol :: Marionette, defect, P3)
Tracking
(Not tracked)
NEW
People
(Reporter: whimboo, Unassigned)
References
(Blocks 2 open bugs, )
Details
Attachments
(3 files)
Found this issue by https://github.com/mozilla/geckodriver/issues/322:
1497878465153 Marionette TRACE 6 -> [0,15,"findElement",{"using":"id","value":"secure-login-link"}]
1497878465242 Marionette TRACE 6 <- [1,15,null,{"value":{"element-6066-11e4-a52e-4f735466cecf":"2087c93d-eaac-5142-a4fc-fff7af636dab","ELEMENT":"2087c93d-eaac-5142-a4fc-fff7af636dab"}}]
1497878465247 Marionette TRACE 6 -> [0,16,"clickElement",{"id":"2087c93d-eaac-5142-a4fc-fff7af636dab"}]
++DOCSHELL 0x14aa48800 == 15 [pid = 99490] [id = {cecb5990-d5d5-ef45-b72c-a53ae81c3a5b}]
++DOMWINDOW == 40 (0x14aa49000) [pid = 99490] [serial = 40] [outer = 0x0]
++DOMWINDOW == 41 (0x14aa49800) [pid = 99490] [serial = 41] [outer = 0x14aa49000]
1497878465549 Marionette DEBUG Received DOM event "pagehide" for "http://agendaweek.com/"
1497878465748 Marionette DEBUG Canceled page load listener because no navigation has been detected
1497878465751 Marionette TRACE 6 <- [1,16,null,{}]
Surprisingly no `beforeunload` event is fired and as such we cancel the page load. I will have a look what's wrong here.
|
Reporter | |
Comment 1•7 years ago
|
||
The problem here is not the page load algorithm, but that the `a` element is fully overlayed by a child node with a CSS rule of `display: block`. As such the click will be received by the child but not the `a` node itself. Because of that the navigation never occurs.
Here the minimized test file:
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html>
<body>
<a id="link" href="https://mozilla.org">
<p style="display:block">Overlayed Text</p>
</a>
</body>
</html>
To make this problem appear both the DOCTYPE and `display:block` are necessary. Removing one of those causes Marionette to correctly navigate.
Andreas and David, given that you know the specs better than me, is there something we have to add to the webdriver spec?
Flags: needinfo?(dburns)
Flags: needinfo?(ato)
Summary: click() fails to wait for page load because no "beforeunload" event is fired → Clicking an element like a link fails if child fully overlays area
|
||
Comment 2•7 years ago
|
||
Where is clicking happening exactly?
Are we not hitting the right rect when looking at the rects for the element?
what does elementFromPoint return?
Flags: needinfo?(dburns)
Flags: needinfo?(ato)
|
|
Reporter | |
Comment 3•7 years ago
|
||
In the above case the test would look like:
def test_click(self):
self.marionette.navigate(%testcase%)
elem = self.marionette.find_element(By.ID, "link")
elem.click()
As tried to say in the last comment, we are synthesizing the click at the center of <a> but that most likely will cause <p> to receive the click event because it overlays everything? I haven't looked into the details.
|
|
Reporter | |
Updated•7 years ago
|
Assignee: hskupin → nobody
Status: ASSIGNED → NEW
|
|
||
Updated•7 years ago
|
Assignee: nobody → dburns
|
|
||
Comment 4•7 years ago
|
||
I have had a quick look at this and we appear to be clicking on the html element.
element.getInViewCentrePoint is thinking the point that needs clicking is x:8, y:12 but document.elementFromPoint(8, 12) returns <html>
However we need to be looking at y being closer to 17 as that is the centre height for the overlay.
|
|
Reporter | |
Comment 5•7 years ago
|
||
This is blocking people from running tests with geckodriver (webdriver). Bumping priority to P1.
David, are you going to work on this or should someone else pick it up?
Flags: needinfo?(dburns)
Priority: -- → P1
|
|
Reporter | |
Comment 6•7 years ago
|
||
With the webdriver conforming click enabled we get the following error:
TEST-UNEXPECTED-ERROR | test_minimized.py TestMinimizedTestCase.test | ElementNotInteractableException: Element <a id="link"> could not be scrolled into view
stacktrace:
WebDriverError@chrome://marionette/content/error.js:227:5
ElementNotInteractableError@chrome://marionette/content/error.js:335:5
webdriverClickElement@chrome://marionette/content/interaction.js:180:11
async*interaction.clickElement@chrome://marionette/content/interaction.js:151:11
async*clickElement/<@chrome://marionette/content/listener.js:1368:14
navigate/<@chrome://marionette/content/listener.js:448:13
TaskImpl_run@resource://gre/modules/Task.jsm:331:42
TaskImpl@resource://gre/modules/Task.jsm:280:3
asyncFunction@resource://gre/modules/Task.jsm:252:14
Task_spawn@resource://gre/modules/Task.jsm:166:12
navigate@chrome://marionette/content/listener.js:447:12
clickElement@chrome://marionette/content/listener.js:1367:5
Why are we trying to scroll it into view? Maybe because it's completely overlayed and `element.isInView()` wrongly returns false? It's clearly in the view by default.
The failure we should get here is a ElementClickInterceptedError, right?
Attached you can find the testcase. Not sure if we should fix it for seleniumClickElement.
Flags: needinfo?(dburns) → needinfo?(ato)
|
|
Reporter | |
Comment 7•7 years ago
|
||
Andreas why should this bug block bug 1321516? We don't have a failing case in our suite yet, so to save our time we should not try to get this fixed for the legacy clickElement. I thought that this is what we agreed on last time we spoke about this bug.
|
||
Comment 8•7 years ago
|
||
The problem here is that we’re wrongly calculating the element’s
in-view centre point in element.getInViewCentrePoint when we have
an inline element encapsulating a block-level element in an XHTML
document.
XHTML has a special rendering mode where the parent <a> element in
this case inherits the margins/layout box of the child. In the
following document, the first DOMRect’s Y axis is reported to be
1 px, whereas it in a similar HTML document would be 16 px.
> <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
> "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
> <html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
> <head>
> <title>XHTML might be the future</title>
> </head>
>
> <body>
> <a href="https://mozilla.org">
> <p style="display: block">overlayed text</p>
> </a>
> </body>
> </html>
This causes element.getPointerInteractablePaintTree to get
the paint tree (document.elementsFromPoint) of somewhere
else in the document that doesn’t contain the <a> element.
interaction.webdriverClickElement then, for the first call to
element.isInView, thinks it is out of the viewport and then tries
to scroll it into view, and then for the second check it returns an
"element not interactable" error because it is still not in view.
An "element click intercepted" error is expected the way the
specification is written right now, but because a click to <p>
would bubble through down to <a>, I wonder if there is a second
specification issue that we need to take into account for a certain
range of elements such as <p>.
Flags: needinfo?(ato)
|
|
||
Comment 9•7 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #7)
> Andreas why should this bug block bug 1321516? We don't have a
> failing case in our suite yet, so to save our time we should not
> try to get this fixed for the legacy clickElement. I thought that
> this is what we agreed on last time we spoke about this bug.
It means our WebDriver click implementation is conformant, but
that the specification is probably wrong in one, possibly two
places. Because we don’t want to ship a new click implementation
that is sub par to the current one, we need to first address the
specification issues and then our implementation before we can
enable interaction.webdriverClickElement by default.
We should indeed not fix this for the legacy
interaction.seleniumClickElement, which passes with the test case
you attached.
|
|
Reporter | |
Comment 10•7 years ago
|
||
Right, but given that both implementations fail I don't see a reason why it should block the switch over to the conformant click implementation. Personally I would like to try out the conformant method ASAP if you could fix the minor JS failure on bug 1321516.
|
|
||
Comment 11•7 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #10)
> Right, but given that both implementations fail I don't see a
> reason why it should block the switch over to the conformant click
> implementation.
For me, the test passes when using interaction.seleniumClickElement,
which would mean we would regress if we enable
webdriverClickElement.
Is this not the case on your system?
OS: Unspecified → All
Hardware: Unspecified → All
|
|
Reporter | |
Comment 12•7 years ago
|
||
(In reply to Andreas Tolfsen ‹:ato› from comment #11)
> For me, the test passes when using interaction.seleniumClickElement,
> which would mean we would regress if we enable
> webdriverClickElement.
>
> Is this not the case on your system?
The test clearly fails with `seleniumClickElement`. I think what you missed is that the test is actually not fully complete and misses a final assertion, which checks that `mozilla.org` has been opened.
Can you please verify that? If you see the same we should remove the blocker.
Flags: needinfo?(ato)
|
|
||
Comment 13•7 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #12)
> (In reply to Andreas Tolfsen ‹:ato› from comment #11)
>
> > For me, the test passes when using
> > interaction.seleniumClickElement, which would mean we would
> > regress if we enable webdriverClickElement.
> >
> > Is this not the case on your system?
>
> The test clearly fails with `seleniumClickElement`. I think what
> you missed is that the test is actually not fully complete and
> misses a final assertion, which checks that `mozilla.org` has been
> opened.
>
> Can you please verify that? If you see the same we should remove
> the blocker.
I see, your point is that the click itself doesn’t return an
error, but it also doesn’t actually hit the element or initiate a
document load. I can reproduce that.
With webdriverClickElement I get an ‘element not interactable’
error, which I explained earlier in comment #8. I suspect that
it is alsot he reason why seleniumClickElement doesn’t hit the
element.
Flags: needinfo?(ato)
|
|
Reporter | |
Comment 14•7 years ago
|
||
Correct. And as such this bug doesn't block us switching to the conforming click on bug 1321516. Instead it makes the failure more verbose.
No longer blocks: 1321516
|
|
Reporter | |
Comment 15•7 years ago
|
||
David, are you still working on this P1 bug? It would be good to get it investigated.
Updating the geckodriver issue because #322 was closed, and #1007 exists now.
Flags: needinfo?(dburns)
|
|
||
Comment 16•7 years ago
|
||
I dont have time to work on this at the moment
Assignee: dburns → nobody
Flags: needinfo?(dburns)
|
|
Reporter | |
Comment 17•7 years ago
|
||
I just tried to run the testcase as attached again, now that our interactability checks are turned on by default and the reason why this test is failing is clear now:
TEST-UNEXPECTED-ERROR | _a/test_minimized.py TestMinimizedTestCase.test | ElementNotInteractableException: Element <a id="link" href="https://mozilla.org"> could not be scrolled into view
Given that the paragraph completely overlays the link element, we are trying to scroll the link into view. This is actually not possible, and as such we fail in `element.isInView()`.
When using this HTML testcase I see the following tree for elementsFromPoint:
> [object HTMLParagraphElement],https://mozilla.org/,[object HTMLBodyElement],[object HTMLHtmlElement]
Which is fine. But using Marionette I only get the [object HTMLHtmlElement]
returned by `element.getPointerInteractablePaintTree`.
Digging into this method I can see that we get three client rects returned:
rect: 8/4 8/20
rect: 8/16 1272/35.19999694824219
rect: 8/39.19999694824219 8/55.19999694824219
|
|
Reporter | |
Comment 18•7 years ago
|
||
As you can see here two of the client rects of the link element only span a small portion on the left side of the block. Only the 2nd rect has the correct dimensions for a valid check.
Right now we only pick the very first return client rect, and throw away the others. Maybe we have to take all of them into account?
The spec says:
https://w3c.github.io/webdriver/webdriver-spec.html#dfn-in-view
> Let center point be the in-view center point of the first indexed element in rectangles.
So I assume we have to get the spec updated.
Flags: needinfo?(ato)
|
|
Reporter | |
Updated•7 years ago
|
Summary: Clicking an element like a link fails if child fully overlays area → Clicking an element like a link fails if child fully overlays area due to: "ElementNotInteractableException: Element could not be scrolled into view"
|
|
Reporter | |
Updated•7 years ago
|
Attachment #8943972 -
Attachment mime type: text/plain → text/html
|
|
Reporter | |
Updated•7 years ago
|
Attachment #8943969 -
Attachment mime type: text/plain → text/html
|
|
Reporter | |
Comment 19•7 years ago
|
||
Hm, Google Chrome returns only two client rects, whereby also the second rect contains the wanted dimensions.
|
|
||
Comment 20•7 years ago
|
||
First to clear up some misunderstandings I first had when I looked
at the test cases. The test case with the red boxes [1] has a JS
blob that simply paints the client rects in order as separate <div>
elements for visualisation purposes. One should take no note of
the elements themselves, but it is worth noting that they overlay
the already-overlayed link, making it ineligable for clicking.
The link is actually meant to be clickable.
If I remember correctly, the reason the WebDriver standard uses the
first client rect as basis for calculating the in-view centre point
has to do with multiline links. The centre point of the _bounding
box_ of a multiline link might not target the link itself since it
is an inline element.
I also believe it is possible to conjure up a test case that makes
the current specification text fail with multiline links, because
you can’t always guarantee that the centre point of the first
client rect is always clickable.
In the concrete case of <a style="display: inline"><p style="display:
block">foo</p></a> the problem would be fixed by using the <a>
element’s bounding box. However, changing the WebDriver in-view
centre point algoritm to that would break multiline links because
it insists to click the centre point.
I think whimboo is right there are inherent specification problems
here to do with the calculation of an inline element’s centre point.
At the moment I can’t think of a solution that will fix both problems.
Using the bounding box will cause problems with inline links like this:
> a sldkaj sdlkja sdlkajs dlkaj sdlkajs [this is a
> link that spans lines] asdlak jdlka dalskd alks.
In this example the bounding box will span both lines and the
centre point will be somewhere along “dlkaj” and “asdlak”,
which isn’t clickable. The first rect will be “this is a”,
and the centre point of this is clickable.
In the case of this specific bug, the first client rect will be
empty and located away from the actual link because <a> is an inline
element and that implies whitespace.
(Thanks to whimboo for walking me through the attached test cases on IRC.)
[1] https://bug1374283.bmoattachments.org/attachment.cgi?id=8943972
Flags: needinfo?(ato)
|
|
||
Updated•7 years ago
|
Priority: P1 → P2
|
|
Reporter | |
Updated•5 years ago
|
Priority: P2 → P3
|
||
Updated•2 years ago
|
Severity: normal → S3
|
|
||
Updated•2 years ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•