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

RESOLVED FIXED in mozilla9

Status

()

Core
DOM: Core & HTML
P2
normal
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mounir, Assigned: bz)

Tracking

Trunk
mozilla9
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
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.
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?
Created attachment 555930 [details] [diff] [review]
Save the original string when parsing nonnegative or positive integer values shows them as non-strict.
Attachment #555930 - Flags: review?(mounir)
Assignee: nobody → bzbarsky
Priority: -- → P2
Whiteboard: [mentor=volkmar] → [need review]
(Reporter)

Comment 3

6 years ago
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
Attachment #555930 - Flags: review?(mounir) → review+
(Reporter)

Comment 4

6 years ago
(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.
(Reporter)

Updated

6 years ago
Whiteboard: [need review] → [need landing]
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
Whiteboard: [need landing]
The orange was something else.

Pushed http://hg.mozilla.org/integration/mozilla-inbound/rev/94a4a478d774
Flags: in-testsuite?
Target Milestone: --- → mozilla9
http://hg.mozilla.org/mozilla-central/rev/94a4a478d774
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.