[BUG] Playwright fails when taking screenshot due to isVisible being true, but should be false

[danielmhair]

System info

• Playwright Version: v1.35.1, v1.36.1
• Operating System: Linux
• Browser: All
• Other info:

Source code

• I provided exact source code that allows reproducing the issue locally.

Link to the GitHub repository with the repro

https://github.com/danielmhair/playwright-display-contents-error

---

First off, you guys are making an amazing product and I absolutely love playwright. I rarely have issues and its very fast. Keep up the good work!

---

Steps

• Run the following:

  npm ci
  npx playwright install-deps
  npx playwright test tests/example.spec.ts
  

• Notice the failed tests

---

Explanation

I cannot seem to get this exact error in this case, but in my company's site, I get this.

elementHandle.screenshot: Node is either not visible or not an HTMLElement
=========================== logs ===========================
taking element screenshot
  waiting for element to be visible and stable
  element is visible and stable
============================================================

The HTML, defined in the repo above called index.html, has a div with a class tempPositionWrapper. This class has display: contents. Because of this, the element itself does not generate a box in the normal flow of the document. Instead, it behaves as if its contents were inserted directly into the parent element. This causes the element, in this case, to have a computed width, height, x, and y values of 0 when calling getBoundingClientRect(). So I'm wondering if playwright thinks it's visible since it does "technically" have width and height due to its children, but that element directly doesn't because of the style display: contents.

See the image for more clarity:

But you will notice one of the failed tests in the repo claim that the element is visible, but it should not be due to your statement in your documentation: "Element is considered visible when it has non-empty bounding box and does not have visibility:hidden computed style. Note that elements of zero size or with display:none are not considered visible", specifically when you state that "elements of zero size are not considered visible".

The reason I mention this, is because the error we get on our company's site states that "Node is either not visible or not an HTMLElement" and as you see in the HTML, our element IS an HTMLElement, so the error is assuming the node is not visible, which is correct. But because the method to get the selector did not throw "Element Not Found" (or something similar), it then proceeded to give back the element.

---

Expected

Expected isVisible() to return false
Expected queries to the element to throw "Element Not Found" or something similiar.

Actual
isVisible() returns true
Element is returned
Screenshot was called and throws "Node is either not visible or not an HTMLElement"

[retroace]

Since this is an automation framework I think a much better implementation would be to take screenshot as the element is seen on the screen. I think the behaviour shown by isVisible is correct.
So I think that playwright should be able to take screenshot of the element regardless of what property it has. When an element is visible then we should take screenshot of that element. I might be able to contribute fix to this bug. i will investigate further on this

Here's my assumption on what the fix is going to be

1. Behaviour of isVisible is returning it correctly and docs should explain this edge case
2. Screenshot should provide screenshot of the element

[dgozman]

@danielmhair Thank you for the issue and for your kind words!

The display:contents behavior is intentional, introduced in #11212. It is convenient to consider such elements visible, as long as their children are visible. Otherwise, too many valid usecases will not be possible, like clicking such a button.

The fact that screenshot fails with Node is either not visible or not an HTMLElement error is not ideal! In theory, we should be able to make it work. If not, at least improving the error message to include "display:contents" as a reason would be great.

[danielmhair]

That's a good point ☝️. Thanks for explaining it