Changeset 216259 in webkit


Ignore:
Timestamp:
May 5, 2017, 12:26:11 PM (8 years ago)
Author:
Chris Dumez
Message:

Attr Nodes should not have children
​https://bugs.webkit.org/show_bug.cgi?id=171688
<rdar://problem/31998412>

Reviewed by Andreas Kling.

Source/WebCore:

Attr Nodes should not have children as per the latest DOM specification:

Firefox and Chrome both have been matching the DOM specification for a while so I think
we should do the same. This aligns us with other browsers, simplifies the code, is
more efficient and the code being removed has been prone to security bugs.

Test: fast/dom/Attr/cannot-have-children.html

  • dom/Attr.cpp:

(WebCore::Attr::Attr):
(WebCore::Attr::create):
(WebCore::Attr::setValue):
(WebCore::Attr::cloneNodeInternal):

  • dom/Attr.h:
  • Have Attr subclass Node instead of ContainerNode as it can no longer have children.
  • Drop logic to dealing with children / creating a Text child.
  • dom/CharacterData.cpp:

(WebCore::CharacterData::notifyParentAfterChange):
Drop useless check found by the compiler. parentNode() can no longer be an Attr node.

  • dom/Node.cpp:

(WebCore::appendTextContent):
appendTextContent() is called by Node.TextContent(). For Attr Nodes, we should no longer traverse
its subtree to gather Text Nodes. Instead, we now return Attr.value, as per the specification:

  • dom/Range.cpp:

(WebCore::lengthOfContentsInNode):
As per ​https://dom.spec.whatwg.org/#concept-node-length, we should return the number of children
for Attr Nodes, which will always be 0.

  • xml/XPathUtil.cpp:

(WebCore::XPath::isValidContextNode):
Always return true for TEXT_NODE as the !(node->parentNode() && node->parentNode()->isAttributeNode())
check will also with true now. This is because a parentNode() cannot be an Attribute Node.

LayoutTests:

  • fast/dom/Attr/cannot-have-children-expected.txt: Added.
  • fast/dom/Attr/cannot-have-children.html: Added.

Add layout test coverage. I have verified that this test passes in both
Firefox and Chrome.

  • dom/html/level1/*: Removed legacy / outdated tests.
  • dom/xhtml/level1/*: Removed legacy / outdated tests.
  • fast/dom/Attr/child-nodes-cache-expected.txt: Removed.
  • fast/dom/Attr/child-nodes-cache.html: Removed.
  • fast/dom/Attr/child-nodes-length-cache-expected.txt: Removed.
  • fast/dom/Attr/child-nodes-length-cache.html: Removed.
  • fast/dom/Attr/invalidate-nodelist-after-attr-setvalue-expected.txt: Removed.
  • fast/dom/Attr/invalidate-nodelist-after-attr-setvalue.html: Removed.
  • fast/dom/attribute-change-on-mutate-expected.txt: Removed.
  • fast/dom/attribute-change-on-mutate.html: Removed.
  • svg/custom/image-with-attr-change-after-delete-crash-expected.txt: Removed.
  • svg/custom/image-with-attr-change-after-delete-crash.html: Removed.
  • traversal/moz-bug590771-expected.txt: Removed.
  • traversal/moz-bug590771.html: Removed.

Removed some outdated tests.

  • fast/custom-elements/reactions-for-webkit-extensions-expected.txt:
  • fast/custom-elements/reactions-for-webkit-extensions.html:
  • fast/dom/Attr/change-id-via-attr-node-value-expected.txt:
  • fast/dom/Attr/change-id-via-attr-node-value.html:
  • fast/dom/Element/normalize-crash.html:
  • fast/dom/Element/normalize-crash2.html:
  • fast/dom/HTMLLinkElement/event-while-removing-attribute-expected.txt:
  • fast/dom/HTMLLinkElement/event-while-removing-attribute.html:
  • fast/dom/MutationObserver/observe-attributes-expected.txt:
  • fast/dom/MutationObserver/observe-attributes.html:
  • fast/dom/import-attribute-node.html:
  • fast/dom/insertedIntoDocument-child.html:
  • fast/dom/insertedIntoDocument-sibling.html:
  • fast/dom/no-assert-for-malformed-js-url-attribute-expected.txt:
  • fast/dom/no-assert-for-malformed-js-url-attribute.html:
  • fast/dom/normalize-attributes-mutation-event-crash.html:
  • fast/dom/serialize-nodes.xhtml:
  • http/tests/security/xss-DENIED-iframe-src-alias-expected.txt:

Update existing tests so they stop relying on Attr Node having Text children.

Location:
trunk
Files:
2 added
183 deleted
26 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r216254 r216259  
     12017-05-05  Chris Dumez  <cdumez@apple.com>
     2
     3        Attr Nodes should not have children
     4        https://bugs.webkit.org/show_bug.cgi?id=171688
     5        <rdar://problem/31998412>
     6
     7        Reviewed by Andreas Kling.
     8
     9        * fast/dom/Attr/cannot-have-children-expected.txt: Added.
     10        * fast/dom/Attr/cannot-have-children.html: Added.
     11        Add layout test coverage. I have verified that this test passes in both
     12        Firefox and Chrome.
     13
     14        * dom/html/level1/*: Removed legacy / outdated tests.
     15        * dom/xhtml/level1/*: Removed legacy / outdated tests.
     16
     17        * fast/dom/Attr/child-nodes-cache-expected.txt: Removed.
     18        * fast/dom/Attr/child-nodes-cache.html: Removed.
     19        * fast/dom/Attr/child-nodes-length-cache-expected.txt: Removed.
     20        * fast/dom/Attr/child-nodes-length-cache.html: Removed.
     21        * fast/dom/Attr/invalidate-nodelist-after-attr-setvalue-expected.txt: Removed.
     22        * fast/dom/Attr/invalidate-nodelist-after-attr-setvalue.html: Removed.
     23        * fast/dom/attribute-change-on-mutate-expected.txt: Removed.
     24        * fast/dom/attribute-change-on-mutate.html: Removed.
     25        * svg/custom/image-with-attr-change-after-delete-crash-expected.txt: Removed.
     26        * svg/custom/image-with-attr-change-after-delete-crash.html: Removed.
     27        * traversal/moz-bug590771-expected.txt: Removed.
     28        * traversal/moz-bug590771.html: Removed.
     29        Removed some outdated tests.
     30
     31        * fast/custom-elements/reactions-for-webkit-extensions-expected.txt:
     32        * fast/custom-elements/reactions-for-webkit-extensions.html:
     33        * fast/dom/Attr/change-id-via-attr-node-value-expected.txt:
     34        * fast/dom/Attr/change-id-via-attr-node-value.html:
     35        * fast/dom/Element/normalize-crash.html:
     36        * fast/dom/Element/normalize-crash2.html:
     37        * fast/dom/HTMLLinkElement/event-while-removing-attribute-expected.txt:
     38        * fast/dom/HTMLLinkElement/event-while-removing-attribute.html:
     39        * fast/dom/MutationObserver/observe-attributes-expected.txt:
     40        * fast/dom/MutationObserver/observe-attributes.html:
     41        * fast/dom/import-attribute-node.html:
     42        * fast/dom/insertedIntoDocument-child.html:
     43        * fast/dom/insertedIntoDocument-sibling.html:
     44        * fast/dom/no-assert-for-malformed-js-url-attribute-expected.txt:
     45        * fast/dom/no-assert-for-malformed-js-url-attribute.html:
     46        * fast/dom/normalize-attributes-mutation-event-crash.html:
     47        * fast/dom/serialize-nodes.xhtml:
     48        * http/tests/security/xss-DENIED-iframe-src-alias-expected.txt:
     49        Update existing tests so they stop relying on Attr Node having Text children.
     50
    1512017-05-05  Ryan Haddad  <ryanhaddad@apple.com>
    252

  • trunk/LayoutTests/fast/custom-elements/reactions-for-webkit-extensions-expected.txt

    r208082 r216259  
    44PASS data on CharacterData must enqueue an attributeChanged reaction when replacing an existing attribute
    55PASS data on CharacterData must not enqueue an attributeChanged reaction when replacing an existing unobserved attribute
    6 PASS appendData on CharacterData must enqueue an attributeChangedCallback when mutating an existing attribute
    7 PASS insertData on CharacterData must enqueue an attributeChangedCallback when mutating an existing attribute
    8 PASS deleteData on CharacterData must enqueue an attributeChangedCallback when mutating an existing attribute
    9 PASS replaceData on CharacterData must enqueue an attributeChangedCallback when mutating an existing attribute
    106PASS remove(HTMLOptionElement) on HTMLOptionsCollection must enqueue disconnectedCallback when removing a custom element
    117PASS remove(HTMLOptionElement) on HTMLSelectElement must enqueue disconnectedCallback when removing a custom element

  • trunk/LayoutTests/fast/custom-elements/reactions-for-webkit-extensions.html

    r208360 r216259  
    1818
    1919testAttributeMutator(function (element, name, value) {
    20     element.attributes[name].firstChild.data = value;
     20    element.attributes[name].value = value;
    2121}, 'data on CharacterData');
    22 
    23 test(function () {
    24     var element = define_new_custom_element(['title']);
    25     var instance = document.createElement(element.name);
    26     instance.setAttribute('title', 'hello');
    27     assert_array_equals(element.takeLog().types(), ['constructed', 'attributeChanged']);
    28     instance.attributes.title.firstChild.appendData(' world');
    29     var logEntries = element.takeLog();
    30     assert_array_equals(logEntries.types(), ['attributeChanged']);
    31     assert_attribute_log_entry(logEntries.last(), {name: 'title', oldValue: 'hello', newValue: 'hello world', namespace: null});
    32 }, 'appendData on CharacterData must enqueue an attributeChangedCallback when mutating an existing attribute');
    33 
    34 test(function () {
    35     var element = define_new_custom_element(['title']);
    36     var instance = document.createElement(element.name);
    37     instance.setAttribute('title', 'foo');
    38     assert_array_equals(element.takeLog().types(), ['constructed', 'attributeChanged']);
    39     instance.attributes.title.firstChild.insertData(2, 'bar');
    40     var logEntries = element.takeLog();
    41     assert_array_equals(logEntries.types(), ['attributeChanged']);
    42     assert_attribute_log_entry(logEntries.last(), {name: 'title', oldValue: 'foo', newValue: 'fobaro', namespace: null});
    43 }, 'insertData on CharacterData must enqueue an attributeChangedCallback when mutating an existing attribute');
    44 
    45 test(function () {
    46     var element = define_new_custom_element(['title']);
    47     var instance = document.createElement(element.name);
    48     instance.setAttribute('title', 'hello world');
    49     assert_array_equals(element.takeLog().types(), ['constructed', 'attributeChanged']);
    50     instance.attributes.title.firstChild.deleteData(5, 100);
    51     var logEntries = element.takeLog();
    52     assert_array_equals(logEntries.types(), ['attributeChanged']);
    53     assert_attribute_log_entry(logEntries.last(), {name: 'title', oldValue: 'hello world', newValue: 'hello', namespace: null});
    54 }, 'deleteData on CharacterData must enqueue an attributeChangedCallback when mutating an existing attribute');
    55 
    56 test(function () {
    57     var element = define_new_custom_element(['title']);
    58     var instance = document.createElement(element.name);
    59     instance.setAttribute('title', 'hello');
    60     assert_array_equals(element.takeLog().types(), ['constructed', 'attributeChanged']);
    61     instance.attributes.title.firstChild.replaceData(1, 4, 'i');
    62     var logEntries = element.takeLog();
    63     assert_array_equals(logEntries.types(), ['attributeChanged']);
    64     assert_attribute_log_entry(logEntries.last(), {name: 'title', oldValue: 'hello', newValue: 'hi', namespace: null});
    65 }, 'replaceData on CharacterData must enqueue an attributeChangedCallback when mutating an existing attribute');
    6622
    6723test_with_window(function (contentWindow) {

  • trunk/LayoutTests/fast/dom/Attr/change-id-via-attr-node-value-expected.txt

    r211395 r216259  
    3636PASS document.body.getAttribute("id") is "f"
    3737PASS attrNode.textContent is "f"
    38 PASS attrNode.childNodes.length is 1
     38PASS attrNode.childNodes.length is 0
    3939
    40 7. Attr.replaceChild().
    41 PASS document.getElementById("f") is null
    42 PASS document.getElementById("g") is document.body
    43 PASS document.body.id is "g"
    44 PASS document.body.getAttribute("id") is "g"
    45 PASS attrNode.textContent is "g"
    46 PASS attrNode.childNodes.length is 1
    47 
    48 8. Attr.insertBefore().
    49 PASS document.getElementById("g") is null
    50 PASS document.getElementById("0g") is document.body
    51 PASS document.body.id is "0g"
    52 PASS document.body.getAttribute("id") is "0g"
    53 PASS attrNode.textContent is "0g"
    54 PASS attrNode.childNodes.length is 2
    55 
    56 9. attr.appendChild().
    57 PASS document.getElementById("0g") is null
    58 PASS document.getElementById("0g2") is document.body
    59 PASS document.body.id is "0g2"
    60 PASS document.body.getAttribute("id") is "0g2"
    61 PASS attrNode.textContent is "0g2"
    62 PASS attrNode.childNodes.length is 3
    63 
    64 10. Attr.removeChild()
    65 PASS document.body.getAttributeNode("id").childNodes.length is 0
    66 PASS document.getElementById("h") is null
    67 PASS document.getElementById("") is null
    68 PASS document.body.id is ""
    69 PASS document.body.getAttribute("id") is ""
    70 PASS document.body.getAttributeNode("id").textContent is ""
    71 
    72 11. Changing Text.nodeValue.
    73 PASS attrNode.firstChild.nodeValue is "i"
    74 PASS document.getElementById("i") is document.body
    75 PASS document.body.id is "i"
    76 PASS document.body.getAttribute("id") is "i"
    77 PASS attrNode.textContent is "i"
    78 PASS attrNode.childNodes.length is 1
    79 
    80 12. Chnaging Attr.textContent.
     407. Changing Attr.textContent.
    8141PASS document.getElementById("i") is null
    8242PASS document.getElementById("hi") is document.body
    … …  
    8444PASS document.body.getAttribute("id") is "hi"
    8545PASS attrNode.textContent is "hi"
    86 PASS attrNode.childNodes.length is 1
     46PASS attrNode.childNodes.length is 0
    8747
    88 13. Text.splitText().
     488. Node.normalize(), joining text nodes.
    8949PASS document.getElementById("hi") is document.body
    9050PASS document.body.id is "hi"
    9151PASS document.body.getAttribute("id") is "hi"
    9252PASS document.body.getAttributeNode("id").textContent is "hi"
    93 PASS document.body.getAttributeNode("id").childNodes.length is 2
     53PASS document.body.getAttributeNode("id").childNodes.length is 0
    9454
    95 14. Node.normalize(), joining text nodes.
    96 PASS document.getElementById("hi") is document.body
    97 PASS document.body.id is "hi"
    98 PASS document.body.getAttribute("id") is "hi"
    99 PASS document.body.getAttributeNode("id").textContent is "hi"
    100 PASS document.body.getAttributeNode("id").childNodes.length is 1
    101 
    102 16. Changing Text.data.
    103 PASS document.getElementById("j") is null
    104 PASS document.getElementById("k") is document.body
    105 PASS document.body.id is "k"
    106 PASS document.body.getAttribute("id") is "k"
    107 PASS attrNode.textContent is "k"
    108 PASS attrNode.childNodes.length is 1
    109 
    110 17. Changing text child with appendData().
    111 PASS document.getElementById("k") is null
    112 PASS document.getElementById("kl") is document.body
    113 PASS document.body.id is "kl"
    114 PASS document.body.getAttribute("id") is "kl"
    115 PASS attrNode.textContent is "kl"
    116 PASS attrNode.childNodes.length is 1
    117 
    118 18. Changing text child with insertData().
    119 PASS document.getElementById("kl") is null
    120 PASS document.getElementById("k1l") is document.body
    121 PASS document.body.id is "k1l"
    122 PASS document.body.getAttribute("id") is "k1l"
    123 PASS attrNode.textContent is "k1l"
    124 PASS attrNode.childNodes.length is 1
    125 
    126 19. Changing text child with deleteData().
    127 PASS document.getElementById("k1l") is null
    128 PASS document.getElementById("l") is document.body
    129 PASS document.body.id is "l"
    130 PASS document.body.getAttribute("id") is "l"
    131 PASS attrNode.textContent is "l"
    132 PASS attrNode.childNodes.length is 1
    133 
    134 20. Changing text child with replaceData().
    135 PASS document.getElementById("l") is null
    136 PASS document.getElementById("mn") is document.body
    137 PASS document.body.id is "mn"
    138 PASS document.body.getAttribute("id") is "mn"
    139 PASS attrNode.textContent is "mn"
    140 PASS attrNode.childNodes.length is 1
    141 
    142 21. Remove an Attr node.
     559. Remove an Attr node.
    14356PASS document.body.id is ""
    14457PASS document.getElementById("mn") is null
    … …  
    14659PASS document.body.getAttributeNode("id") is null
    14760
    148 22. Add an Attr node.
     6110. Add an Attr node.
    14962PASS document.getElementById("o") is document.body
    15063PASS document.body.id is "o"
    15164PASS document.body.getAttribute("id") is "o"
    15265
    153 23. Add an Attr node over an existing one.
     6611. Add an Attr node over an existing one.
    15467PASS document.getElementById("o") is null
    15568PASS document.getElementById("p") is document.body

  • trunk/LayoutTests/fast/dom/Attr/change-id-via-attr-node-value.html

    r211395 r216259  
    4747shouldBe('document.body.getAttribute("id")', '"f"');
    4848shouldBe('attrNode.textContent', '"f"');
    49 shouldBe('attrNode.childNodes.length', '1');
     49shouldBe('attrNode.childNodes.length', '0');
    5050
    51 // Firefox doesn't support these for Attr nodes.
    52 debug("\n7. Attr.replaceChild().");
    53 try {
    54     attrNode.replaceChild(document.createTextNode("g"), attrNode.firstChild);
    55     shouldBe('document.getElementById("f")', 'null');
    56     shouldBe('document.getElementById("g")', 'document.body');
    57     shouldBe('document.body.id', '"g"');
    58     shouldBe('document.body.getAttribute("id")', '"g"');
    59     shouldBe('attrNode.textContent', '"g"');
    60     shouldBe('attrNode.childNodes.length', '1');
    61 } catch (ex) {
    62     debug(ex);
    63 }
    64 
    65 debug("\n8. Attr.insertBefore().");
    66 try {
    67     attrNode.insertBefore(document.createTextNode("0"), attrNode.firstChild);
    68     shouldBe('document.getElementById("g")', 'null');
    69     shouldBe('document.getElementById("0g")', 'document.body');
    70     shouldBe('document.body.id', '"0g"');
    71     shouldBe('document.body.getAttribute("id")', '"0g"');
    72     shouldBe('attrNode.textContent', '"0g"');
    73     shouldBe('attrNode.childNodes.length', '2');
    74 } catch (ex) {
    75     debug(ex);
    76 }
    77 
    78 debug("\n9. attr.appendChild().");
    79 try {
    80     attrNode.appendChild(document.createTextNode("2"));
    81     shouldBe('document.getElementById("0g")', 'null');
    82     shouldBe('document.getElementById("0g2")', 'document.body');
    83     shouldBe('document.body.id', '"0g2"');
    84     shouldBe('document.body.getAttribute("id")', '"0g2"');
    85     shouldBe('attrNode.textContent', '"0g2"');
    86     shouldBe('attrNode.childNodes.length', '3');
    87 } catch (ex) {
    88     debug(ex);
    89 }
    90 
    91 debug("\n10. Attr.removeChild()");
    92 attrNode.nodeValue = "h";
    93 attrNode.removeChild(attrNode.firstChild);
    94 shouldBe('document.body.getAttributeNode("id").childNodes.length', '0');
    95 shouldBe('document.getElementById("h")', 'null');
    96 shouldBe('document.getElementById("")', 'null');
    97 shouldBe('document.body.id', '""');
    98 shouldBe('document.body.getAttribute("id")', '""');
    99 shouldBe('document.body.getAttributeNode("id").textContent', '""');
    100 
    101 debug("\n11. Changing Text.nodeValue.");
    102 attrNode.nodeValue = "h";
    103 attrNode.firstChild.nodeValue = "i";
    104 shouldBe('attrNode.firstChild.nodeValue', '"i"');
    105 shouldBe('document.getElementById("i")', 'document.body');
    106 shouldBe('document.body.id', '"i"');
    107 shouldBe('document.body.getAttribute("id")', '"i"');
    108 shouldBe('attrNode.textContent', '"i"');
    109 shouldBe('attrNode.childNodes.length', '1');
    110 
    111 debug("\n12. Chnaging Attr.textContent.");
     51debug("\n7. Changing Attr.textContent.");
    11252attrNode.textContent = "hi";
    11353shouldBe('document.getElementById("i")', 'null');
    … …  
    11656shouldBe('document.body.getAttribute("id")', '"hi"');
    11757shouldBe('attrNode.textContent', '"hi"');
    118 shouldBe('attrNode.childNodes.length', '1');
     58shouldBe('attrNode.childNodes.length', '0');
    11959
    120 debug("\n13. Text.splitText().");
    121 attrNode.firstChild.splitText(1);
    122 shouldBe('document.getElementById("hi")', 'document.body');
    123 shouldBe('document.body.id', '"hi"');
    124 shouldBe('document.body.getAttribute("id")', '"hi"');
    125 shouldBe('document.body.getAttributeNode("id").textContent', '"hi"');
    126 shouldBe('document.body.getAttributeNode("id").childNodes.length', '2');
    127 
    128 debug("\n14. Node.normalize(), joining text nodes.");
     60debug("\n8. Node.normalize(), joining text nodes.");
    12961attrNode.normalize();
    13062shouldBe('document.getElementById("hi")', 'document.body');
    … …  
    13264shouldBe('document.body.getAttribute("id")', '"hi"');
    13365shouldBe('document.body.getAttributeNode("id").textContent', '"hi"');
    134 shouldBe('document.body.getAttributeNode("id").childNodes.length', '1');
     66shouldBe('document.body.getAttributeNode("id").childNodes.length', '0');
    13567
    136 debug("\n16. Changing Text.data.");
    137 attrNode.firstChild.data = "k";
    138 shouldBe('document.getElementById("j")', 'null');
    139 shouldBe('document.getElementById("k")', 'document.body');
    140 shouldBe('document.body.id', '"k"');
    141 shouldBe('document.body.getAttribute("id")', '"k"');
    142 shouldBe('attrNode.textContent', '"k"');
    143 shouldBe('attrNode.childNodes.length', '1');
    144 
    145 debug("\n17. Changing text child with appendData().");
    146 attrNode.firstChild.appendData("l");
    147 shouldBe('document.getElementById("k")', 'null');
    148 shouldBe('document.getElementById("kl")', 'document.body');
    149 shouldBe('document.body.id', '"kl"');
    150 shouldBe('document.body.getAttribute("id")', '"kl"');
    151 shouldBe('attrNode.textContent', '"kl"');
    152 shouldBe('attrNode.childNodes.length', '1');
    153 
    154 debug("\n18. Changing text child with insertData().");
    155 attrNode.firstChild.insertData(1, "1");
    156 shouldBe('document.getElementById("kl")', 'null');
    157 shouldBe('document.getElementById("k1l")', 'document.body');
    158 shouldBe('document.body.id', '"k1l"');
    159 shouldBe('document.body.getAttribute("id")', '"k1l"');
    160 shouldBe('attrNode.textContent', '"k1l"');
    161 shouldBe('attrNode.childNodes.length', '1');
    162 
    163 debug("\n19. Changing text child with deleteData().");
    164 attrNode.firstChild.deleteData(0, 2);
    165 shouldBe('document.getElementById("k1l")', 'null');
    166 shouldBe('document.getElementById("l")', 'document.body');
    167 shouldBe('document.body.id', '"l"');
    168 shouldBe('document.body.getAttribute("id")', '"l"');
    169 shouldBe('attrNode.textContent', '"l"');
    170 shouldBe('attrNode.childNodes.length', '1');
    171 
    172 debug("\n20. Changing text child with replaceData().");
    173 attrNode.firstChild.replaceData(0, 1, "mn");
    174 shouldBe('document.getElementById("l")', 'null');
    175 shouldBe('document.getElementById("mn")', 'document.body');
    176 shouldBe('document.body.id', '"mn"');
    177 shouldBe('document.body.getAttribute("id")', '"mn"');
    178 shouldBe('attrNode.textContent', '"mn"');
    179 shouldBe('attrNode.childNodes.length', '1');
    180 
    181 debug("\n21. Remove an Attr node.");
     68debug("\n9. Remove an Attr node.");
    18269document.body.removeAttributeNode(attrNode);
    18370shouldBe('document.body.id', '""');
    … …  
    18673shouldBe('document.body.getAttributeNode("id")', 'null');
    18774
    188 debug("\n22. Add an Attr node.");
     75debug("\n10. Add an Attr node.");
    18976var attrNode = document.createAttribute("id");
    19077attrNode.value = "o";
    … …  
    19481shouldBe('document.body.getAttribute("id")', '"o"');
    19582
    196 debug("\n23. Add an Attr node over an existing one.");
     83debug("\n11. Add an Attr node over an existing one.");
    19784var attrNode = document.createAttribute("id");
    19885attrNode.value = "p";

  • trunk/LayoutTests/fast/dom/Element/normalize-crash.html

    r120792 r216259  
    2121
    2222elem.setAttribute("b", "a");
    23 elem.attributes[0].appendChild(document.createTextNode("hi"));
     23try {
     24    elem.attributes[0].appendChild(document.createTextNode("hi"));
     25} catch(e) {
     26}
    2427elem.attributes[0].addEventListener("DOMSubtreeModified", handler,  false);
    2528elem.normalize();

  • trunk/LayoutTests/fast/dom/Element/normalize-crash2.html

    r178363 r216259  
    66 
    77var testDiv = document.getElementById("testDiv");
    8 testDiv.attributes[0].appendChild(new Text("test"));
     8
     9try {
     10    testDiv.attributes[0].appendChild(new Text("test"));
     11} catch (e) {
     12}
     13
    914testDiv.cloneNode(false);
    1015gc();

  • trunk/LayoutTests/fast/dom/HTMLLinkElement/event-while-removing-attribute-expected.txt

    r215787 r216259  
    77PASS Before load event handled for original link element.
    88PASS Before load event handled for original link element.
    9 PASS Before load event handled for original link element.
    109PASS successfullyParsed is true
    1110

  • trunk/LayoutTests/fast/dom/HTMLLinkElement/event-while-removing-attribute.html

    r215787 r216259  
    2929
    3030    var relAttr = origLink.getAttributeNode('rel');
    31     var textNode = relAttr.childNodes[0];
    32 
    33     var newTextNode = document.createTextNode("author");
    34 
    35     relAttr.replaceChild(newTextNode, textNode);
     31    relAttr.value = "author";
    3632
    3733    setTimeout(step2, 0);

  • trunk/LayoutTests/fast/dom/MutationObserver/observe-attributes-expected.txt

    r137662 r216259  
    191191PASS mutations[0].oldValue is "foo"
    192192
    193 Test that mutating an attribute by attaching a child to an attr node delivers mutation records
    194 PASS mutations.length is 1
    195 PASS mutations[0].target is div
    196 PASS mutations[0].type is "attributes"
    197 PASS mutations[0].attributeName is "data-test"
    198 PASS mutations[0].oldValue is "foo"
    199 
    200193Test that mutating via setAttributeNode delivers mutation records
    201194PASS mutations.length is 3

  • trunk/LayoutTests/fast/dom/MutationObserver/observe-attributes.html

    r203941 r216259  
    785785        observer.observe(div, { attributes: true, attributeOldValue: true });
    786786        div.attributes['data-test'].value = 'bar';
    787 
    788         setTimeout(finish, 0);
    789     }
    790 
    791     function finish() {
    792         shouldBe('mutations.length', '1');
    793         shouldBe('mutations[0].target', 'div');
    794         shouldBe('mutations[0].type', '"attributes"');
    795         shouldBe('mutations[0].attributeName', '"data-test"');
    796         shouldBe('mutations[0].oldValue', '"foo"');
    797 
    798         observer.disconnect();
    799         debug('');
    800         runNextTest();
    801     }
    802 
    803     start();
    804 }
    805 
    806 function testMutateThroughAttrNodeChild() {
    807     var observer;
    808 
    809     function start() {
    810         debug('Test that mutating an attribute by attaching a child to an attr node delivers mutation records');
    811 
    812         mutations = null;
    813         observer = new MutationObserver(function(mutations) {
    814             window.mutations = mutations;
    815         });
    816 
    817         div = document.createElement('div');
    818         div.setAttribute('data-test', 'foo');
    819         observer.observe(div, { attributes: true, attributeOldValue: true });
    820         div.attributes['data-test'].appendChild(document.createTextNode('bar'));
    821787
    822788        setTimeout(finish, 0);
    … …  
    984950    testStyleAttributePropertyAccessIgnoreNoop,
    985951    testMutateThroughAttrNodeValue,
    986     testMutateThroughAttrNodeChild,
    987952    testSetAndRemoveAttributeNode,
    988953    testMixedNodeAndElementOperations,

  • trunk/LayoutTests/fast/dom/import-attribute-node.html

    r120792 r216259  
    1313
    1414  var importedNode = xmld2.importNode(srcElem.attributes["test"], false);
    15   if (importedNode.firstChild.nodeValue != "baz")
    16     throw "wrong imported attribute child: '" + importedNode.firstChild.nodeValue + "'";
     15  if (importedNode.nodeValue != "baz")
     16    throw "wrong imported attribute child: '" + importedNode.nodeValue + "'";
    1717
    1818  dstElem.setAttributeNode(importedNode);

  • trunk/LayoutTests/fast/dom/insertedIntoDocument-child.html

    r120792 r216259  
    2929    attr = document.createAttribute("width");
    3030    object.setAttributeNode(attr);
    31     attr.appendChild(document.createTextNode(1));
    32     attr.childNodes[0].addEventListener("DOMNodeRemoved", handler, false);
     31    attr.value = "1";
    3332
    3433    embed = document.createElement("embed");

  • trunk/LayoutTests/fast/dom/insertedIntoDocument-sibling.html

    r120792 r216259  
    3131    attr = document.createAttribute("width");
    3232    object.setAttributeNode(attr);
    33     attr.appendChild(document.createTextNode(1));
    34     attr.childNodes[0].addEventListener("DOMNodeRemoved", handler, false);
     33    attr.value += "1";
    3534   
    3635    document.body.appendChild(object);

  • trunk/LayoutTests/fast/dom/no-assert-for-malformed-js-url-attribute-expected.txt

    r214915 r216259  
    1 CONSOLE MESSAGE: line 1: SyntaxError: Unexpected identifier 'orem'
     1CONSOLE MESSAGE: line 14: SyntaxError: Unexpected identifier 'orem'
    22This tests that we do not assert when a malformed JS URL is passed to the 'src' attribute of an iframe. The test passes if it does not ASSERT.
    33

  • trunk/LayoutTests/fast/dom/no-assert-for-malformed-js-url-attribute.html

    r214915 r216259  
    1212{
    1313    var testFrame1 = document.getElementById('testFrame1');
    14     testFrame1.getAttributeNode("src").firstChild.appendData("missingFunction(this) orem ipsum dosolorem");
     14    testFrame1.getAttributeNode("src").value += "missingFunction(this) orem ipsum dosolorem";
    1515
    1616    var testFrame2 = document.getElementById('testFrame2');
    17     testFrame2.getAttributeNode("src").firstChild.appendData("javascript:missingFunction(this) orem ipsum dosolorem");
     17    testFrame2.getAttributeNode("src").value += "javascript:missingFunction(this) orem ipsum dosolorem";
    1818}
    1919</script>

  • trunk/LayoutTests/fast/dom/normalize-attributes-mutation-event-crash.html

    r203535 r216259  
    1313el.setAttribute('a', 'a')
    1414el.setAttribute('b', 'b')
    15 el.attributes[1].appendChild(document.createTextNode("test"))
     15el.attributes[1].value += "test"
    1616el.attributes[1].addEventListener('DOMSubtreeModified', function() { el.removeAttribute('b') },  false)
    1717el.normalize()

  • trunk/LayoutTests/fast/dom/serialize-nodes.xhtml

    r120792 r216259  
    2929    c.setAttributeNS("urn:bar-ns", "bar:name", "two");
    3030    var attr = d.createAttributeNS(null, "name");
    31     var text = d.createTextNode("three");
    32     attr.appendChild(text);
     31    attr.value += "three";
    3332    c.setAttributeNode(attr);
    3433   

  • trunk/LayoutTests/http/tests/security/xss-DENIED-iframe-src-alias-expected.txt

    r204236 r216259  
    1 CONSOLE MESSAGE: line 85: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8080". Protocols, domains, and ports must match.
    21CONSOLE MESSAGE: line 85: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8080". Protocols, domains, and ports must match.
    32CONSOLE MESSAGE: line 85: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8080". Protocols, domains, and ports must match.
    … …  
    87CONSOLE MESSAGE: line 31: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8080". Protocols, domains, and ports must match.
    98CONSOLE MESSAGE: line 36: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8080". Protocols, domains, and ports must match.
    10 CONSOLE MESSAGE: line 42: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8080". Protocols, domains, and ports must match.
    11 CONSOLE MESSAGE: line 46: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8080". Protocols, domains, and ports must match.
    12 CONSOLE MESSAGE: line 52: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8080". Protocols, domains, and ports must match.
    13 CONSOLE MESSAGE: line 60: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8080". Protocols, domains, and ports must match.
    14 CONSOLE MESSAGE: line 64: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8080". Protocols, domains, and ports must match.
    159CONSOLE MESSAGE: line 70: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8080". Protocols, domains, and ports must match.
    1610CONSOLE MESSAGE: line 75: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8080". Protocols, domains, and ports must match.

  • trunk/Source/WebCore/ChangeLog

    r216258 r216259  
     12017-05-05  Chris Dumez  <cdumez@apple.com>
     2
     3        Attr Nodes should not have children
     4        https://bugs.webkit.org/show_bug.cgi?id=171688
     5        <rdar://problem/31998412>
     6
     7        Reviewed by Andreas Kling.
     8
     9        Attr Nodes should not have children as per the latest DOM specification:
     10        - https://dom.spec.whatwg.org/#interface-attr
     11        - https://dom.spec.whatwg.org/#dom-attr-value
     12        - https://dom.spec.whatwg.org/#concept-node-ensure-pre-insertion-validity (Step 1)
     13
     14        Firefox and Chrome both have been matching the DOM specification for a while so I think
     15        we should do the same. This aligns us with other browsers, simplifies the code, is
     16        more efficient and the code being removed has been prone to security bugs.
     17
     18        Test: fast/dom/Attr/cannot-have-children.html
     19
     20        * dom/Attr.cpp:
     21        (WebCore::Attr::Attr):
     22        (WebCore::Attr::create):
     23        (WebCore::Attr::setValue):
     24        (WebCore::Attr::cloneNodeInternal):
     25        * dom/Attr.h:
     26        - Have Attr subclass Node instead of ContainerNode as it can no longer have children.
     27        - Drop logic to dealing with children / creating a Text child.
     28
     29        * dom/CharacterData.cpp:
     30        (WebCore::CharacterData::notifyParentAfterChange):
     31        Drop useless check found by the compiler. parentNode() can no longer be an Attr node.
     32
     33        * dom/Node.cpp:
     34        (WebCore::appendTextContent):
     35        appendTextContent() is called by Node.TextContent(). For Attr Nodes, we should no longer traverse
     36        its subtree to gather Text Nodes. Instead, we now return Attr.value, as per the specification:
     37        - https://dom.spec.whatwg.org/#dom-node-textcontent
     38
     39        * dom/Range.cpp:
     40        (WebCore::lengthOfContentsInNode):
     41        As per https://dom.spec.whatwg.org/#concept-node-length, we should return the number of children
     42        for Attr Nodes, which will always be 0.
     43
     44        * xml/XPathUtil.cpp:
     45        (WebCore::XPath::isValidContextNode):
     46        Always return true for TEXT_NODE as the !(node->parentNode() && node->parentNode()->isAttributeNode())
     47        check will also with true now. This is because a parentNode() cannot be an Attribute Node.
     48
    1492017-05-05  Brian Burg  <bburg@apple.com>
    250

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

    r215787 r216259  
    4141
    4242Attr::Attr(Element& element, const QualifiedName& name)
    43     : ContainerNode(element.document())
     43    : Node(element.document(), CreateOther)
    4444    , m_element(&element)
    4545    , m_name(name)
    … …  
    4848
    4949Attr::Attr(Document& document, const QualifiedName& name, const AtomicString& standaloneValue)
    50     : ContainerNode(document)
     50    : Node(document, CreateOther)
    5151    , m_name(name)
    5252    , m_standaloneValue(standaloneValue)
    … …  
    5656Ref<Attr> Attr::create(Element& element, const QualifiedName& name)
    5757{
    58     Ref<Attr> attr = adoptRef(*new Attr(element, name));
    59     attr->createTextChild();
    60     return attr;
     58    return adoptRef(*new Attr(element, name));
    6159}
    6260
    6361Ref<Attr> Attr::create(Document& document, const QualifiedName& name, const AtomicString& value)
    6462{
    65     Ref<Attr> attr = adoptRef(*new Attr(document, name, value));
    66     attr->createTextChild();
    67     return attr;
     63    return adoptRef(*new Attr(document, name, value));
    6864}
    6965
    7066Attr::~Attr()
    7167{
    72 }
    73 
    74 void Attr::createTextChild()
    75 {
    76     ASSERT(refCount());
    77     if (!value().isEmpty()) {
    78         auto textNode = document().createTextNode(value().string());
    79 
    80         // This does everything appendChild() would do in this situation (assuming m_ignoreChildrenChanged was set),
    81         // but much more efficiently.
    82         textNode->setParentNode(this);
    83         setFirstChild(textNode.ptr());
    84         setLastChild(textNode.ptr());
    85     }
    8668}
    8769
    … …  
    10587void Attr::setValue(const AtomicString& value)
    10688{
    107     EventQueueScope scope;
    108     m_ignoreChildrenChanged++;
    109     removeChildren();
    11089    if (m_element) {
    11190        Style::AttributeChangeInvalidation styleInvalidation(*m_element, qualifiedName(), elementAttribute().value(), value);
    … …  
    11392    } else
    11493        m_standaloneValue = value;
    115     createTextChild();
    116     m_ignoreChildrenChanged--;
    11794
    11895    invalidateNodeListAndCollectionCachesInAncestors(&m_name, m_element);
    … …  
    137114Ref<Node> Attr::cloneNodeInternal(Document& targetDocument, CloningOperation)
    138115{
    139     Ref<Attr> clone = adoptRef(*new Attr(targetDocument, qualifiedName(), value()));
    140     cloneChildNodes(clone);
    141     return WTFMove(clone);
    142 }
    143 
    144 // DOM Section 1.1.1
    145 bool Attr::childTypeAllowed(NodeType type) const
    146 {
    147     return type == TEXT_NODE;
    148 }
    149 
    150 void Attr::childrenChanged(const ChildChange&)
    151 {
    152     if (m_ignoreChildrenChanged > 0)
    153         return;
    154 
    155     invalidateNodeListAndCollectionCachesInAncestors(&qualifiedName(), m_element);
    156 
    157     StringBuilder valueBuilder;
    158     TextNodeTraversal::appendContents(*this, valueBuilder);
    159 
    160     AtomicString oldValue = value();
    161     AtomicString newValue = valueBuilder.toAtomicString();
    162     if (m_element)
    163         m_element->willModifyAttribute(qualifiedName(), oldValue, newValue);
    164 
    165     if (m_element) {
    166         Style::AttributeChangeInvalidation styleInvalidation(*m_element, qualifiedName(), oldValue, newValue);
    167         elementAttribute().setValue(newValue);
    168     } else
    169         m_standaloneValue = newValue;
    170 
    171     if (m_element) {
    172         NoEventDispatchAssertion::DisableAssertionsInScope allowedScope;
    173         m_element->attributeChanged(qualifiedName(), oldValue, newValue);
    174     }
     116    return adoptRef(*new Attr(targetDocument, qualifiedName(), value()));
    175117}
    176118

  • trunk/Source/WebCore/dom/Attr.h

    r211395 r216259  
    3434class MutableStyleProperties;
    3535
    36 // Attr can have Text children
    37 // therefore it has to be a fullblown Node. The plan
    38 // is to dynamically allocate a textchild and store the
    39 // resulting nodevalue in the attribute upon
    40 // destruction. however, this is not yet implemented.
    41 
    42 class Attr final : public ContainerNode {
     36class Attr final : public Node {
    4337public:
    4438    static Ref<Attr> create(Element&, const QualifiedName&);
    … …  
    7064    Attr(Document&, const QualifiedName&, const AtomicString& value);
    7165
    72     void createTextChild();
    73 
    7466    String nodeName() const final { return name(); }
    7567    NodeType nodeType() const final { return ATTRIBUTE_NODE; }
    … …  
    8375
    8476    bool isAttributeNode() const final { return true; }
    85     bool childTypeAllowed(NodeType) const final;
    86 
    87     void childrenChanged(const ChildChange&) final;
    8877
    8978    Attribute& elementAttribute();
    … …  
    9685
    9786    RefPtr<MutableStyleProperties> m_style;
    98     unsigned m_ignoreChildrenChanged { 0 };
    9987};
    10088

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

    r214915 r216259  
    210210void CharacterData::notifyParentAfterChange(ContainerNode::ChildChangeSource source)
    211211{
    212 #if !ASSERT_DISABLED
    213     auto assertNoEventDispatch = std::make_unique<NoEventDispatchAssertion>();
    214 #endif
    215 
    216212    document().incDOMTreeVersion();
    217213
    … …  
    226222    };
    227223
    228 #if !ASSERT_DISABLED
    229     // Attribute CharacterData is expected to fire events.
    230     if (is<Attr>(*parentNode()))
    231         assertNoEventDispatch = nullptr;
    232 #endif
    233 
    234224    parentNode()->childrenChanged(change);
    235225}

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

    r214819 r216259  
    14701470    case Node::COMMENT_NODE:
    14711471        isNullString = false;
    1472         content.append(static_cast<const CharacterData*>(node)->data());
     1472        content.append(downcast<CharacterData>(*node).data());
    14731473        break;
    14741474
    14751475    case Node::PROCESSING_INSTRUCTION_NODE:
    14761476        isNullString = false;
    1477         content.append(static_cast<const ProcessingInstruction*>(node)->data());
     1477        content.append(downcast<ProcessingInstruction>(*node).data());
    14781478        break;
    14791479   
     1480    case Node::ATTRIBUTE_NODE:
     1481        isNullString = false;
     1482        content.append(downcast<Attr>(*node).value());
     1483        break;
     1484
    14801485    case Node::ELEMENT_NODE:
    14811486        if (node->hasTagName(brTag) && convertBRsToNewlines) {
    … …  
    14851490        }
    14861491        FALLTHROUGH;
    1487     case Node::ATTRIBUTE_NODE:
    14881492    case Node::DOCUMENT_FRAGMENT_NODE:
    14891493        isNullString = false;

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

    r215946 r216259  
    537537    switch (node.nodeType()) {
    538538    case Node::DOCUMENT_TYPE_NODE:
     539    case Node::ATTRIBUTE_NODE:
    539540        return 0;
    540541    case Node::TEXT_NODE:
    … …  
    544545        return downcast<CharacterData>(node).length();
    545546    case Node::ELEMENT_NODE:
    546     case Node::ATTRIBUTE_NODE:
    547547    case Node::DOCUMENT_NODE:
    548548    case Node::DOCUMENT_FRAGMENT_NODE:

  • trunk/Source/WebCore/xml/XPathUtil.cpp

    r204466 r216259  
    6666        case Node::ELEMENT_NODE:
    6767        case Node::PROCESSING_INSTRUCTION_NODE:
     68        case Node::TEXT_NODE:
    6869            return true;
    6970        case Node::DOCUMENT_FRAGMENT_NODE:
    7071        case Node::DOCUMENT_TYPE_NODE:
    7172            return false;
    72         case Node::TEXT_NODE:
    73             return !(node->parentNode() && node->parentNode()->isAttributeNode());
    7473    }
    7574    ASSERT_NOT_REACHED();
Note: See TracChangeset for help on using the changeset viewer.