Bug 50632 - IndexedDB returns the wrong exceptions
Summary: IndexedDB returns the wrong exceptions
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: Jeremy Orlow
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-12-07 08:58 PST by Jeremy Orlow
Modified: 2010-12-09 04:30 PST (History)
4 users (show)

See Also:


Attachments
Patch (17.34 KB, patch)
2010-12-07 09:04 PST, Jeremy Orlow
no flags Details | Formatted Diff | Diff
Patch (17.32 KB, patch)
2010-12-08 06:50 PST, Jeremy Orlow
no flags Details | Formatted Diff | Diff
Patch (17.31 KB, patch)
2010-12-08 10:02 PST, Jeremy Orlow
steveblock: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Orlow 2010-12-07 08:58:22 PST
IndexedDB returns the wrong exceptions
Comment 1 Jeremy Orlow 2010-12-07 09:04:25 PST
Created attachment 75820 [details]
Patch
Comment 2 WebKit Review Bot 2010-12-07 09:12:41 PST
Attachment 75820 [details] did not pass style-queue:

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


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Hans Wennborg 2010-12-07 09:19:07 PST
Comment on attachment 75820 [details]
Patch

Looks fine as far as I can tell. A few comments about the messages:


> WebCore/dom/ExceptionCode.cpp:231
> +    "A record/index/objectStore by the name you requested does not exist.",

I think we should avoid personal pronouns in these messages. Perhaps "... the requested name does not exist."?

> WebCore/dom/ExceptionCode.cpp:232
> +    "Your request cannot be completed due to a failed constraint.",

Ditto.

> WebCore/dom/ExceptionCode.cpp:233
> +    "The data you provided does not meet the requirements of the function.",

Ditto.
Comment 4 WebKit Review Bot 2010-12-07 10:13:29 PST
Attachment 75820 [details] did not pass style-queue:

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


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 WebKit Review Bot 2010-12-07 11:14:39 PST
Attachment 75820 [details] did not pass style-queue:

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


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 WebKit Review Bot 2010-12-07 12:15:59 PST
Attachment 75820 [details] did not pass style-queue:

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


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 WebKit Review Bot 2010-12-07 21:40:16 PST
Attachment 75820 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/update-webkit']" exit_code: 2
Updating OpenSource
Incomplete data: Delta source ended unexpectedly at /usr/lib/git-core/git-svn line 5061

Died at WebKitTools/Scripts/update-webkit line 132.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Jeremy Orlow 2010-12-08 06:50:08 PST
Another shot at the names.  Steve, can you please review?
Comment 9 Jeremy Orlow 2010-12-08 06:50:28 PST
Created attachment 75898 [details]
Patch
Comment 10 Andrei Popescu 2010-12-08 07:21:03 PST
LGTM
Comment 11 Hans Wennborg 2010-12-08 07:26:10 PST
Comment on attachment 75898 [details]
Patch

LGTM

> WebCore/dom/ExceptionCode.cpp:234
> +    "This function is not allowed to be called in such a context",

ultra nit: period.

> WebCore/dom/ExceptionCode.cpp:235
> +    "The data supplied cannot be serialized according to the structured cloning algorithm",

ditto
Comment 12 Steve Block 2010-12-08 07:39:09 PST
Comment on attachment 75898 [details]
Patch

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

Are the new values set in the spec? Do you have a link?

> WebCore/bindings/js/JSDOMBinding.cpp:90
> +#include "IDBDatabaseExceptionException.h"

ExceptionException?

> WebCore/bindings/v8/V8Proxy.cpp:40
> +#include "IDBDatabaseException.h"

Should this be guarded? If not, why is it for JSC?

> WebCore/storage/IDBDatabaseException.idl:35
> +        readonly attribute DOMString        message;

Why the odd spacing?
Comment 13 Jeremy Orlow 2010-12-08 08:40:12 PST
(In reply to comment #12)
> (From update of attachment 75898 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=75898&action=review
> 
> Are the new values set in the spec? Do you have a link?

The new numerical values are not yet in the spec, but I intend to change the spec in the next day or two.  There's been a bug to do so for a while.
 
> > WebCore/bindings/js/JSDOMBinding.cpp:90
> > +#include "IDBDatabaseExceptionException.h"
> 
> ExceptionException?

Bah.  Good catch!
 
> > WebCore/bindings/v8/V8Proxy.cpp:40
> > +#include "IDBDatabaseException.h"
> 
> Should this be guarded? If not, why is it for JSC?

Just following conventions in the various files.

> > WebCore/storage/IDBDatabaseException.idl:35
> > +        readonly attribute DOMString        message;
> 
> Why the odd spacing?

Just copying from the other Exception files.
Comment 14 Jeremy Orlow 2010-12-08 10:02:54 PST
Created attachment 75918 [details]
Patch
Comment 15 Steve Block 2010-12-08 10:10:02 PST
Comment on attachment 75918 [details]
Patch

r=me
Comment 16 Jeremy Orlow 2010-12-09 04:30:17 PST
Committed r73605: <http://trac.webkit.org/changeset/73605>