Last Comment Bug 688093 - Update reflect.js for spec changes related to "-0"
: Update reflect.js for spec changes related to "-0"
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
-- normal (vote)
: mozilla11
Assigned To: Atul Aggarwal
: Andrew Overholt [:overholt]
Depends on: 669578
  Show dependency treegraph
Reported: 2011-09-20 21:24 PDT by Ainsley Chong
Modified: 2011-11-10 03:12 PST (History)
8 users (show)
mounir: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch v1 (3.10 KB, patch)
2011-11-08 01:24 PST, Atul Aggarwal
Ms2ger: review+
Details | Diff | Splinter Review
Patch v1.01 (3.22 KB, patch)
2011-11-08 05:18 PST, Atul Aggarwal
mounir: checkin+
Details | Diff | Splinter Review

Description User image Ainsley Chong 2011-09-20 21:24:31 PDT
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:5.0) Gecko/20100101 Firefox/5.0
Build ID: 20110622131247

Steps to reproduce:

Set a non-negative signed integer attribute using element.setAttribute(-0), and attempt to get the IDL reflection using element[attribute]. 

For example:

input.setAttribute(maxLength, "-0")
input[maxLength] = ?

textarea.setAttribute(maxLength, -0)
textarea[maxLength] =?

Actual results:

input[maxLength] = 0
textarea[maxLength] = 0

Expected results:

According to the spec, non-negative signed integers should fail to parse "-0" and should return the default value.

input[maxLength] = default value of -1
textarea[maxLength] = default value of -1
Comment 1 User image Mounir Lamouri (:mounir) 2011-09-20 22:35:25 PDT
I think no browser actually do what the spec requires here. I wonder if we should ask for a spec change or just have browser fixed.
Comment 2 User image :Ms2ger (⌚ UTC+1/+2) 2011-09-21 01:53:03 PDT
I think it makes more sense to get rid of the "rules for parsing non-negative integers" algorithm, and replace it with calling the "rules for parsing integers" with a >=0 check. Mounir, do you want to file?
Comment 3 User image Mounir Lamouri (:mounir) 2011-09-21 15:48:20 PDT
Spec bug filed. Hopefully, the specifications will change.
Comment 4 User image Atul Aggarwal 2011-11-06 00:59:07 PDT
Spec bug has been fixed. Maybe we can close this bug now.
Comment 5 User image :Ms2ger (⌚ UTC+1/+2) 2011-11-06 12:38:09 PST
Still need some cleanup in reflect.js. In particular, the todo about this bug should be removed, and I think we can use one regexp instead of two in stringToInteger. Want to fix?
Comment 6 User image Atul Aggarwal 2011-11-08 01:24:53 PST
Created attachment 572755 [details] [diff] [review]
Patch v1
Comment 7 User image :Ms2ger (⌚ UTC+1/+2) 2011-11-08 04:41:34 PST
Comment on attachment 572755 [details] [diff] [review]
Patch v1


>+      if (-0x80000000 <= result[1] && result[1] <= 0x7FFFFFFF) {
>+        // If the value is within allowed value range for signed/unsigned integer, return value
>+        if (nonNegative === false || 0 <= result[1]) {

I think this would probably be slightly clearer as

if ((nonNegative ? 0 : -0x80000000) <= result[1] && result[1] <= 0x7FFFFFFF)

r=me either way.
Comment 8 User image Atul Aggarwal 2011-11-08 05:18:28 PST
Created attachment 572787 [details] [diff] [review]
Patch v1.01

Change suggested by Ms2ger.
Comment 9 User image Marco Bonardo [::mak] 2011-11-09 05:18:29 PST
Comment 10 User image j.j. 2011-11-09 21:13:57 PST
Someone should update the misleading summary now.

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