[🐛 Bug]: "Get Text" atom returns an empty string for elements within a closed ShadowDOM

[whimboo]

What happened?

Originally filed as a geckodriver issue: mozilla/geckodriver#2097

Since Firefox 108 the Get Text atom only returns an empty string for elements inside of a ShadowDOM. Over on https://bugzilla.mozilla.org/show_bug.cgi?id=1824664 I've made some investigations and manually reverted the changes of javascript/atoms/dom.js until the problem is gone. The causing change is the following:

9e5016d#diff-53bbf9a76b3e18a714284bd37ffdbcb670cbf537319cdaee94aacca31bcc497d

CC @43081j, @shs96c

How can we reproduce the issue?

A test that I was using internally with Marionette is:

        self.marionette.navigate("https://opensource.adobe.com/spectrum-web-components/storybook/iframe.html?id=button-primary-fill-sizes--m&args=&viewMode=story")
        button = self.marionette.find_element(By.CSS_SELECTOR, "#root-inner > sp-story-decorator:nth-child(2) > sp-button:nth-child(1)")

        assert button.text == "Click Me"

Relevant log output

Nothing

Operating System

All

Selenium version

Latest

What are the browser(s) and version(s) where you see this issue?

Firefox since version 108

What are the browser driver(s) and version(s) where you see this issue?

geckodriver 0.33.0

Are you using Selenium Grid?

No

[github-actions]

@whimboo, thank you for creating this issue. We will troubleshoot it as soon as we can.

---

Info for maintainers

Triage this issue by using labels.

If information is missing, add a helpful comment and then I-issue-template label.

If the issue is a question, add the I-question label.

If the issue is valid but there is no time to troubleshoot it, consider adding the help wanted label.

If the issue requires changes or fixes from an external project (e.g., ChromeDriver, GeckoDriver, MSEdgeDriver, W3C), add the applicable G-* label, and it will provide the correct link and auto-close the issue.

After troubleshooting the issue, please add the R-awaiting answer label.

Thank you!

[whimboo]

The problem here is actually the check for the assignedSlot in bot.dom.isNodeDistributedIntoShadowDom.

[bwajtr]

I'm not sure if this makes any difference, but we've run into the same issue and I've prepared a simple demonstration project, where it can be reproduced using simple webcomponent with a <slot>, Java, Selenium 4.15.0, Firefox 119 and geckodriver 0.33.0:
https://github.com/bwajtr/selenium-firefox-gettext-webcomponents

[43081j]

@whimboo what makes you think its anything to do with isNodeDistributedIntoShadowDom?

if you're right, you're saying firefox doesn't implement assignedSlot?

just trying to figure out how you got to where you are, and see if i can help

its been a while but tldr is that we very simply just iterate over the children, and if we hit a slot, we get its assignedNodes and carry on. we correctly use isNodeDistributedIntoShadowDom to avoid traversing slotted children twice (the right place to traverse them is when we get the slot's assigned nodes). all of that looks to be working, so i think i need more info

[whimboo]

So when I remove the check for the assignedSlot in line 1371 of your patch the test case from my initial comment works just fine. But with the check it fails.

Please note that I don't have detailed knowledge yet how slots work so in case of more information I would have to dig into it myself. And Firefox supports assignedSlot since version 63.

[43081j]

yes because you will be traversing the text node as if it isn't slotted. it might be making it work but it isn't whats wrong afaict

it just happens to work because you're treating the text node as a regular non-slotted node at that point

which suggests the bug actually lives inside the code which traverses a slot's assignedNodes (inside appendVisibleTextLinesFromNodeInComposedDom_)

given this html:

<some-element>
  #shadow-root
    <slot></slot>
  #text("Some text")
</some-element>

we basically do:

• traverse some-element children
• realise it has a text node which has an assignedSlot, so we ignore it
• traverse its shadow root
• reach the slot at some point, get its assignedNodes
• visit the text node (a 2nd time) but this time don't discard it

[43081j]

here's the logic which does that stuff:

selenium/javascript/atoms/dom.js

Lines 1313 to 1337 in 573eee0

	if (bot.dom.isElement(node, 'CONTENT') || bot.dom.isElement(node, 'SLOT')) {
	var parentNode = node;
	while (parentNode.parentNode) {
	parentNode = parentNode.parentNode;
	}
	if (parentNode instanceof ShadowRoot) {
	// If the element is <content> and we're inside a shadow DOM then just
	// append the contents of the nodes that have been distributed into it.
	var contentElem = /** @type {!Object} */ (node);
	var shadowChildren;
	if (bot.dom.isElement(node, 'CONTENT')) {
	shadowChildren = contentElem.getDistributedNodes();
	} else {
	shadowChildren = contentElem.assignedNodes();
	}
	goog.array.forEach(shadowChildren, function(node) {
	bot.dom.appendVisibleTextLinesFromNodeInComposedDom_(
	node, lines, shown, whitespace, textTransform);
	});
	} else {
	// if we're not inside a shadow DOM, then we just treat <content>
	// as an unknown element and use anything inside the tag
	bot.dom.appendVisibleTextLinesFromElementInComposedDom_(
	castElem, lines);
	}

its pretty old now, and tbh someone should throw away the shadow dom v0 stuff these days

but i imagine if there is a bug, its somewhere in there

ill see if i can reproduce it

[43081j]

@whimboo can you try it on your end with isShown forced to true?

i.e. this line:

selenium/javascript/atoms/dom.js

Lines 1411 to 1412 in 573eee0

	bot.dom.appendVisibleTextLinesFromNodeInComposedDom_(
	node, lines, shown, whitespace, textTransform);

just temporarily make it explicitly pass true rather than shown. i wonder if its the visibility code getting tripped up

[whimboo]

I tested with shown set to true but that doesn't help. Because the surrounding method passes null for whitespace and textTransform as well I also tested that but without changes.

[43081j]

i did try reproduce, but am struggling:
https://jsfiddle.net/xmhr2cjg/

this is basically all the dom stuff copy pasted with the goog helpers replaced

in the console, if you inspect the custom element and call getVisibleText($0) it works in both chrome and firefox for me

[whimboo]

@43081j that was actually a great idea to copy all the methods and remove deps for the Google modules. It's indeed passing for me as well when everything is executed from within the web page. As such I've also copied the full generated minified atom code for getVisibleText and used it the same way. And by surprise it works as well!

So that means that something actually causes trouble for us in Firefox when we run this generated atom code in privileged code. As such I'll go ahead and take the reduced code from your jsfiddle to try to find out what's actually broken on our side. Work will be continued to be tracked on https://bugzilla.mozilla.org/show_bug.cgi?id=1824664.

That said it looks like that there is actually no bug in the Selenium atoms code and that we do not need to keep this issue open. Sorry for wasting your time on that issue, but on the other side also a big thank you for preparing that code that I can now easily use!

[43081j]

i'm still super curious what the problem is so will be sure to track the other related issues

let me know if it ultimately does come back here and is a selenium problem after all

[whimboo]

@43081j I have actually one more question. The stripped down code of the Get Text atom as used in the jsfiddle example, from which revision has it been taken? I'm asking because in case of a closed ShadowRoot it is able to return the text of a link contained in that ShadowRoot, while the compiled versions (even the latest one from trunk) fail and return an empty string. Or maybe this is caused by the fact that the example code is using true for shown?

Note that internally we always have access to the ShadowRoot object whether the ShadowDOM is open or closed, and as such we can use querySelector to find an inner element, and trying to get details like the text from it.

Here a new https://jsfiddle.net/umzdhajn/ that demonstrates that behavior.

[whimboo]

Ok, so the problematic lines here are these:

selenium/javascript/atoms/dom.js

Lines 590 to 594 in 3041af3

	if (parent.host.shadowRoot !== parent) {
	// There is a younger shadow root, which will take precedence over
	// the shadow this element is in, thus this element won't be
	// displayed.
	return false;

[43081j]

with a closed shadow root, i'd expect it to treat the element as if there is no shadow root, i.e. just return the text

since assignedSlot will be null in those cases

i.e. in your fiddle everything inside the shadow root should be treated as if it doesn't exist (since normally we can't access a closed shadow root)

[whimboo]

But that's not how it works right now. Because of the above check it is marked as not being displayed (shown) and returns an empty string. I wonder if the above check should only be executed when parent.mode === "open, and otherwise take the default code path.

[43081j]

is that because in your case parent.host.shadowRoot is nullish from being a closed root?

do you know how we managed to get to an element inside that root in the first place?

you're probably right, just trying to get my head around how we're dealing with closed roots since i thought they wouldn't even be traversed, so wouldn't reach that point

[whimboo]

See the new fiddle for the workaround in how to get into a closed shadow root. With that you can retrieve the element and call the atom on.

In Firefox we won't need such a workaround given that for privileged code we have a openOrClosedShadowRoot property that can always be accessed whether if the Shadow DOM is open or closed.

[whimboo]

While this check may have side-effects it at least would give code a chance to retrieve the visible text at all from the element. If there is agreement I could at least create a PR and try to get a test written as well for it.

[43081j]

ah interesting, so you expose the closed root as a property on your components

yeah for sure thats what'll be tripping things up since host.shadowRoot will be nullish

really you want a way to define how it should get the shadow root i think. i.e. a way to say "anytime you try access shadowRoot, actually use openOrClosedShadowRoot and it'd naturally all work as-is

[whimboo]

Sadly openOrClosedShadowRoot is not a public facing Web API and as such cannot be used when code is run in the scope of the page. And the thing is that with a closed Shadow DOM it's on purpose that shadowRoot is not available.

Let me re-phrase the summary of the issue and maybe @shs96c and @AutomatedTester have some further input as well.

[whimboo]

I've patched our internal usage of the atom and a try build shows no failures anymore for all the wdspec tests that cover closed Shadow DOMs.

Also no browser at the moment works for closed Shadow DOM. For Firefox some show green because we run the atoms in privileged scope where we always have access to .shadowRoot.

[43081j]

yeah im not sure how we could safely update the code over here 🤔

since the behaviour you want is only possible because you have a special environment where shadowRoot is accessible, closed or not. but in regular browser configurations this shouldn't be possible

so if we wanted the code here to do what you want, i think it'd need to allow you to customise it rather than removing/changing that check for everyone

[whimboo]

Please note that the WebDriver classic specification that is used by all browsers to retrieve the visible text relies on a given revision of the atoms. So it's not that we can customize it for our own needs.

But what would be bad on the extra closed check. Code that is not able to pierce into a closed Shadow DOM would not even be able to process elements within that one, and as such wouldn't even regress with such a change. Or do I miss something?

[43081j]

just reading through it properly now, i think it should actually be this:

      if (parent.host.shadowRoot && parent.host.shadowRoot !== parent) {
        // There is a younger shadow root, which will take precedence over
        // the shadow this element is in, thus this element won't be
        // displayed.
        return false;

i.e. the extra shadowRoot truthy check

its been years but from what i understand, this condition is in case you're somehow in a shadow root which is no longer the shadow root (how you get to that state, i dont remember... if its even possible anymore).

the check was intended to compare two non-null roots iirc, so we shouldn't be running that logic at all if either side is nullish

[whimboo]

Sorry for the delay and thank you a lot for the response. This actually sounds reasonable and I'm happy to create a PR for this particular scenario. Locally it works fine with a new test added for an element within a closed shadow root.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.