1215806 - Marionette Element.displayed() works inconsistently for the elements that aren't in viewport because of scroll

Status: RESOLVED INCOMPLETE

(Testing Graveyard :: JSMarionette, defect)

Description

[Oleg Zasypkin [:azasypkin]]

While working on bug 1209419 we noticed that element.displayed() works inconsistently for the elements that are not visible because of scroll:

1) Element.displayed() is always "true" for the elements that are rendered, but not visible because of scroll;

2) If Element that is hidden behind scroll has CSS transform applied (in reduced test case "transform:translateX(0.01rem)"), then Element.displayed() returns "false" if element is rendered but not visible because of scroll, and "true" once we scroll to this element (element appears in the viewport).

So at least one of these behaviors is incorrect, I'd say that's the 1st one, but I'm not sure how Element.displayed() is supposed to work.

I've created branch [1] with dedicated simplified use case [2] to illustrate the issue.

Messages app use case is a bit more complex: we have scrollable list of the messages and every messages contains "checkbox", that smoothly appears using "transform: translate". So in test we want to check that every message has checkbox, but message.displayed() and checkbox-inside-message.displayed() return opposite values. If at least message.displayed() returns "false" when it's hidden behind scroll we'd know that we should scroll to see the message.

[1] https://github.com/azasypkin/gaia/tree/marionette-weird-scroll
[2] https://github.com/azasypkin/gaia/commit/308c0dc8bd650cb896250671d6a82ef681348760

Comment 1

[Oleg Zasypkin [:azasypkin]]

Hey Gareth,

Do you have any clue what is wrong here and how to fix/investigate this?

Thanks!

Comment 2

[Julien Wajsberg [:julienw]]

Hey Oleg, in the case of SMS, I know we have a transparent label that go on top of the message; maybe that's the reason here ?

Comment 3

[Oleg Zasypkin [:azasypkin]]

(In reply to Julien Wajsberg [:julienw] from comment #2)
> Hey Oleg, in the case of SMS, I know we have a transparent label that go on
> top of the message; maybe that's the reason here ?

Mmm, honestly I'm not sure how this can make a difference when message is visible and when it's not because of scroll, but will double check anyway :)

Thanks!

Comment 4

[Oleg Zasypkin [:azasypkin]]

Tried to remove <label> from markup entirely - same result :/

Comment 5

[Gareth Aye [:gaye] (back from PTO)]

David Burns or Andreas Tolfsen would know more about displayed behavior

Comment 6

[David Burns :automatedtester]

isDisplayed algorithm is mostly described in http://w3c.github.io/webdriver/webdriver-spec.html#element-displayedness. 

The algorithm tries to see if an element would be visible if it were scrolled to without scrolling. This allows to be able to workout the visibility of elements without having to have the jerkiness of scrolling the screen allover the place.

Comment 7

[Oleg Zasypkin [:azasypkin]]

(In reply to David Burns :automatedtester from comment #6)
> isDisplayed algorithm is mostly described in
> http://w3c.github.io/webdriver/webdriver-spec.html#element-displayedness. 
> 
> The algorithm tries to see if an element would be visible if it were
> scrolled to without scrolling. This allows to be able to workout the
> visibility of elements without having to have the jerkiness of scrolling the
> screen allover the place.

Good to know, thanks David!

One more question for you then :) - do you have any clue why element that is slightly moved with "transform: translateX(0.01rem)" can be considered as not displayed if it's behind scroll and displayed when we move it to the viewport?

Thanks!

Comment 8

[David Burns :automatedtester]

That does seem weird. A lot of the code tries to do things via overflows and I wonder if have a weird bug.

If you could create reduced test case for me?

Comment 9

[Oleg Zasypkin [:azasypkin]]

Hey David,

I've tried to create even more reduced test case, but the issue isn't reproduced with something very simple :/

So I've updated [1] and [2] to make it as simple as possible, I've just added very simple test inside SMS app that you can easily run (TEST_FILES=./apps/sms/test/marionette/conversation_test.js). What it does is just iterates through list of nodes and queries "displayed()" property, then it adds css class to body that applies transform to the target nodes ("transform: scaleZ(0)") and iterates once again - this time displayed() returns "false" for hidden nodes.

Could you please take a look, you'll likely spot what is wrong much faster?

Thanks!

[1] https://github.com/azasypkin/gaia/tree/marionette-weird-scroll
[2] https://github.com/azasypkin/gaia/commit/fd1de25abc58063067199992e1a7a90d22b6a0fe

Comment 10

[David Burns :automatedtester]

(In reply to Oleg Zasypkin [:azasypkin][⏰UTC+1] from comment #9)
> Hey David,
> 
> I've tried to create even more reduced test case, but the issue isn't
> reproduced with something very simple :/
> 
> So I've updated [1] and [2] to make it as simple as possible, I've just
> added very simple test inside SMS app that you can easily run
> (TEST_FILES=./apps/sms/test/marionette/conversation_test.js). What it does
> is just iterates through list of nodes and queries "displayed()" property,
> then it adds css class to body that applies transform to the target nodes
> ("transform: scaleZ(0)") and iterates once again - this time displayed()
> returns "false" for hidden nodes.
> 
> Could you please take a look, you'll likely spot what is wrong much faster?
> 
> Thanks!
> 
> [1] https://github.com/azasypkin/gaia/tree/marionette-weird-scroll
> [2]
> https://github.com/azasypkin/gaia/commit/
> fd1de25abc58063067199992e1a7a90d22b6a0fe

Just to let you know, I have seen your n-i but am swamped from being away last week for W3C things. I will try get to this later in the week.

leaving n-i in place

Comment 11

[Oleg Zasypkin [:azasypkin]]

(In reply to David Burns :automatedtester from comment #10)
> Just to let you know, I have seen your n-i but am swamped from being away
> last week for W3C things. I will try get to this later in the week.
> 
> leaving n-i in place

Thanks for the note! There is no hurry here :)

Comment 12

[Andreas Tolfsen ❲:ato❳]

(In reply to Oleg Zasypkin [:azasypkin][⏰UTC+1] from comment #0)
> While working on bug 1209419 we noticed that element.displayed() works
> inconsistently for the elements that are not visible because of scroll:

Please note that the "displayedness" concept in WebDriver/Marionette is _extremely_ vague and imprecise.  I've summarised the challenges involved in ascertaining element visibility in WebDriver here: https://sny.no/2015/10/visibility

The algorithm used by Marionette is a JavaScript atom that comes from the Selenium project.  Since it isn't documented anywhere, the best source of truth is the implementation: https://github.com/SeleniumHQ/selenium/blob/master/javascript/atoms/dom.js#L566

Note that the description of "element displayedness" in the WebDriver specification that dburns linked above _is not_ a reflection of the Selenium atom, and you should disregard this when trying to understand what is happening here.
 
> 1) Element.displayed() is always "true" for the elements that are rendered,
> but not visible because of scroll;

As far as I can tell the visibility atom doesn't take into account that an element is outside the viewport.

> 2) If Element that is hidden behind scroll has CSS transform applied (in
> reduced test case "transform:translateX(0.01rem)"), then Element.displayed()
> returns "false" if element is rendered but not visible because of scroll,
> and "true" once we scroll to this element (element appears in the viewport).

I'm not sure if I understand this completely, but note that the visibility atom is, despite its name, not run "atomically" in content.  This means a transition that moves the element, or a scroll, is bound to impact the calculations and assumptions it makes, since the algorithm isn't being run on a snapshot of the document or in a sandbox.

> Messages app use case is a bit more complex: we have scrollable list of the
> messages and every messages contains "checkbox", that smoothly appears using
> "transform: translate". So in test we want to check that every message has
> checkbox, but message.displayed() and checkbox-inside-message.displayed()
> return opposite values. If at least message.displayed() returns "false" when
> it's hidden behind scroll we'd know that we should scroll to see the message.

What are the resolved CSS properties for the elements that fail the displayed check here?  Note that the algorithm does not know anything about absolute positioning (it relies entirely on tree traversal and the relationship between nodes in the document) or CSS3 transforms.

Comment 13

[Oleg Zasypkin [:azasypkin]]

Sorry, somehow missed your reply :/

> What are the resolved CSS properties for the elements that fail the displayed check here?

What properties are interested in specifically?

ni? myself to dump getComputedStyle output here, however I believe it should be the same before and after applying "transform: scaleZ(0)" (except for the transform obviously).

Comment 14

[Oleg Zasypkin [:azasypkin]]

Attachment: info.zip

So this archive consists of 3 json files:

1. "initial.json" contains initial node's info - node is hidden behind scroll and Marionette considers this node as _displayed_;

2. "with-scale-z.json" file contains node info once we added special class ".scale-node" to body that applies "transform: scaleZ(1);" to every target node - now Marionette says that node _is not displayed_.

* "with-scale-z-in-view-port.json" - contains node info once it became displayed again after we scrolled up.

Every json file contains the following data:

* Whether or not node is considered as displayed by MarionetteJS;
* node's getBoundingClientRect info;
* full dump of window.getComputedStyle(node).

1st and 2nd files are identical...

So do you see anything wrong here? Full test is in [1], can be run with (TEST_FILES=./apps/sms/test/marionette/conversation_test.js make test-integration-test) in case you want to try.

--

+ I'm wondering if there is any recommended way to test scroll behavior with Marionette/WebDriver, I specifically mean the cases when we want to test that user can scroll up and actually see the node in the viewport (we had bugs when user couldn't scroll and it'd be great to use Marionette to avoid such issues in the future).

Or we have to implement this on our side?

Thanks!

[1] https://github.com/azasypkin/gaia/tree/marionette-weird-scroll

Comment 15

[Andreas Tolfsen ❲:ato❳]

I’m sorry, I don’t really have in-depth knowledge of the Selenium isDisplayed algorithm.

Comment 16

[Julien Wajsberg [:julienw]]

Andreas, can you think of somebody who would know ?

Comment 17

[Andreas Tolfsen ❲:ato❳]

(In reply to Julien Wajsberg [:julienw] from comment #16)
> Andreas, can you think of somebody who would know ?

The best advice I can give is to read the source code.  The notation of element displayedness in Selenium is arcane black magic.

I believe the best point to get started is in the Closure-compiled JavaScript found here: https://github.com/SeleniumHQ/selenium/blob/master/javascript/atoms/dom.js#L672

It’s worth noting that the W3C WebDriver WG had hoped to include a command for element visibility in the specification, but that due to lacking platform primitives we have for the time being given up specifying it.

An old attempt at an improved displayedness algorithm can be read in the appendix of the specification, but it’s _not_ the same as the one used by Selenium: https://w3c.github.io/webdriver/webdriver-spec.html#element-displayedness

I’ve also written about the futility of visibility tests here: https://sny.no/2015/10/visibility

Comment 18

[Firefox Bug Husbandry Bot]

Bulk closed as part of https://bugzilla.mozilla.org/show_bug.cgi?id=1422750