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

Status: RESOLVED FIXED

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

Description

[Ainsley Chong]

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

Comment 1

[Mounir Lamouri (:mounir)]

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

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

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

[Mounir Lamouri (:mounir)]

Spec bug filed. Hopefully, the specifications will change.

Comment 4

[Atul Aggarwal]

Spec bug has been fixed. Maybe we can close this bug now.

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

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

[Atul Aggarwal]

Attachment: Patch v1

Comment 7

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

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.

Comment 8

[Atul Aggarwal]

Attachment: Patch v1.01

Change suggested by Ms2ger.

Comment 9

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

https://hg.mozilla.org/mozilla-central/rev/b2f23e353c64

Comment 10

[j.j.]

Someone should update the misleading summary now.