| Summary: | script.innerText interop issues with new lines | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Luke Warlow <lwarlow> |
| Component: | DOM | Assignee: | Nobody <webkit-unassigned> |
| Status: | NEW --- | ||
| Severity: | Normal | CC: | ahmad.saleem792, annevk, cdumez, karlcow, rniwa, webkit-bug-importer |
| Priority: | P2 | Keywords: | BrowserCompat, InRadar |
| Version: | Safari 17 | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
It's worth pointing out this only happens when the script is attached to the document *before* setting innerText to the value. If I had to hazard a guess the innerText setter is probably resulting in HTMLScriptElement::childrenChanged() being called after the first text node is inserted which executes the script early. Such an interesting bug! We don't have this test on WPT - since all browsers pass all 'setter' test: https://wpt.fyi/results/html/dom/elements/the-innertext-and-outertext-properties/innertext-setter.html?label=master&label=experimental&aligned&q=setter Would be good to add test coverage on WPT as well. I wonder if it is related to some of the failures in https://wpt.fyi/results/dom/nodes/insertion-removing-steps To note that this is working with textContent data:text/html,<body><script>const script = document.createElement('script'); document.body.appendChild(script); script.textContent = `console.log('line 1');\nconsole.log('line 2');`</script> And this is working if we do innerText without \n data:text/html,<body><script>const script = document.createElement('script'); document.body.appendChild(script); script.innerText = `console.log('line 1');console.log('line 2');`</script> So this is the processing of `\n` in the innerText setter which screws up things. https://searchfox.org/wubkat/rev/b36cbce69fddb7da33823f316bd8ead5bebee970/Source/WebCore/html/HTMLScriptElement.cpp#112-135 HTMLScriptElement::setTextContent() calls setTextContent(WTFMove(newValue)); https://searchfox.org/wubkat/rev/b36cbce69fddb7da33823f316bd8ead5bebee970/Source/WebCore/dom/Node.cpp#1774 HTMLScriptElement::setInnerText() calls setInnerText(WTFMove(newValue)); https://searchfox.org/wubkat/rev/b36cbce69fddb7da33823f316bd8ead5bebee970/Source/WebCore/html/HTMLElement.cpp#542 These two are very different. ExceptionOr<void> HTMLElement::setInnerText(String&& text) { // FIXME: This doesn't take whitespace collapsing into account at all. if (!text.contains([](UChar c) { return c == '\n' || c == '\r'; })) { stringReplaceAll(WTFMove(text)); return { }; } if (isConnected() && isTextControlInnerTextElement()) { if (!text.contains('\r')) { stringReplaceAll(WTFMove(text)); return { }; } String textWithConsistentLineBreaks = makeStringBySimplifyingNewLines(text); stringReplaceAll(WTFMove(textWithConsistentLineBreaks)); return { }; } // FIXME: This should use replaceAll(), after we fix that to work properly for DocumentFragment. // Add text nodes and <br> elements. Ref fragment = textToFragment(document(), WTFMove(text)); // It's safe to dispatch events on the new fragment since author scripts have no access to it yet. ScriptDisallowedScope::EventAllowedScope allowedScope(fragment.get()); return replaceChildrenWithFragment(*this, WTFMove(fragment)); } I wonder what String textWithConsistentLineBreaks = makeStringBySimplifyingNewLines(text); does to the string. The OUTPUT is
-------
line 1
-------
for:
console.log('line 1');\nconsole.log('line 2');
console.log('line 1');\rconsole.log('line 2');
The OUTPUT is
-------
line 1
line 2
-------
for:
console.log('line 1');console.log('line 2');
console.log('line 1');\fconsole.log('line 2');
console.log('line 1');\tconsole.log('line 2');
console.log('line 1');\vconsole.log('line 2');
https://github.com/WebKit/WebKit/blob/501b962b3159b23f860841c5b2dd0931df79f186/Source/WTF/wtf/text/StringView.cpp#L461-L481 template<typename CharacterType> static String makeStringBySimplifyingNewLinesSlowCase(const String& string, unsigned firstCarriageReturn) { unsigned length = string.length(); unsigned resultLength = firstCarriageReturn; auto characters = string.span<CharacterType>(); CharacterType* resultCharacters; auto result = String::createUninitialized(length, resultCharacters); memcpy(resultCharacters, characters.data(), firstCarriageReturn * sizeof(CharacterType)); for (unsigned i = firstCarriageReturn; i < length; ++i) { if (characters[i] != '\r') resultCharacters[resultLength++] = characters[i]; else { resultCharacters[resultLength++] = '\n'; if (i + 1 < length && characters[i + 1] == '\n') ++i; } } if (resultLength < length) result = StringImpl::createSubstringSharingImpl(*result.impl(), 0, resultLength); return result; } |
There's an interop issue with WebKit and its newline handling inside of script.innerText setter. 1. Load data:text/html,<body><script>const script = document.createElement('script'); document.body.appendChild(script); script.innerText = `console.log('line 1');\nconsole.log('line 2');`</script> in Safari. 2. See only 'line 1' is logged. Repeat in Chromium and Firefox and see line 2 is logged as well.