Bug 58465 – IndexedDB createObjectStore should throw if name is null

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Mark Pilgrim (Google)]

http://dvcs.w3.org/hg/IndexedDB/raw-file/tip/Overview.html#widl-IDBDatabase-createObjectStore states that the name property can not be null. Mozilla throws, but WebKit does not. Instead, we create an object store whose name is the 4-character string "null".

[Mark Pilgrim (Google)]

Created attachment 89419 [details]
test case

[Mark Pilgrim (Google)]

Similar to, but not a duplicate of, bug 58365.

[Mark Pilgrim (Google)]

Created attachment 91410 [details]
Patch

[Mark Pilgrim (Google)]

Similar to bug 58365, the root problem is in the IDL file which needs to specify that a null DOMString is treated as null. Then the C++ isNull() check works as expected.

[David Grogan]

Comment on attachment 91410 [details]
Patch

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

r+

> LayoutTests/storage/indexeddb/mozilla/create-objectstore-null-name.html:51
> +    evalAndExpectException("db.createObjectStore(null);", "IDBDatabaseException.CONSTRAINT_ERR");

It looks like the specific exception isn't dictated.  I'm assuming firefox throws CONSTRAINT_ERR?

[Mark Pilgrim (Google)]

(In reply to comment #5)
> > LayoutTests/storage/indexeddb/mozilla/create-objectstore-null-name.html:51
> > +    evalAndExpectException("db.createObjectStore(null);", "IDBDatabaseException.CONSTRAINT_ERR");
> 
> It looks like the specific exception isn't dictated.  I'm assuming firefox throws CONSTRAINT_ERR?

I thought so, but upon double-checking, it throws NON_TRANSIENT_ERR instead. Updating patch...

[Mark Pilgrim (Google)]

Created attachment 91414 [details]
Throw NON_TRANSIENT_ERR instead

This patch better matches Firefox's behavior.

[David Grogan]

Comment on attachment 91414 [details]
Throw NON_TRANSIENT_ERR instead

r+

[WebKit Commit Bot]

Comment on attachment 91414 [details]
Throw NON_TRANSIENT_ERR instead

Rejecting attachment 91414 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sf', 'ap..." exit_code: 2

Last 500 characters of output:
-objectstore-null-name.html
patch unexpectedly ends in middle of line
patch: **** malformed patch at line 67:  

fatal: pathspec 'LayoutTests/storage/indexeddb/mozilla/create-objectstore-null-name.html' did not match any files
Failed to git add LayoutTests/storage/indexeddb/mozilla/create-objectstore-null-name.html. at /Projects/CommitQueue/Tools/Scripts/svn-apply line 439.

Failed to run "[u'/Projects/CommitQueue/Tools/Scripts/svn-apply', u'--reviewer', u'Tony Chang', u'--force']" exit_code: 2

Full output: http://queues.webkit.org/results/8710633

[Mark Pilgrim (Google)]

Any idea why this failed to commit? The queue tells me nothing.

[Tony Chang]

Created attachment 94240 [details]
Patch for landing

[Tony Chang]

(In reply to comment #10)
> Any idea why this failed to commit? The queue tells me nothing.

The diff was missing a newline at the end of the file.

[WebKit Commit Bot]

Comment on attachment 94240 [details]
Patch for landing

Clearing flags on attachment: 94240

Committed r86970: <http://trac.webkit.org/changeset/86970>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Daniel Bates]

(In reply to comment #12)
> (In reply to comment #10)
> > Any idea why this failed to commit? The queue tells me nothing.
> 
> The diff was missing a newline at the end of the file.

Did you edit this patch by hand? I added the missing new line, applied the patch using svn-apply then used svn-create-patch to recreate the patch P. And P has a new line at the end of the file. Similarly, I was able to create a valid patch by following the aforementioned steps and calling svn diff instead of  svn-create-patch.

[Mark Pilgrim (Google)]

(In reply to comment #15)
> (In reply to comment #12)
> > (In reply to comment #10)
> > > Any idea why this failed to commit? The queue tells me nothing.
> > 
> > The diff was missing a newline at the end of the file.
> 
> Did you edit this patch by hand? I added the missing new line

Aha, that's it. I've fixed my editor so it doesn't strip final newlines.

Thanks.

[Mark Pilgrim (Google)]

Patch that landed has repeated test results, causing test to fail.

[Tony Chang]

(In reply to comment #17)
> Patch that landed has repeated test results, causing test to fail.

Sorry, probably patched twice into a new file.  Will fix.

[Tony Chang]

http://trac.webkit.org/changeset/87393