Henrik Skupin [:whimboo][⌚️UTC+1] Reporter
	Description • 6 years ago

In bug 1406505 I added a workaround for bug 1413493 which should take the first cell into account for checking the element paint tree. Interestingly this check fails if the table has multiple cells per row:

> ElementNotInteractableException: Element <tr> could not be scrolled into view

	Henrik Skupin [:whimboo][⌚️UTC+1] Reporter
	Comment 1 • 6 years ago

This issue was originally reported at https://github.com/mozilla/geckodriver/issues/1228.

	Henrik Skupin [:whimboo][⌚️UTC+1] Reporter
	Comment 2 • 6 years ago

Two problems here:

1) Having an even number of cells `getPointerInteractablePaintTree` might contain a tree without the cell, because the in-view center point might be in-between both `td` elements in case of both have the same width. Given that bug 1413493 is still not fixed the paint tree will start with the `table` element.

2) If the widths of the cells are different and the click really happens on top of a cell, the calculated center point will still be the one for the `tr` element, and as such spans the whole width of it. As such not the first cell but another one is part of the paint tree, and the check `tree.includes(el.cells[0])` simply fails.

So my proposal here for a proper workaround would be:

1) If the element is a `tr` get the in view center point of it
2) Check which of the available cells contains that point in the boundary box
3) Use that cell for the paint tree check

It's still unclear what to do when we really hit the case that the test clicks in-between the cells. I think that there is nothing we can do at all for now. Maybe we can raise an exception with the proper wording?

Andreas, does the above sound ok for you?

	Andreas Tolfsen ❲:ato❳
	Comment 3 • 6 years ago

I think your proposal makes sense, but given a hypothetical situation
where bug 1413493 is fixed, would that solve the problem?  It seems
there is always a potential the click point may end up “between
cells”.  Is a change to the specification required or will it resolve
itself with bug 1413493?

	Andreas Tolfsen ❲:ato❳
	Comment 4 • 6 years ago

No need for a needinfo on this.  Not time critical.

	Henrik Skupin [:whimboo][⌚️UTC+1] Reporter
	Comment 5 • 6 years ago

(In reply to Andreas Tolfsen ‹:ato› from comment #3)
> I think your proposal makes sense, but given a hypothetical situation
> where bug 1413493 is fixed, would that solve the problem?  It seems
> there is always a potential the click point may end up “between
> cells”.  Is a change to the specification required or will it resolve
> itself with bug 1413493?

No, this bug is about <tr> and faking it with <td>. So those situation would be present when the bug is fixed:

1) Trying to click the <tr> even with a <td> element overlaying it at the center point will still have the <tr> in the paint tree, and as such the check will pass. So the click point doesn't matter as long as it is in view.

2) Trying to click on a <td> will only succeed when the wanted cell is part of the paint tree. Otherwise we will still fail.

My workaround simply fakes 1) and doesn't touch 2). So once bug 1413493 has been fixed we can safely remove our workaround code.

	Louis
	Comment 6 • 6 years ago

(In reply to Henrik Skupin (:whimboo) from comment #2)
> Two problems here:
> 
> 1) Having an even number of cells `getPointerInteractablePaintTree` might
> contain a tree without the cell, because the in-view center point might be
> in-between both `td` elements in case of both have the same width. Given
> that bug 1413493 is still not fixed the paint tree will start with the
> `table` element.
> 
> 2) If the widths of the cells are different and the click really happens on
> top of a cell, the calculated center point will still be the one for the
> `tr` element, and as such spans the whole width of it. As such not the first
> cell but another one is part of the paint tree, and the check
> `tree.includes(el.cells[0])` simply fails.
> 
> So my proposal here for a proper workaround would be:
> 
> 1) If the element is a `tr` get the in view center point of it
> 2) Check which of the available cells contains that point in the boundary box
> 3) Use that cell for the paint tree check
> 
> It's still unclear what to do when we really hit the case that the test
> clicks in-between the cells. I think that there is nothing we can do at all
> for now. Maybe we can raise an exception with the proper wording?
> 
> Andreas, does the above sound ok for you?

Hi Hendrik, 

I just run into the exact problem with <tr> on latest FF 60 and Selenium 3.12, so I guess the bug hasn't been fixed yet.

I try your workaround: I calculated the center point of this <tr> and identify the <td> in it, then I click on this <td>. I don't get ElementNotInteractableException anymore, but this <td> or <tr> isn't clicked, it just runs through. When I debug it, after the click action, this FF instance is frozen, I can't even click on this <tr> manually. 

Did I execute your workaround wrongly? Could you please have an advice? I have tried all solutions I found on internet, but I am still stuck here. Thanks

	satmce
	Comment 7 • 6 years ago

Hi Team, We are also experiencing this issue in our automation regression after upgrade from 2.x to 3.x. This is breaking many of existing regression test which is blocking our selenium & firefox upgrade. This will be great help if you can increase the priority & fix this problem. Or give some more clear workaround details to resolve this issue.

Thanks,
Sathish

	Rich
	Comment 8 • 6 years ago

(In reply to Andreas Tolfsen 「:ato」 from comment #4)
> No need for a needinfo on this.  Not time critical.

Hi Andreas,

Just out of curiosity, what the is criteria for time critical?

	Andreas Tolfsen ❲:ato❳
	Comment 9 • 6 years ago

Hi Rich,

(In reply to Rich from comment #8)

> Just out of curiosity, what the is criteria for time critical?

As in, this bug is something we want to fix (P3), but it is not in
progress or currently a priority.