Changeset 278821 in webkit


Ignore:
Timestamp:
Jun 13, 2021 12:16:48 PM (13 months ago)
Author:
Chris Dumez
Message:

Relax "parent must be an HTMLElement" restriction in outerHTML setter
​https://bugs.webkit.org/show_bug.cgi?id=226808

Reviewed by Ryosuke Niwa.

Source/WebCore:

Made the following change to our outerHTML setter for better compatibility and to better
match the specification [1]:

  • Stop throwing an exception when the parent is not an HTML element. This new behavior matches the specification, Blink and Gecko behavior.

I did not fully align us with the specification because we are mostly aligned with Blink at
the moment. In particular:

  • The specification says the outerHTML setter should be a no-op when the parent is null. Firefox matches the specification but WebKit & Blink throw a NoModificationAllowedError.
  • The specification says we should allow setting outerHTML if the parent is a DocumentFragment. Firefox allows this but WebKit & Blink throw a NoModificationAllowedError.
  • WebKit & Blink have some Text node merging logic that is not present in the specification and which Gecko doesn't implement.

[1] ​https://w3c.github.io/DOM-Parsing/#dom-element-outerhtml

Test: fast/dom/set-outer-html-special-cases.html

  • dom/Element.cpp:

(WebCore::Element::setOuterHTML):

LayoutTests:

  • fast/dom/set-outer-html-special-cases-expected.txt: Added.
  • fast/dom/set-outer-html-special-cases.html: Added.

Add layout test coverage

  • fast/dynamic/outerHTML-no-element-expected.txt:

Rebaseline test due to different exception message.

  • platform/mac-wk1/imported/w3c/web-platform-tests/mathml/relations/css-styling/padding-border-margin/margin-003-expected.txt:
  • platform/mac-wk2/imported/w3c/web-platform-tests/mathml/relations/css-styling/padding-border-margin/margin-003-expected.txt:

Rebaseline WPT test. This is actually a progression because we're no longer throwing. However, the test is still failing
later on.

Location:
trunk
Files:
3 added
3 deleted
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r278818 r278821  
     12021-06-13  Chris Dumez  <cdumez@apple.com>
     2
     3        Relax "parent must be an HTMLElement" restriction in outerHTML setter
     4        https://bugs.webkit.org/show_bug.cgi?id=226808
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        * fast/dom/set-outer-html-special-cases-expected.txt: Added.
     9        * fast/dom/set-outer-html-special-cases.html: Added.
     10        Add layout test coverage
     11
     12        * fast/dynamic/outerHTML-no-element-expected.txt:
     13        Rebaseline test due to different exception message.
     14
     15        * platform/mac-wk1/imported/w3c/web-platform-tests/mathml/relations/css-styling/padding-border-margin/margin-003-expected.txt:
     16        * platform/mac-wk2/imported/w3c/web-platform-tests/mathml/relations/css-styling/padding-border-margin/margin-003-expected.txt:
     17        Rebaseline WPT test. This is actually a progression because we're no longer throwing. However, the test is still failing
     18        later on.
     19
    1202021-06-13  Alan Bujtas  <zalan@apple.com>
    221

  • trunk/LayoutTests/fast/dynamic/outerHTML-no-element-expected.txt

    r219663 r278821  
    33If 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.
    44
    5 This test passed - expected error - NoModificationAllowedError: The object can not be modified.
     5This test passed - expected error - NoModificationAllowedError: Cannot set outerHTML on element because it doesn't have a parent

  • trunk/Source/WebCore/ChangeLog

    r278820 r278821  
     12021-06-13  Chris Dumez  <cdumez@apple.com>
     2
     3        Relax "parent must be an HTMLElement" restriction in outerHTML setter
     4        https://bugs.webkit.org/show_bug.cgi?id=226808
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        Made the following change to our outerHTML setter for better compatibility and to better
     9        match the specification [1]:
     10        - Stop throwing an exception when the parent is not an HTML element. This new behavior matches
     11          the specification, Blink and Gecko behavior.
     12
     13        I did not fully align us with the specification because we are mostly aligned with Blink at
     14        the moment. In particular:
     15        - The specification says the outerHTML setter should be a no-op when the parent is null.
     16          Firefox matches the specification but WebKit & Blink throw a NoModificationAllowedError.
     17        - The specification says we should allow setting outerHTML if the parent is a DocumentFragment.
     18          Firefox allows this but WebKit & Blink throw a NoModificationAllowedError.
     19        - WebKit & Blink have some Text node merging logic that is not present in the specification
     20          and which Gecko doesn't implement.
     21
     22        [1] https://w3c.github.io/DOM-Parsing/#dom-element-outerhtml
     23
     24        Test: fast/dom/set-outer-html-special-cases.html
     25
     26        * dom/Element.cpp:
     27        (WebCore::Element::setOuterHTML):
     28
    1292021-06-13  Sam Weinig  <weinig@apple.com>
    230

  • trunk/Source/WebCore/dom/Element.cpp

    r278277 r278821  
    32493249ExceptionOr<void> Element::setOuterHTML(const String& html)
    32503250{
    3251     auto* parentElement = this->parentElement();
    3252     if (!is<HTMLElement>(parentElement))
    3253         return Exception { NoModificationAllowedError };
    3254 
    3255     Ref<HTMLElement> parent = downcast<HTMLElement>(*parentElement);
     3251    // The specification allows setting outerHTML on an Element whose parent is a DocumentFragment and Gecko supports this.
     3252    // However, as of June 2021, Blink matches our behavior and throws a NoModificationAllowedError for non-Element parents.
     3253    RefPtr parent = parentElement();
     3254    if (UNLIKELY(!parent)) {
     3255        if (!parentNode())
     3256            return Exception { NoModificationAllowedError, "Cannot set outerHTML on element because it doesn't have a parent" };
     3257        return Exception { NoModificationAllowedError, "Cannot set outerHTML on element because its parent is not an Element" };
     3258    }
     3259
    32563260    RefPtr<Node> prev = previousSibling();
    32573261    RefPtr<Node> next = nextSibling();
    32583262
    3259     auto fragment = createFragmentForInnerOuterHTML(parent, html, AllowScriptingContent);
     3263    auto fragment = createFragmentForInnerOuterHTML(*parent, html, AllowScriptingContent);
    32603264    if (fragment.hasException())
    32613265        return fragment.releaseException();
    … …  
    32653269        return replaceResult.releaseException();
    32663270
     3271    // The following is not part of the specification but matches Blink's behavior as of June 2021.
    32673272    RefPtr<Node> node = next ? next->previousSibling() : nullptr;
    32683273    if (is<Text>(node)) {
Note: See TracChangeset for help on using the changeset viewer.