Skip to content

Safely evaluate parents and path for orphan nodes. - #81

Closed
edlebert wants to merge 2 commits into
rubycdp:mainfrom
edlebert:fix-null-parentNode
Closed

Safely evaluate parents and path for orphan nodes.#81
edlebert wants to merge 2 commits into
rubycdp:mainfrom
edlebert:fix-null-parentNode

Conversation

@edlebert

Copy link
Copy Markdown

It's possible for Cuprite to evaluate the parents or the path for a node
that no longer belongs to the document.

It's possible for Cuprite to evaluate the parents or the path for a node
that no longer belongs to the document.
@route

route commented Apr 22, 2019

Copy link
Copy Markdown
Member

@edlebert could you describe scenario when it is possible? Looks like also there'a at least one failing test in all builds

@route

route commented Jan 27, 2020

Copy link
Copy Markdown
Member

I'll try to rebase and see if it's useful, didn't hear from you ;)

@edlebert

Copy link
Copy Markdown
Author

Thank you! Yes, it definitely is an issue. It can happen for "waiting" Capybara code such as expect(page).to have_text 'Some Text' when the DOM can change during evaluation.

@Mifrill

Mifrill commented Aug 29, 2021

Copy link
Copy Markdown
Contributor

okay, we have only one fail test after the merge with the updated master. Not bad! I will take a look, it is also nice to have some tests with the scenario

@route

route commented Aug 24, 2026

Copy link
Copy Markdown
Member

Thanks for the patch, and sorry this sat so long without a resolution. I tried to build a concrete repro against current main to see if the underlying bug still applies, and I don't think it's reachable anymore (if it ever was independent of what #122/#125 already fixed).

Repro attempt: using a real Chrome session via Cuprite, I located a 3-levels-deep element (html > body > div#root > div#mid > span#leaf), kept the Ruby-side node handle, then removed the whole #root subtree from document via removeChild — leaving leaf.parentNode → mid → root fully intact, just disconnected from document. That's exactly the "parent chain never reaches document" scenario this PR's parents()/path() change is meant to handle safely.

Result: both .path and a direct call to _cuprite.parents(this) raised immediately with Ferrum::NodeNotFoundError: No node with given id found / Capybara::Cuprite::ObsoleteNode — before any of Cuprite's own JS (parents()/path()) ever executes. Chrome's DevTools Protocol invalidates a node's backend id as soon as it's detached from the document, so the parent != document && parent !== null vs parent && parent !== document distinction this PR changes is dead code in that scenario — the CDP layer throws first, every time.

And Ferrum::NodeNotFoundError is already a recognized retryable error via invalid_element_errors in driver.rb (see #122), so Capybara correctly retries rather than needing the JS-level fix here.

This matches my original question in 2019 ("could you describe scenario when it is possible?"), which never got a concrete answer or test. Since I can't reproduce the failure mode on current main, and the error path it would need is already handled elsewhere, I'm closing this. The path()/parents() dedup was a reasonable cleanup on its own, but not enough to carry the PR without the bug it was meant to fix.

@route route closed this Aug 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants