1794099 - "WebDriver:GetElementText" started to fail with `TypeError: c is undefined`

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect, P1)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Originally filed as: https://github.com/mozilla/geckodriver/issues/2050

When using the following XML file as testcase the Marionette test as added afterward fails.

XML

<?xml version="1.0" encoding="UTF-8"?>
<dgbRecord xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="http://www.digibis.com/schemas/dgb-bibdetail-v1.0.xsd"><recordHeader><id>779</id><controlNumber>BAB20120007468</controlNumber><section cod="MON">GEN-Monografías</section><originalObjectType>monografia</originalObjectType><thumbnail>http://digibis-int.digibis.com:8080/digibisbib_int/i18n//api/thumbnail.do?id=2730</thumbnail><accessRestriction>digital_contents</accessRestriction><mainTitle>Se puede consultar, ver la ficha y acceder al contenido</mainTitle><publication>Madrid : Digibís, 2012</publication><publicationDate>2012</publicationDate></recordHeader><metadata format="ficha_xml"><bibliographic>
<title label="Título">Se puede consultar, ver la ficha y acceder al contenido</title>
<publication label="Publicación">Madrid : Digibís, 2012</publication>
<physicalDescription label="Descripción física">1</physicalDescription>
<note label="Notas">Prueba de accesos. Búsqueda Europeana</note>
</bibliographic></metadata></dgbRecord>

Marinoette test:

        self.marionette.navigate("file:///Users/foo/testcase.xml")
        elem = self.marionette.find_element(By.TAG_NAME, "publication")
        print(elem.text)

And here the exact failure:

ERROR _a/test_minimized.py TestMinimizedTestCase.test_case - marionette_driver.errors.UnknownException: TypeError: can't access property "display", c is undefined
stacktrace:
	$b@chrome://remote/content/marionette/atom.js:85:50
	X@chrome://remote/content/marionette/atom.js:83:252
	jc@chrome://remote/content/marionette/atom.js:96:68
	oc@chrome://remote/content/marionette/atom.js:101:95
	atom.getElementText/</<@chrome://remote/content/marionette/atom.js:104:295
	atom.getElementText/<@chrome://remote/content/marionette/atom.js:104:481
	atom.getElementText@chrome://remote/content/marionette/atom.js:104:505
	getElementText@chrome://remote/content/marionette/actors/MarionetteCommandsChild.jsm:335:22
	receiveMessage@chrome://remote/content/marionette/actors/MarionetteCommandsChild.jsm:121:31

It means that something in the imported Selenium atoms started to fail.

I can reproduce locally and will run a regression check to find out what exact change caused this regression.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

As it looks like this is a very old problem when running Firefox Nightly builds. I'm able to reproduce back to Firefox 97. So I assume it's a preference change that happened for 105 and enabled a feature by default on release which was disabled before. This makes it harder to investigate because mozregression cannot be used.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

After inspecting a couple of now globally enabled preferences I found that layout.css.computed-style.styles-outside-flat-tree makes the difference here. The value was changed on bug 1784943.

Given the minimized code of atom.js it's not fully clear to me if that is an expected behavior or not. Emilio mind having a look?

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

The uncompiled code of the getVisibleText atom can be found at:
https://github.com/SeleniumHQ/selenium/blob/dcdff48f36ace73100e0d521e307583975f3a0c0/javascript/atoms/dom.js#L1007

Comment 4

[Emilio Cobos Álvarez (:emilio)]

I assume that ni? was for me.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: WIP: Bug 1794099 - Make XML pretty-printer keep the document nodes on the flat tree.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

So, this is kind of expected, but is a side-effect of how we implement XML pretty-printing. If you toggle devtools.inspector.showAllAnonymousContent, you can see on devtools the DOM of the page is really hidden out of the flattened tree, and all the rendered stuff is in a shadow tree.

Our behavior per spec is correct: those elements shouldn't be getting styles (and were getting wrong styles before).

Now there are multiple ways of fixing it.

• The first (ideal) is fixing marionette to avoid looking at computed styles of elements out of the flattened tree (these returns a style with length == 0).
• Somehow don't do the XML pretty-printing when using marionette?
• We could also shove the elements somewhere in a <slot> in the XML pretty-printing code, so that they get back in the flat tree.

I attached a PoC for the later. The former is probably preferred but it's not too much code.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #6)

So, this is kind of expected, but is a side-effect of how we implement XML pretty-printing. If you toggle devtools.inspector.showAllAnonymousContent, you can see on devtools the DOM of the page is really hidden out of the flattened tree, and all the rendered stuff is in a shadow tree.

Thanks Emilio for explaining whereby I'm not really familiar with the underlying issue and I would have to dig deeper into that problem. Maybe James has a position here.

Our behavior per spec is correct: those elements shouldn't be getting styles (and were getting wrong styles before).

Now there are multiple ways of fixing it.

• The first (ideal) is fixing marionette to avoid looking at computed styles of elements out of the flattened tree (these returns a style with length == 0).

Per WebDriver spec we are required to use the Selenium atoms to get the visible text. But I wonder if there is something that could be done on the Atom side to get this fixed. Therefore we might have to know which specific code is causing this problem.

• Somehow don't do the XML pretty-printing when using marionette?

I've never seen yet that people use WebDriver to automate tasks for XML files. Even it will be rarely used it's a valid case. So I assume this would involve setting the above mentioned pref?

• We could also shove the elements somewhere in a <slot> in the XML pretty-printing code, so that they get back in the flat tree.

Sorry but I don't know anything about that and would like to defer to James here.

Comment 8

[James Graham [:jgraham]]

It sounds like this isn't really related to XML, so much as the selenium atom code not working well with shadow dom. We should test if the latest version of that code fixes this.

That being said, I don't know that we should do much here. The way that XML pretty printing works is pretty much an implementation detail that people shouldn't depend on. I doubt there are enough people doing this kind of thing that we should change the implementation just so that it works with some pre-shadow-dom selenium code. If people are using WebDriver for reading XML content then replacing the elem.text call with elem.getProperty("textContent") seems more likely to do what they want (and won't be doing style checks that don't make any sense in the context of an XML document).

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to James Graham [:jgraham] from comment #8)

It sounds like this isn't really related to XML, so much as the selenium atom code not working well with shadow dom. We should test if the latest version of that code fixes this.

See my comment above. It's still not working. I wonder if we should file an issue for Selenium to get this fixed?

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1784943

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:dholbert, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #9)

(In reply to James Graham [:jgraham] from comment #8)

It sounds like this isn't really related to XML, so much as the selenium atom code not working well with shadow dom. We should test if the latest version of that code fixes this.

See my comment above. It's still not working. I wonder if we should file an issue for Selenium to get this fixed?

What do you think about that, James? Not sure if it's possible but maybe worth investigating?

Comment 13

[James Graham [:jgraham]]

I wonder if we should just add a try/catch in the marionette code to handle this case. It seems worth filing an upstream bug if we can write a non-XML-pretty-print testcase that ends up with the same kind of error.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to James Graham [:jgraham] from comment #13)

I wonder if we should just add a try/catch in the marionette code to handle this case. It seems worth filing an upstream bug if we can write a non-XML-pretty-print testcase that ends up with the same kind of error.

Or we handle it differently similar to isElementEnabled in case of XML files but not sure how complicated that could be:
https://searchfox.org/mozilla-central/rev/2809416b216b498ec3d8b0e65c25a135f4f5f37d/remote/marionette/interaction.sys.mjs#729-732

The following code would make it work and I'm happy to provide a patch for Marionette:

   async getElementText(options = {}) {
     const { elem } = options;

-    return lazy.atom.getElementText(elem, this.document.defaultView);
+    if (
+      ["application/xml", "text/xml"].includes(this.document.contentType)
+    ) {
+      return elem.textContent;
+    } else {
+      return lazy.atom.getElementText(elem, this.document.defaultView);
+    }
   }

Comment 15

[James Graham [:jgraham]]

I don't think we want to switch on content-type; it's possible to have XML content types that are given style information rather than the default presentation.

From our point of view it makes sense to treat the Selenium atoms as fallible and have some reasonable fallback (e.g. to innerText or textContent) if they fail. That's even a thing the spec could require.

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Bug 1794099 - [marionette] "Get Element Text" has to return text for pretty-printed XML.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eca05a9ca8e8
[marionette] "Get Element Text" has to return text for pretty-printed XML. r=webdriver-reviewers,jgraham

Comment 18

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/36795 for changes under testing/web-platform/tests

Comment 19

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/eca05a9ca8e8

Comment 20

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+2]]

No complaints from Selenium nor any report from users yet. So I assume we are fine with that change.