679671 - element.setAttribute trim leading and trailing whitespaces when the attribute is of type long or unsigned long and the value an valid integer

Status: RESOLVED FIXED

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

Description

[Mounir Lamouri (:mounir)]

Testcase:
data:text/html,<script>var i = document.createElement("input"); i.setAttribute("maxlength", "    22    "); alert("attr: '" + i.getAttribute("maxlength") + "'");</script>

Should show '   22   ' but shows '22'. Webkit and Presto correctly show '   22  '.
Also, this is working as expected if the attribute isn't of type long (like 'type') or if the value isn't a valid integer (like '   foo   '). Note that it doesn't work for unsigned long and long attributes (can be checked with 'size').

This bug has been found during the work for a int attribute reflection test suite (bug 669578). So we do not need to write tests for this bug specifically.

The code concerned by this bug is likely here:
content/base/src/nsAttrValue.{h,cpp}
The code in nsAttrValue is called by some code here:
content/html/content/src/nsGenericHTMLElement.{h,cpp}
Actually, the macros in the header are used by HTML elements like:
content/html/content/src/nsHTMLInputElement.cpp

What is likely to happen is that the code parse the value looking for a number, found it but saves the string representation being the actual number instead of the number with the leading and trailing whitespaces.

Comment 1

[Boris Zbarsky [:bzbarsky]]

This is just a silly bug in nsAttrValue::ParseNonNegativeIntValue.  It does this:

  SetIntValueAndType(originalVal, eInteger, nsnull);

whereas it should do this:

  SetIntValueAndType(originalVal, eInteger, strict ? nsnull : &aString);

ParsePositiveIntValue has the same issue.

Mounir, do you already have tests for this that you want to attach here?

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Save the original string when parsing nonnegative or positive integer values shows them as non-strict.

Comment 3

[Mounir Lamouri (:mounir)]

Comment on attachment 555930 [details] [diff] [review]
Save the original string when parsing nonnegative or positive integer values shows them as non-strict.

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

Oups, that was my fault... Thank you for fixing that :)

r=mounir

Comment 4

[Mounir Lamouri (:mounir)]

(In reply to Boris Zbarsky (:bz) from comment #1)
> Mounir, do you already have tests for this that you want to attach here?

Bug 669578 has tests for that.

Comment 5

[Ed Morley [:emorley]]

Something in this push caused orange, so backed out of inbound:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&usebuildbot=1&rev=89b87e96dc17
http://hg.mozilla.org/integration/mozilla-inbound/rev/ef216b965a04

Comment 6

[Boris Zbarsky [:bzbarsky]]

The orange was something else.

Pushed http://hg.mozilla.org/integration/mozilla-inbound/rev/94a4a478d774

Comment 7

[Ed Morley [:emorley]]

http://hg.mozilla.org/mozilla-central/rev/94a4a478d774