Open Bug 1374283 Opened 7 years ago Updated 1 year 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)

52 Branch
defect

Tracking

(Not tracked)

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.
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
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)
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.
Assignee: hskupin → nobody
Status: ASSIGNED → NEW
Assignee: nobody → dburns
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.
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
Blocks: 1307760
Depends on: 1386605
Attached file testcase
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)
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.
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)
(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.
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.
(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
Depends on: 1394354
(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)
(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)
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
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.
I dont have time to work on this at the moment
Assignee: dburns → nobody
Flags: needinfo?(dburns)
Attached file HTML testcase
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
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)
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"
Attachment #8943972 - Attachment mime type: text/plain → text/html
Attachment #8943969 - Attachment mime type: text/plain → text/html
Hm, Google Chrome returns only two client rects, whereby also the second rect contains the wanted dimensions.
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)
Priority: P1 → P2
Blocks: 1493112
No longer blocks: 1493112
Blocks: 1405967
Priority: P2 → P3
Severity: normal → S3
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: