WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
117507
Support latest Web IDL indexed property getters
https://bugs.webkit.org/show_bug.cgi?id=117507
Summary
Support latest Web IDL indexed property getters
Chris Dumez
Reported
2013-06-11 06:29:52 PDT
We should support the latest Web IDL indexed property getters and stop using the outdated [IndexedGetter] IDL extended attribute. This is already supported by Blink in:
https://chromiumcodereview.appspot.com/14751005
Attachments
WIP Patch
(49.47 KB, patch)
2013-06-11 08:25 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(53.64 KB, patch)
2013-06-12 06:19 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(53.69 KB, patch)
2013-06-12 06:21 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Chris Dumez
Comment 1
2013-06-11 08:25:40 PDT
Created
attachment 204337
[details]
WIP Patch
Chris Dumez
Comment 2
2013-06-12 06:19:30 PDT
Created
attachment 204430
[details]
Patch
Chris Dumez
Comment 3
2013-06-12 06:21:39 PDT
Created
attachment 204431
[details]
Patch Rebase on master.
Kentaro Hara
Comment 4
2013-06-12 06:26:39 PDT
Comment on
attachment 204430
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=204430&action=review
> Source/WebCore/bindings/scripts/CodeGenerator.pm:52 > + "octet" => 1);
I think "byte" and "octet" are supported neither CodeGeneratorJS.pm nor CodeGeneratorV8.pm. We might want to support them. (I think DataView's methods are written in custom bindings because of lack of support of "byte" and "octet".)
> Source/WebCore/html/RadioNodeList.idl:32 > + getter Node ([IsIndex,Default=Undefined] optional unsigned long index);
Not related to this patch, I wonder why we need [IsIndex] in some getters and don't need it in other getters.
Kentaro Hara
Comment 5
2013-06-12 06:27:34 PDT
(In reply to
comment #4
)
> (From update of
attachment 204430
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=204430&action=review
> > > Source/WebCore/bindings/scripts/CodeGenerator.pm:52 > > + "octet" => 1); > > I think "byte" and "octet" are supported neither CodeGeneratorJS.pm nor CodeGeneratorV8.pm. We might want to support them. (I think DataView's methods are written in custom bindings because of lack of support of "byte" and "octet".)
Of course, in a follow-up patch.
Chris Dumez
Comment 6
2013-06-12 06:40:19 PDT
Comment on
attachment 204430
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=204430&action=review
>> Source/WebCore/bindings/scripts/CodeGenerator.pm:52 >> + "octet" => 1); > > I think "byte" and "octet" are supported neither CodeGeneratorJS.pm nor CodeGeneratorV8.pm. We might want to support them. (I think DataView's methods are written in custom bindings because of lack of support of "byte" and "octet".)
Yes, I know. Better support will be needed later on. For now, this makes sure that numeric indexed getters are properly detected in the canvas typed arrays since I use those types now.
>> Source/WebCore/html/RadioNodeList.idl:32 >> + getter Node ([IsIndex,Default=Undefined] optional unsigned long index); > > Not related to this patch, I wonder why we need [IsIndex] in some getters and don't need it in other getters.
I was wondering the same. The only difference is that an INDEX_SIZE_ERR exception will be thrown if index < 0. I was thinking that maybe some specs specify this behavior while other do not? Just speculation, I am really not sure what the real reason is yet. However, it is important that such refactoring patch does not change the behavior.
WebKit Commit Bot
Comment 7
2013-06-12 07:41:32 PDT
Comment on
attachment 204431
[details]
Patch Clearing flags on attachment: 204431 Committed
r151499
: <
http://trac.webkit.org/changeset/151499
>
WebKit Commit Bot
Comment 8
2013-06-12 07:41:35 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug