1258205 - setAttribute doesn't throw InvalidCharacterError if the attribute already exists

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Oriol Brufau [:Oriol]]

Attachment: testcase

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:48.0) Gecko/20100101 Firefox/48.0
Build ID: 20160319030558

Steps to reproduce:

Create an element with an attribute which does not match the Name production, e.g.
> <div class="test" ~="123"></div>

Use setAttribute on that element with that attribute
> el.setAttribute('~', 'bad');


Actual results:

The value of the attribute changes.


Expected results:

An InvalidCharacterError should be thrown.

(https://www.w3.org/TR/dom/#dom-element-setattribute)

Comment 1

[Oriol Brufau [:Oriol]]

Attachment: Make setAttribute check if attribute name is valid even if it already exists

Comment 2

[Andrea Marchesini [:baku]]

Comment on attachment 8740491 [details] [diff] [review]
Make setAttribute check if attribute name is valid even if it already exists

Review of attachment 8740491 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/Element.cpp
@@ +1180,5 @@
> +  aError = nsContentUtils::CheckQName(aName, false);
> +  if (aError.Failed()) {
> +    return;
> +  }
> +  

remove this extra space.

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

NI to make sure this doesn't get lost :)

Comment 4

[Oriol Brufau [:Oriol]]

Attachment: Make setAttribute check if attribute name is valid even if it already exists

Removing space.

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 8740910 [details] [diff] [review]
Make setAttribute check if attribute name is valid even if it already exists

Review of attachment 8740910 [details] [diff] [review]:
-----------------------------------------------------------------

Carrying forward r=baku

Comment 6

[Carsten Book [:Tomcat]]

does not apply cleanly:

patching file dom/base/Element.cpp
Hunk #1 FAILED at 1171
1 out of 1 hunks FAILED -- saving rejects to file dom/base/Element.cpp.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh setAttribute.patch

could you take a look, thanks!

Comment 7

[Oriol Brufau [:Oriol]]

Attachment: Make setAttribute check if attribute name is valid even if it already exists

Sorry, my local code was reasonably recent so I didn't think it was necessary to update it. But it was, because the code just below my change was modified.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cd9ea65ce9b9

Comment 9

[Carsten Book [:Tomcat]]

backedout for issues in w-p-t 2 tests like https://treeherder.mozilla.org/logviewer.html#?job_id=25874835&repo=mozilla-inbound

Comment 10

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8de93854b963

Comment 11

[Oriol Brufau [:Oriol]]

Attachment: Make setAttribute check if attribute name is valid even if it already exists

If I understand correctly, the problem was that I didn't tell the test that now the expected result is pass instead of fail.

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9f21863863d0

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9f21863863d0