Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Node is either not clickable or not an HTMLElement #8627

Closed
abozhilov opened this issue Jul 4, 2022 · 15 comments
Closed

[Bug]: Node is either not clickable or not an HTMLElement #8627

abozhilov opened this issue Jul 4, 2022 · 15 comments
Labels

Comments

@abozhilov
Copy link
Contributor

abozhilov commented Jul 4, 2022

Bug description

Steps to reproduce the problem:

Given the following simple script:

import puppeteer from 'puppeteer';

(async () => {
  const browser = await puppeteer.launch();
  const page = await browser.newPage();

  await page.goto('https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set');
  await Promise.all([
    page.waitForNavigation(),
    page.click('#set\\.prototype\\.keys > a')
  ]);
  await browser.close();
})();

It throws the following error:

throw new Error('Node is either not clickable or not an HTMLElement');

That's due to area calculations which returns 0:

function computeQuadArea(quad: Point[]): number {

Puppeteer version

15.3.0

Node.js version

16.13.0

npm version

^15.3.0

What operating system are you seeing the problem on?

Linux

Relevant log output

No response

@abozhilov abozhilov added the bug label Jul 4, 2022
@abozhilov
Copy link
Contributor Author

I'm not sure why do you use an experimental API https://chromedevtools.github.io/devtools-protocol/tot/DOM/#method-getContentQuads to get the quads?

It could be implemented same logic via Element.getBoundingClientRect() or Element.getClientRects().

@OrKoN
Copy link
Collaborator

OrKoN commented Jul 5, 2022

Although I don't know the reason why Puppeteer uses the CDP implementation experimental methods are often created specifically for Puppeteer/DevTools (and remain experimental for years because experimental does not mean it does not work as expected or not supported, just that the API might change in the future). Puppeteer does not automatically wait for the element to become visible and that error is usually an indication that an element is not visible, and, thus, not clickable.

Would you like to work on a PR that makes use of Element.getBoundingClientRect() or Element.getClientRects() and see if it improves the situation?

@abozhilov
Copy link
Contributor Author

Thanks for the clarifications about the experimental API.

Regarding the visibility, indeed the element is not visible in the viewport, but page.click scrolls the element into the view and then perform the click action. It might need couple of ms after the scrolling action to render the element properly.

Anyway, I'll submit a PR and we can have a further discussion.

@abozhilov abozhilov reopened this Jul 5, 2022
@jrandolf
Copy link
Contributor

jrandolf commented Jul 5, 2022

@abozhilov The reason we don't use Element.getBoundingClientRect() is because not every ElementHandle handles Element (counterintuitive, I know; we have a PR fixing this). For example, you can do the following:

import puppeteer from "puppeteer";
(async () => {
  const browser = await puppeteer.launch();
  const page = await browser.newPage();
  const timeout = 30000;
  page.setDefaultTimeout(timeout);
  await page.goto("https://example.com");
  const handle = await page.$x("//html/body/div/p[1]/text()");
  console.log(await handle[0].clickablePoint());
  await browser.close();
})();

See #8552

@abozhilov
Copy link
Contributor Author

abozhilov commented Jul 5, 2022

@jrandolf that's true, and what I thought is to perform a feature detection for getBoundingClientRect and obviously it should fail for TextNodes and element that doesn't support it.
Hopefully the PR you mentioned would fix the problem as it's really important.

@jrandolf
Copy link
Contributor

jrandolf commented Jul 5, 2022

The PR I mentioned doesn't fix the problem. It just fixes the incorrect semantics of the situation. In order to get this working, it seems you need to wait for the selector to appear before clicking. Have you tried this?

@abozhilov
Copy link
Contributor Author

Yes, I've tried. The element is in the DOM tree. As I mentioned the problem comes from getting element Quads and area calculation which returns 0.

@jrandolf
Copy link
Contributor

jrandolf commented Jul 5, 2022

Then what I thinks make sense to do next is to attempt the CDP command and if it doesn't work, attempt getBoundingClientRect (if it exists in the element).

@abozhilov
Copy link
Contributor Author

Agreed! I'll try to submit a PR in the next couple of days.

@abozhilov
Copy link
Contributor Author

abozhilov commented Jul 5, 2022

@OrKoN @jrandolf I've debugged the problem, and DOM.getContentQuads works as expected only if the element is visible on the viewport, otherwise it doesn't return proper quads.
So the problem in this scenario is not within clickablePoint function itself. It is in a private function #scrollIntoViewIfNeeded() https://github.com/puppeteer/puppeteer/blob/main/src/common/ElementHandle.ts#L249

It does use the Element.scrollIntoView function which does not block the main thread. So when I invoke page.click we have a race condition and clickablePoint is called before the scrolling action finishes.

I managed to fix it, as I used DOM.scrollIntoViewIfNeeded - https://chromedevtools.github.io/devtools-protocol/tot/DOM/#method-scrollIntoViewIfNeeded instead of the private function. It waits until the scrolling action finishes and then everything else works correctly.

Let me know if you agree to submit a PR which will replace the #scrollIntoViewIfNeeded body with a call to the devtools API.

@jrandolf
Copy link
Contributor

jrandolf commented Jul 5, 2022

SGTM!

abozhilov added a commit to abozhilov/puppeteer that referenced this issue Jul 5, 2022
This patch fixes page.#scrollIntoViewIfNeededso that it works with devtools protocol.
Now it blocks the main thread and waits until the scrolling action finishes.

Issues: puppeteer#8627, puppeteer#1805

BREAKING CHANGE: page.#scrollIntoViewIfNeeded throws erros which come from the internal implementation.
 - `Protocol error (DOM.scrollIntoViewIfNeeded): Node is detached from document`
 - `Protocol error (DOM.scrollIntoViewIfNeeded): Node does not have a layout object`
Also now it does work with TextNode properly and does not throw an error.
abozhilov added a commit to abozhilov/puppeteer that referenced this issue Jul 7, 2022
This patch fixes page.#scrollIntoViewIfNeeded, so that it works with devtools protocol.
Now it blocks the main thread and waits until the scrolling action finishes in Chrome.
Fallbacks to the old implementation if `DOM.scrollIntoViewIfNeeded` is not supported for Firefox.

Issues: puppeteer#8627, puppeteer#1805
abozhilov added a commit to abozhilov/puppeteer that referenced this issue Jul 7, 2022
This patch fixes page.#scrollIntoViewIfNeeded, so that it works with devtools protocol.
Now it blocks the main thread and waits until the scrolling action finishes in Chrome.
Fallbacks to the old implementation if `DOM.scrollIntoViewIfNeeded` is not supported for Firefox.

Issues: puppeteer#8627, puppeteer#1805
abozhilov added a commit to abozhilov/puppeteer that referenced this issue Jul 7, 2022
This patch fixes page.#scrollIntoViewIfNeeded, so that it works with devtools protocol.
Now it blocks the main thread and waits until the scrolling action finishes in Chrome.
Fallbacks to the old implementation if `DOM.scrollIntoViewIfNeeded` is not supported for Firefox.

Issues: puppeteer#8627, puppeteer#1805
OrKoN pushed a commit to abozhilov/puppeteer that referenced this issue Jul 8, 2022
This patch fixes page.#scrollIntoViewIfNeeded, so that it works with devtools protocol.
Now it blocks the main thread and waits until the scrolling action finishes in Chrome.
Fallbacks to the old implementation if `DOM.scrollIntoViewIfNeeded` is not supported for Firefox.

Issues: puppeteer#8627, puppeteer#1805
OrKoN pushed a commit that referenced this issue Jul 8, 2022
This patch fixes page.#scrollIntoViewIfNeeded, so that it works with devtools protocol.
Now it blocks the main thread and waits until the scrolling action finishes in Chrome.
Fallbacks to the old implementation if `DOM.scrollIntoViewIfNeeded` is not supported for Firefox.

Issues: #8627, #1805
@OrKoN OrKoN closed this as completed Jul 8, 2022
@akshg05
Copy link

akshg05 commented Aug 23, 2022

I am still facing this issue :/
Any workarounds please?

@namtx
Copy link

namtx commented Oct 24, 2022

The issue is still there.

@OrKoN
Copy link
Collaborator

OrKoN commented Oct 25, 2022

please open a new issue with all details and a repro!

@chiriko1
Copy link

The issue doesn't seem to be solved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants