Clean up attribute handling: part 2 - attributeNode
authorbenjamin@webkit.org <benjamin@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 Feb 2015 22:00:50 +0000 (22:00 +0000)
committerbenjamin@webkit.org <benjamin@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 Feb 2015 22:00:50 +0000 (22:00 +0000)
commit9add6b6073b1e54f8e1f7a582304c670a1de69e6
tree8ab26b7485cd432710008e986826c9a5f1e783bf
parent9cf1198ef0e038ef1921edb9d7e846f98f11f85a
Clean up attribute handling: part 2 - attributeNode
https://bugs.webkit.org/show_bug.cgi?id=141109

Patch by Benjamin Poulain <bpoulain@apple.com> on 2015-02-02
Reviewed by Andreas Kling.

Source/WebCore:

Our implementation was covering some old legacy behaviors of Firefox,
even copying bugs in some cases.

The spec (https://dom.spec.whatwg.org) now defines the behavior precisely,
let's move a bit closer to that.

Tests: fast/dom/Element/attribute-ascii-case-insensitive-3.html
       fast/dom/Element/attribute-setAttributeNode-multiple-times.html
       fast/dom/Element/attribute-setAttributeNodeNS-multiple-times.html
       fast/dom/Element/mozilla-dom-base-tests/test_bug1075702.html
       fast/dom/Element/mozilla-dom-base-tests/test_bug339494.html
       fast/dom/Element/mozilla-dom-base-tests/test_bug364092.xhtml
       fast/dom/Element/setAttributeNode-overriding-lowercase-values.html

* dom/Element.cpp:
(WebCore::findAttrNodeInList):
New getter for the name-without-namespace case.

(WebCore::Element::setAttributeNode):
This one is the tricky one: https://dom.spec.whatwg.org/#dom-element-setattributenode

When setAttributeNode() is used with an AttributeNode without namespace,
getting the old value behaves like getAttribute(), with ASCII lowercase name matching.
When used with a namespace, getting the old value behaves like getAttributeNS().

Setting the value is a whole different story, the name used always keeps
the original case.

Now that's a bit tricky for us because AttributeNodes are just legacy stuff we don't
used internally.

We have 4 cases to handle:
1) The name being set is lowercase, there was no conflicting name on the element.
   That's easy, we just override any node that would exist, set the name otherwise.
2) The name is lowercase but there was an existing attribute for it.
   -We create a new AttributeNode for the name to represent the old name.
   -We check the names are the same with attribute.name().matches(attrNode->qualifiedName())
    and override the value.
3) The name has uppercase characters, there is no conflicting name.
   We would not find an element to remove, we just use setAttributeInternal() as usual
   to add the attribute;
4) The name has uppercase characters, there is a lowercase conflicing name.
   This is the weird behavior: we need to nuke the old attribute, then add the new attribute
   with a different case.

   First we remove the attribute with a lowercase name with removeAttributeInternal().
   That becomes the old node.

   There might still be an element of the same name as what we are trying to add. We don't want
   to add another version of the same attribute. We need to use findAttributeIndexByName() again
   to find if there is a conflicting attribute. Then we call setAttributeInternal() which handle
   the both the cases where there was an element or not.

(WebCore::Element::setAttributeNodeNS):
This should work like any "NS" method.

(WebCore::Element::removeAttributeNode):
The method removeAttributeNode() is supposed to be exact.

(WebCore::Element::getAttributeNode):
(WebCore::Element::hasAttribute):
(WebCore::Element::attrIfExists):
* dom/Element.h:
* dom/ElementData.cpp:
(WebCore::ElementData::findAttributeIndexByNameSlowCase): Deleted.
(WebCore::ElementData::findAttributeIndexByNameForAttributeNode): Deleted.
Kill the slow case, every caller has been updated now.
* dom/ElementData.h:
(WebCore::ElementData::findAttributeIndexByName):
* dom/QualifiedName.h:
(WebCore::QualifiedName::matchesIgnoringCaseForLocalName): Deleted.

LayoutTests:

Improve the coverage a little.

Not everything is right yet: some getters return an empty string when they
should return null.

* fast/dom/Element/attribute-ascii-case-insensitive-1-expected.txt:
This is now fixed :)

* fast/dom/Element/attribute-ascii-case-insensitive-3-expected.txt: Added.
* fast/dom/Element/attribute-ascii-case-insensitive-3.html: Added.
Test prefixed-like attribute defined through the parser.

* fast/dom/Element/attribute-setAttributeNode-multiple-times-expected.txt: Added.
* fast/dom/Element/attribute-setAttributeNode-multiple-times.html: Added.
Make sure we don't accumulate nodes.

* fast/dom/Element/attribute-setAttributeNodeNS-multiple-times-expected.txt: Added.
* fast/dom/Element/attribute-setAttributeNodeNS-multiple-times.html: Added.
Same without the crazy setter.

* fast/dom/Element/script-tests/getAttribute-check-case-sensitivity.js:
* fast/dom/Element/getAttribute-check-case-sensitivity-expected.txt:
With the latest spec, getting a node with any uppercase character through
getAttributeNode() always fails. Update the test to use .getAttributeNodeNS()
were needed.

* fast/dom/Element/mozilla-dom-base-tests/test_bug1075702-expected.txt: Added.
* fast/dom/Element/mozilla-dom-base-tests/test_bug1075702.html: Added.
* fast/dom/Element/mozilla-dom-base-tests/test_bug339494-expected.txt: Added.
* fast/dom/Element/mozilla-dom-base-tests/test_bug339494.html: Added.
* fast/dom/Element/mozilla-dom-base-tests/test_bug364092-expected.txt: Added.
* fast/dom/Element/mozilla-dom-base-tests/test_bug364092.xhtml: Added.
(testGetAttributeNodeMixedCase):
(testAttribNodeNamePreservesCaseGetNode):
(testAttribNodeNamePreservesCaseGetNode2):
Some related tests from Gecko, for completeness.

* fast/dom/Element/setAttributeNode-case-insensitivity-expected.txt:
* fast/dom/Element/setAttributeNode-case-insensitivity.html:
Test that the getAttribute part of setAttributeNode() do not ignore the prefix. The spec
says to use the name, not the localname.

* fast/dom/Element/setAttributeNode-for-existing-attribute-expected.txt:
* fast/dom/Element/setAttributeNode-for-existing-attribute.html:
This test was for legacy behavior that came from Firefox. Firefox does not do that anymore.
Keep the test around for regression catching, but add a sentence explaining the 'incorrect'
behavior.

* fast/dom/Element/setAttributeNode-overriding-lowercase-values-1-expected.txt: Added.
* fast/dom/Element/setAttributeNode-overriding-lowercase-values-1.html: Added.
* fast/dom/Element/setAttributeNode-overriding-lowercase-values-2-expected.txt: Added.
* fast/dom/Element/setAttributeNode-overriding-lowercase-values-2.html: Added.
Some coverage for the name overriding craziness.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@179497 268f45cc-cd09-0410-ab3c-d52691b4dbfc
30 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/dom/Element/attribute-ascii-case-insensitive-1-expected.txt
LayoutTests/fast/dom/Element/attribute-ascii-case-insensitive-3-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/Element/attribute-ascii-case-insensitive-3.html [new file with mode: 0644]
LayoutTests/fast/dom/Element/attribute-setAttributeNode-multiple-times-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/Element/attribute-setAttributeNode-multiple-times.html [new file with mode: 0644]
LayoutTests/fast/dom/Element/attribute-setAttributeNodeNS-multiple-times-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/Element/attribute-setAttributeNodeNS-multiple-times.html [new file with mode: 0644]
LayoutTests/fast/dom/Element/getAttribute-check-case-sensitivity-expected.txt
LayoutTests/fast/dom/Element/mozilla-dom-base-tests/test_bug1075702-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/Element/mozilla-dom-base-tests/test_bug1075702.html [new file with mode: 0644]
LayoutTests/fast/dom/Element/mozilla-dom-base-tests/test_bug339494-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/Element/mozilla-dom-base-tests/test_bug339494.html [new file with mode: 0644]
LayoutTests/fast/dom/Element/mozilla-dom-base-tests/test_bug364092-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/Element/mozilla-dom-base-tests/test_bug364092.xhtml [new file with mode: 0644]
LayoutTests/fast/dom/Element/script-tests/getAttribute-check-case-sensitivity.js
LayoutTests/fast/dom/Element/setAttributeNode-case-insensitivity-expected.txt
LayoutTests/fast/dom/Element/setAttributeNode-case-insensitivity.html
LayoutTests/fast/dom/Element/setAttributeNode-for-existing-attribute-expected.txt
LayoutTests/fast/dom/Element/setAttributeNode-for-existing-attribute.html
LayoutTests/fast/dom/Element/setAttributeNode-overriding-lowercase-values-1-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/Element/setAttributeNode-overriding-lowercase-values-1.html [new file with mode: 0644]
LayoutTests/fast/dom/Element/setAttributeNode-overriding-lowercase-values-2-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/Element/setAttributeNode-overriding-lowercase-values-2.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Element.cpp
Source/WebCore/dom/Element.h
Source/WebCore/dom/ElementData.cpp
Source/WebCore/dom/ElementData.h
Source/WebCore/dom/QualifiedName.h