Bug 50472 - Reflected unsigned attributes should be in the range [0, 2^31)
Summary: Reflected unsigned attributes should be in the range [0, 2^31)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Andy Estes
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-12-03 12:26 PST by Andy Estes
Modified: 2010-12-09 00:34 PST (History)
3 users (show)

See Also:


Attachments
Patch (18.20 KB, patch)
2010-12-03 16:11 PST, Andy Estes
no flags Details | Formatted Diff | Diff
Patch (21.81 KB, patch)
2010-12-06 16:24 PST, Andy Estes
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andy Estes 2010-12-03 12:26:41 PST
Reflected unsigned attributes should be in the range [0, 2^31)
Comment 1 Andy Estes 2010-12-03 16:11:35 PST
Created attachment 75573 [details]
Patch
Comment 2 Andy Estes 2010-12-03 16:15:23 PST
This patch implements the algorithm HTML5 describes for getting reflected unsigned attributes. From section 2.8.1:

"If a reflecting IDL attribute is an unsigned integer type (unsigned long) then, on getting, the content attribute must be parsed according to the rules for parsing non-negative integers, and if that is successful, and the value is in the range 0 to 2147483647 inclusive, the resulting value must be returned. If, on the other hand, it fails or returns an out of range value, or if the attribute is absent, the default value must be returned instead, or 0 if there is no default value."
Comment 3 Andy Estes 2010-12-03 16:17:07 PST
Before anyone reviews this, it might be nice to let all the EWS bots do their thing before changing the review flag since I've only tested the code generation on the Mac.
Comment 4 Darin Adler 2010-12-06 09:52:38 PST
Comment on attachment 75573 [details]
Patch

The patch looks great; we also need a test that shows the actual behavior change in running JavaScript. Something in the LayoutTests directory. I’m going to say review- because that’s missing.
Comment 5 Andy Estes 2010-12-06 16:24:43 PST
Created attachment 75749 [details]
Patch
Comment 6 Andy Estes 2010-12-06 16:26:07 PST
(In reply to comment #4)
> (From update of attachment 75573 [details])
> The patch looks great; we also need a test that shows the actual behavior change in running JavaScript. Something in the LayoutTests directory. I’m going to say review- because that’s missing.

Thanks Darin. The new patch adds test cases for marquee's hspace and vspace properties, which are both reflected and unsigned.
Comment 7 WebKit Review Bot 2010-12-06 16:27:32 PST
Attachment 75749 [details] did not pass style-queue:

Failed to run "[u'git', u'reset', u'--hard', u'HEAD']" exit_code: 128
error: Could not write new index file.
fatal: Could not reset index file to revision 'HEAD'.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Andy Estes 2010-12-08 16:37:50 PST
Committed r73564: <http://trac.webkit.org/changeset/73564>
Comment 9 WebKit Review Bot 2010-12-09 00:34:36 PST
http://trac.webkit.org/changeset/73564 might have broken GTK Linux 32-bit Release
The following tests are not passing:
fast/css/input-search-padding.html
fast/css/text-input-with-webkit-border-radius.html
fast/forms/box-shadow-override.html
fast/forms/control-restrict-line-height.html
fast/forms/input-appearance-height.html
fast/forms/placeholder-pseudo-style.html
fast/forms/placeholder-set-value.html
fast/forms/search-cancel-button-style-sharing.html
fast/forms/search-placeholder-value-changed.html
fast/forms/search-rtl.html
fast/forms/search-styled.html
fast/forms/search-transformed.html
fast/forms/search-vertical-alignment.html
fast/forms/search-zoomed.html
fast/forms/searchfield-heights.html