WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
102114
Rename NATIVE_TYPE_ERR to TypeError
https://bugs.webkit.org/show_bug.cgi?id=102114
Summary
Rename NATIVE_TYPE_ERR to TypeError
Joshua Bell
Reported
2012-11-13 11:54:23 PST
Rename NATIVE_TYPE_ERR to TypeError
Attachments
Patch
(34.75 KB, patch)
2012-11-13 12:03 PST
,
Joshua Bell
no flags
Details
Formatted Diff
Diff
Patch
(33.79 KB, patch)
2012-11-13 13:37 PST
,
Joshua Bell
no flags
Details
Formatted Diff
Diff
Patch for landing
(33.66 KB, patch)
2012-11-14 11:47 PST
,
Joshua Bell
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Joshua Bell
Comment 1
2012-11-13 12:03:45 PST
Created
attachment 173945
[details]
Patch
Joshua Bell
Comment 2
2012-11-13 12:07:06 PST
Comment on
attachment 173945
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=173945&action=review
> Source/WebCore/bindings/v8/V8ThrowException.h:32 > +enum V8ErrorType {
Prefixing "ErrorType" itself is not strictly necessary, but there seems to be precedent for prefix consistency between enum and members.
> Source/WebCore/dom/Element.cpp:-1523 > - ec = NATIVE_TYPE_ERR;
These need to be switched to TYPE_MISMATCH_ERR in another patch. Uploading here for now to exercise build bots.
> Source/WebCore/dom/Element.cpp:-1565 > - ec = NATIVE_TYPE_ERR;
Ditto.
> Source/WebCore/dom/ExceptionCode.h:68 > + // WebIDL exception types, handled by the binding layer.
I don't know if we should add all of these now given that they're unused/untested. But I wanted to see if the build bots are happy with these names.
Erik Arvidsson
Comment 3
2012-11-13 12:07:21 PST
Comment on
attachment 173945
[details]
Patch LGTM... I would r+ if I was a reviewere
Joshua Bell
Comment 4
2012-11-13 13:37:45 PST
Created
attachment 173974
[details]
Patch
Kentaro Hara
Comment 5
2012-11-13 15:40:13 PST
Comment on
attachment 173974
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=173974&action=review
> Source/WebCore/ChangeLog:9 > + V8 binding code had colliding enum members, which required prefixing.
For consistency I want to make the same change for JSC too. SyntaxError -> jsSyntaxError
Joshua Bell
Comment 6
2012-11-13 15:44:17 PST
Comment on
attachment 173974
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=173974&action=review
>> Source/WebCore/ChangeLog:9 >> + V8 binding code had colliding enum members, which required prefixing. > > For consistency I want to make the same change for JSC too. SyntaxError -> jsSyntaxError
I didn't find a similar enum in the JSC code. Can you point me at what would need updating?
Kentaro Hara
Comment 7
2012-11-13 15:45:56 PST
(In reply to
comment #6
)
> I didn't find a similar enum in the JSC code. Can you point me at what would need updating?
Ah, JSC uses createSyntaxError() etc instead of enums. Sorry, ignore my comment.
Joshua Bell
Comment 8
2012-11-13 15:48:17 PST
Comment on
attachment 173974
[details]
Patch Thanks. Going ahead with claiming WebCore::GeneralError, WebCore::RangeError etc. in the ExceptionCode enum, even though the types aren't handled by binding code yet.
Darin Adler
Comment 9
2012-11-13 17:34:37 PST
Comment on
attachment 173974
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=173974&action=review
> Source/WebCore/dom/ExceptionCode.h:75 > + GeneralError = 100, > + EvalError = 101, > + RangeError = 102, > + ReferenceError = 103, > + SyntaxError = 104, > + TypeError = 105, > + URIError = 106,
I agree that we might want to add these some day, but it seems a little dangerous to add them all now when they are not implemented in the bindings.
Joshua Bell
Comment 10
2012-11-14 11:47:24 PST
Created
attachment 174215
[details]
Patch for landing
WebKit Review Bot
Comment 11
2012-11-14 12:23:15 PST
Comment on
attachment 174215
[details]
Patch for landing Clearing flags on attachment: 174215 Committed
r134646
: <
http://trac.webkit.org/changeset/134646
>
WebKit Review Bot
Comment 12
2012-11-14 12:23:19 PST
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