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

RESOLVED FIXED in mozilla11

Status

()

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

People

(Reporter: Ainsley Chong, Assigned: Atul Aggarwal)

Tracking

Trunk
mozilla11
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

3.22 KB, patch
mounir
: checkin+
Details | Diff | Splinter Review
(Reporter)

Description

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

Updated

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

Comment 4

6 years ago
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?
(Assignee)

Comment 6

6 years ago
Created attachment 572755 [details] [diff] [review]
Patch v1
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+
(Assignee)

Comment 8

6 years ago
Created attachment 572787 [details] [diff] [review]
Patch v1.01

Change suggested by Ms2ger.
Attachment #572755 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
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
Last Resolved: 6 years ago
Resolution: --- → FIXED

Comment 10

6 years ago
Someone should update the misleading summary now.

Updated

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