Last Comment Bug 679671 - element.setAttribute trim leading and trailing whitespaces when the attribute is of type long or unsigned long and the value an valid integer
: element.setAttribute trim leading and trailing whitespaces when the attribute...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: P2 normal (vote)
: mozilla9
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
:
: Andrew Overholt [:overholt]
Mentors:
Depends on: 669578
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-17 05:27 PDT by Mounir Lamouri (:mounir)
Modified: 2011-09-01 02:00 PDT (History)
4 users (show)
bzbarsky: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Save the original string when parsing nonnegative or positive integer values shows them as non-strict. (1.38 KB, patch)
2011-08-25 19:47 PDT, Boris Zbarsky [:bz] (still a bit busy)
mounir: review+
Details | Diff | Splinter Review

Description Mounir Lamouri (:mounir) 2011-08-17 05:27:46 PDT
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 [:bz] (still a bit busy) 2011-08-25 19:43:00 PDT
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 [:bz] (still a bit busy) 2011-08-25 19:47:24 PDT
Created attachment 555930 [details] [diff] [review]
Save the original string when parsing nonnegative or positive integer values shows them as non-strict.
Comment 3 Mounir Lamouri (:mounir) 2011-08-26 00:37:25 PDT
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) 2011-08-26 00:39:10 PDT
(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] 2011-08-31 17:05:32 PDT
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 [:bz] (still a bit busy) 2011-08-31 21:14:34 PDT
The orange was something else.

Pushed http://hg.mozilla.org/integration/mozilla-inbound/rev/94a4a478d774
Comment 7 Ed Morley [:emorley] 2011-09-01 02:00:16 PDT
http://hg.mozilla.org/mozilla-central/rev/94a4a478d774

Note You need to log in before you can comment on or make changes to this bug.