Skip to content

Commit

Permalink
Setting outerHTML on child of DocumentFragment should not throw error
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=249737
rdar://problem/103746193

Reviewed by Chris Dumez.

This patch aligns WebKit with Gecko / Firefox and Web-Spec [1].

In 238774@main, WebKit aligned with behavior for better compatibility but
it was not correct and clarified later to not throw error [2]. This PR is
essentially revert part of 238774@main with update to align by not throwing error.

[1] https://w3c.github.io/DOM-Parsing/#dom-element-outerhtml
[2] whatwg/html#8657

* Source/WebCore/dom/Element.cpp:
(Element::setOuterHTML): Update to not throw error
* LayoutTests/fast/dynamic/outerHTML-no-element.html: Rebaselined
* LayoutTests/fast/dynamic/outerHTML-no-element-expected.txt: Rebaselined
* LayoutTests/fast/dom/set-outer-html-special-cases.html: Rebaselined
* LayoutTests/fast/dom/set-outer-html-special-cases-expected.txt: Rebaselined

Canonical link: https://commits.webkit.org/266086@main
  • Loading branch information
Ahmad-S792 authored and cdumez committed Jul 15, 2023
1 parent 3a311fd commit b41af45
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
PASS document.getElementById('svgElement').innerHTML is "<g></g>"
PASS document.documentElement.outerHTML = '' threw exception NoModificationAllowedError: Cannot set outerHTML on element because its parent is not an Element.
PASS a.parentNode is null
PASS a.outerHTML = '' threw exception NoModificationAllowedError: Cannot set outerHTML on element because it doesn't have a parent.
PASS a.outerHTML = '' did not throw exception.
PASS successfullyParsed is true

TEST COMPLETE
Expand Down
2 changes: 1 addition & 1 deletion LayoutTests/fast/dom/set-outer-html-special-cases.html
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
// say this should be a no-op though.
a = document.createElement("a");
shouldBe("a.parentNode", "null");
shouldThrowErrorName("a.outerHTML = ''", "NoModificationAllowedError");
shouldNotThrow("a.outerHTML = ''");
}
</script>
</head>
Expand Down
4 changes: 2 additions & 2 deletions LayoutTests/fast/dynamic/outerHTML-no-element-expected.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
test for 4110775 Crash will occur when double-clicking outerHTML link on W3 DOM test

If the test fails, Safari may crash, or you may see a failure message below. If the test passes, you should see a message with a description of the expected exception.
If the test fails, Safari may crash, or you may see a failure message below. If the test passes, you should see "This test passed!".

This test passed - expected error - NoModificationAllowedError: Cannot set outerHTML on element because it doesn't have a parent
This test passed!
6 changes: 3 additions & 3 deletions LayoutTests/fast/dynamic/outerHTML-no-element.html
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
<html>
<body>
<p>test for <a href="rdar://problem/4110775">4110775</a> Crash will occur when double-clicking outerHTML link on W3 DOM test</p>
<p>If the test fails, Safari may crash, or you may see a failure message below. If the test passes, you should see a message with a description of the expected exception.</p>
<p>If the test fails, Safari may crash, or you may see a failure message below. If the test passes, you should see "This test passed!".</p>
<div id="test">This test failed.</div>

<script type="text/javascript">
if (window.testRunner)
testRunner.dumpAsText();
var t = document.getElementById("test");
var outerStr = "<div id='test2'>This test failed!</div>";
var outerStr = "<div id='test2'>This test passed!</div>";
t.outerHTML = outerStr;
try {
t.outerHTML = "<div id='test2'>This test failed!!</div>"
}catch(e) {
document.getElementById("test2").outerHTML = "<div id='test2'>This test passed - expected error - " + e + "</div>";
document.getElementById("test2").outerHTML = "<div id='test2'>This test failed since threw error - " + e + "</div>";
}
</script>

Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/dom/Element.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3738,11 +3738,11 @@ String Element::outerHTML() const
ExceptionOr<void> Element::setOuterHTML(const String& html)
{
// The specification allows setting outerHTML on an Element whose parent is a DocumentFragment and Gecko supports this.
// However, as of June 2021, Blink matches our behavior and throws a NoModificationAllowedError for non-Element parents.
// https://w3c.github.io/DOM-Parsing/#dom-element-outerhtml
RefPtr parent = parentElement();
if (UNLIKELY(!parent)) {
if (!parentNode())
return Exception { NoModificationAllowedError, "Cannot set outerHTML on element because it doesn't have a parent"_s };
return { };
return Exception { NoModificationAllowedError, "Cannot set outerHTML on element because its parent is not an Element"_s };
}

Expand Down

0 comments on commit b41af45

Please sign in to comment.