Closed Bug 688093 Opened 8 years ago Closed 8 years ago

Update reflect.js for spec changes related to "-0"

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla11

People

(Reporter: Ainsley.Chong, Assigned: atulagrwl)

References

Details

Attachments

(1 file, 1 obsolete file)

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.

http://www.whatwg.org/specs/web-apps/current-work/multipage/common-microsyntaxes.html#rules-for-parsing-non-negative-integers

input[maxLength] = default value of -1
textarea[maxLength] = default value of -1
Depends on: 669578
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: x86_64 → All
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?
Spec bug filed. Hopefully, the specifications will change.
Spec bug has been fixed. Maybe we can close this bug now.
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?
Attached patch Patch v1 (obsolete) — Splinter Review
Assignee: nobody → atulagrwl
Attachment #572755 - Flags: review?(Ms2ger)
Comment on attachment 572755 [details] [diff] [review]
Patch v1

Thanks!

>+      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.
Attachment #572755 - Flags: review?(Ms2ger) → review+
Attached patch Patch v1.01Splinter Review
Change suggested by Ms2ger.
Attachment #572755 - Attachment is obsolete: true
Keywords: checkin-needed
Attachment #572787 - Flags: checkin+
Flags: in-testsuite+
Keywords: checkin-needed
Target Milestone: --- → mozilla11
https://hg.mozilla.org/mozilla-central/rev/b2f23e353c64
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Someone should update the misleading summary now.
Summary: "-0" should not reflect 0 for non-negative signed integer attributes → Update reflect.js for spec changes related to "-0"
You need to log in before you can comment on or make changes to this bug.