The spec says: > An element is being rendered if it has any associated CSS layout boxes, SVG layout boxes, or some equivalent in other styling languages. Blink follows that almost to the letter for innerText: https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/dom/element.cc?type=cs&q=::innerText&sq=package:chromium&g=0&l=3774 Modulo display: contents. I filed https://github.com/whatwg/html/issues/3947 for that. Gecko implements a weird IsOrHasAncestorWithDisplayNone thing, which is not equivalent for fallback content. This can be seen in the attachment. I think all the cases should return the same (textContent, that is, "abc"). And thus that GetInnerText should just do something like: if (!GetPrimaryFrame() && !IsDisplayContents()) { return GetTextContent(..); }

Err.

This my improved version of the test which doesn’t use `document.write(…)` (🤮) and instead uses `document.createTextNode(…)`, `element.appendChild(…)` and template literals. This also uses element ids, rather than CSS selectors to refer to the test elements, so that the result looks better: ``` "#canvas-fallback" innerText: "", textContent: "abc" "#video-source" innerText: "abc", textContent: "abc" "#audio-source" innerText: "abc", textContent: "abc" "#display-none" innerText: "abc", textContent: "abc" ``` (results from latest nightly)

This matches other implementations and the spec for fallback content like: <canvas><div>abc (calling div.innerText). We're treating the <div> as 'rendered' because it's not in a display: none subtree, but that's not ok, since it is in fact not rendered. This was added in bug 1226293, and Boris suggested this change, but roc opposed because it'd be hard to spec properly in comment 15. Looks like the HTML spec ended up merging roc's innerText spec, and now it's spec'd in terms of 'being rendered'. I think IsOrHasAncestorWithDisplayNone just doesn't work in any reasonable way for stuff out of the flat tree. Thus I think this change is the right thing.

Comment on attachment 9002759 [details] Bug 1484855: Match the 'is rendered' definition from the spec in innerText. r=smaug Olli Pettay [:smaug] has approved the revision.

Olli, since I can't re-request review in Phab, could you take a look at the test changes from: https://phabricator.services.mozilla.com/D3887?vs=9556&id=9624 The canvas thing is bug 1485076 (not that I changed the test, but we preserve the behavior, wrongly). The rest match other engines.

Comment on attachment 9002759 [details] Bug 1484855: Match the 'is rendered' definition from the spec in innerText. r=smaug Err, see comment 5.

Comment on attachment 9002759 [details] Bug 1484855: Match the 'is rendered' definition from the spec in innerText. r=smaug Olli Pettay [:smaug] has approved the revision.

Pushed by emilio@crisal.io: https://hg.mozilla.org/integration/autoland/rev/db66903f54cc Match the 'is rendered' definition from the spec in innerText. r=smaug

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

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Upstream PR merged