Bug 87055 - Implement DOM4 DOMError
Summary: Implement DOM4 DOMError
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alec Flett
URL:
Keywords: WebExposed
Depends on:
Blocks:
 
Reported: 2012-05-21 15:49 PDT by Alec Flett
Modified: 2012-06-08 14:58 PDT (History)
7 users (show)

See Also:


Attachments
Patch (8.22 KB, patch)
2012-05-21 15:53 PDT, Alec Flett
no flags Details | Formatted Diff | Diff
Patch (12.50 KB, patch)
2012-05-21 16:28 PDT, Alec Flett
no flags Details | Formatted Diff | Diff
Patch (19.95 KB, patch)
2012-05-22 13:06 PDT, Alec Flett
no flags Details | Formatted Diff | Diff
Patch for landing (19.98 KB, patch)
2012-05-23 11:15 PDT, Alec Flett
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alec Flett 2012-05-21 15:49:27 PDT
Implement DOM4 DOMError
Comment 1 Alec Flett 2012-05-21 15:53:20 PDT
Created attachment 143120 [details]
Patch
Comment 2 Alec Flett 2012-05-21 15:55:38 PDT
This is a bit of a strawman - I need this class for some IndexedDB work, but this isn't implemented anywhere. There also isn't any DOM-spec'ed way to create one of these. (aside from exposure in the IndexedDB API)
Comment 3 Alec Flett 2012-05-21 15:57:11 PDT
Here's the spec, FWIW: http://www.w3.org/TR/dom/#interface-domerror
Comment 4 Adam Barth 2012-05-21 16:02:08 PDT
Comment on attachment 143120 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=143120&action=review

> Source/WebCore/dom/DOMError.idl:33
> +    readonly attribute DOMString name;

This line should be intended four more spaces.
Comment 5 Adam Barth 2012-05-21 16:03:59 PDT
Comment on attachment 143120 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=143120&action=review

> Source/WebCore/ChangeLog:11
> +        No new tests. I wasn't sure how to test this because it isn't 
> +        used yet. I'm open to suggestions.

This patch isn't testable because you're not using this object anywhere yet and you haven't exposed the constructor on DOMWindow.  (I'm not sure if you intended to expose the constructor on DOMWindow or not.)

> Source/WebCore/dom/DOMError.idl:31
> +        JSGenerateToNativeObject

Is this attribute needed?  https://trac.webkit.org/wiki/WebKitIDL#JSGenerateToJSObject
Comment 6 Adam Barth 2012-05-21 16:06:36 PDT
Comment on attachment 143120 [details]
Patch

This patch looks fine.  The spec doesn't say to expose this via DOMWindow, so I don't think there's anyway to test this patch.  When you use the object in IDB, we should add some tests to exercise the code.
Comment 7 Adam Barth 2012-05-21 16:07:33 PDT
Comment on attachment 143120 [details]
Patch

Note: You'll need to add the new files to a bunch more build systems.  You can grep WebCore for DOMStringList.cpp and DOMStringList.idl  to get a sense for where all you'll need to list these files.
Comment 8 Alec Flett 2012-05-21 16:28:00 PDT
Created attachment 143129 [details]
Patch
Comment 9 Alec Flett 2012-05-21 16:29:50 PDT
great thanks - I misunderstood that IDL flag. I took a pass through the other build systems. I'll see if this is enough through the bots.
Comment 10 Adam Barth 2012-05-21 16:35:39 PDT
Comment on attachment 143129 [details]
Patch

Looks like you're missing vcproj and xcodebuild files.
Comment 11 Alec Flett 2012-05-22 13:06:53 PDT
Created attachment 143352 [details]
Patch
Comment 12 Alec Flett 2012-05-22 14:56:06 PDT
sorry abarth@ I keep forgetting to land-safely rather than upload.. one more review here (got vcproj and xcode taken care of)
Comment 13 Adam Barth 2012-05-22 15:01:34 PDT
Comment on attachment 143352 [details]
Patch

Looks like your patch didn't apply.  You might need to update it to top-of-tree.
Comment 14 Alec Flett 2012-05-23 11:15:34 PDT
Created attachment 143600 [details]
Patch for landing
Comment 15 WebKit Review Bot 2012-05-23 11:15:49 PDT
Comment on attachment 143600 [details]
Patch for landing

Rejecting attachment 143600 [details] from commit-queue.

alecflett@chromium.org does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.
Comment 16 Alec Flett 2012-05-23 11:30:41 PDT
jsbell@ - cq? I tried to land-safely but of course I couldn't cuz I'm not yet a committer
Comment 17 WebKit Review Bot 2012-05-23 12:38:11 PDT
Comment on attachment 143600 [details]
Patch for landing

Clearing flags on attachment: 143600

Committed r118226: <http://trac.webkit.org/changeset/118226>
Comment 18 WebKit Review Bot 2012-05-23 12:38:16 PDT
All reviewed patches have been landed.  Closing bug.