Bug 249737

Summary: Setting outerHTML on child of DocumentFragment throws error
Product: WebKit Reporter: Nolan Lawson <nlawson>
Component: DOMAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ahmad.saleem792, cdumez, jarhar, karlcow, rniwa, webkit-bug-importer
Priority: P2 Keywords: BrowserCompat, InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=226808
Attachments:
Description Flags
Repro HTML none

Description Nolan Lawson 2022-12-21 12:15:41 PST
Created attachment 464152 [details]
Repro HTML

Steps to repro:

1. Go to https://codepen.io/nolanlawson-the-selector/pen/oNMgmXe
2. Notice that it says an error was thrown

Minimal repro:

    const fragment = new DocumentFragment()
    fragment.appendChild(document.createElement('div'))
    fragment.firstChild.outerHTML = ''

In Firefox this does not throw an Error. In Safari Technology Preview Release 146 (Safari 15.4, WebKit 16614.1.14.10.6), it throws an error:

> Cannot set outerHTML on element because its parent is not an Element

According to the spec [1]:

> If parent is a DocumentFragment, let parent be a new Element […]

So it appears that Firefox is following the spec here, but Safari is not.

Note that my repro tests both a DocumentFragment and a ShadowRoot. Since a ShadowRoot is an instance of DocumentFragment, neither test should throw an error.

Same bug filed on Chrome: https://bugs.chromium.org/p/chromium/issues/detail?id=1403060

[1]: https://w3c.github.io/DOM-Parsing/#dom-element-outerhtml
Comment 1 Karl Dubost 2022-12-22 22:57:04 PST
Blink error:

> Uncaught DOMException: Failed to set the 'outerHTML' property on 'Element': This element's parent is of type '#document-fragment', which is not an element node.
>    at <anonymous>:1:31



WebKit error:
> nomodificationallowederror: Cannot set outerHTML on element because its parent is not an Element


Firefox returns:
''


https://searchfox.org/wubkat/rev/9ced63bbda6ac9231451b1ca549a16f397e28c78/Source/WebCore/dom/Element.cpp#3554-3589

```
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.
    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 Exception { NoModificationAllowedError, "Cannot set outerHTML on element because its parent is not an Element"_s };
    }


// … cut for brevity

}

```

This was added by Chris Dumez in June 2021
https://bugs.webkit.org/show_bug.cgi?id=226808
https://github.com/WebKit/WebKit/commit/dc33e397532ef8140d9e3fa80b7723e16b863746
Comment 2 Radar WebKit Bug Importer 2022-12-28 12:16:16 PST
<rdar://problem/103746193>
Comment 3 Joey Arhar 2022-12-29 08:59:33 PST
I opened a spec issue: https://github.com/whatwg/html/issues/8657
Comment 4 Ahmad Saleem 2023-07-04 09:41:13 PDT
(In reply to Joey Arhar from comment #3)
> I opened a spec issue: https://github.com/whatwg/html/issues/8657

As per following, Safari should align with Gecko behavior and should not throw.

So I think we might need to revert Chris's patch, which made us match Blink.

https://w3c.github.io/DOM-Parsing/#dom-element-outerhtml
Comment 5 Chris Dumez 2023-07-05 09:21:52 PDT
(In reply to Ahmad Saleem from comment #4)
> (In reply to Joey Arhar from comment #3)
> > I opened a spec issue: https://github.com/whatwg/html/issues/8657
> 
> As per following, Safari should align with Gecko behavior and should not
> throw.
> 
> So I think we might need to revert Chris's patch, which made us match Blink.
> 
> https://w3c.github.io/DOM-Parsing/#dom-element-outerhtml

Yes, looks like Blink agreed on the spec issue to stop throwing so we should do the same. Do you want to do it or should I?
Comment 6 Ahmad Saleem 2023-07-05 10:54:04 PDT
PR with revert + not throw error: https://github.com/WebKit/WebKit/pull/15575
Comment 7 EWS 2023-07-15 15:06:51 PDT
Committed 266086@main (b41af45ee409): <https://commits.webkit.org/266086@main>

Reviewed commits have been landed. Closing PR #15575 and removing active labels.