Bug 102114

Summary: Rename NATIVE_TYPE_ERR to TypeError
Product: WebKit Reporter: Joshua Bell <jsbell>
Component: New BugsAssignee: Joshua Bell <jsbell>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, arv, darin, eric.carlson, feature-media-reviews, haraken, japhet, ojan, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 91679    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

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
DetailsFormatted DiffDiff
Patch (33.79 KB, patch)
2012-11-13 13:37 PST, Joshua Bell 		no flags
DetailsFormatted DiffDiff
Patch for landing (33.66 KB, patch)
2012-11-14 11:47 PST, Joshua Bell 		no flags
DetailsFormatted DiffDiff
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.