Bug 58598 - IndexedDB add should fail adding an inline and passed key simultaneously
Summary: IndexedDB add should fail adding an inline and passed key simultaneously
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Joshua Bell
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-14 16:28 PDT by Mark Pilgrim (Google)
Modified: 2011-10-24 11:31 PDT (History)
7 users (show)

See Also:


Attachments
test case (1.71 KB, text/html)
2011-04-14 16:29 PDT, Mark Pilgrim (Google)
no flags Details
Patch (4.57 KB, patch)
2011-10-21 10:41 PDT, Joshua Bell
no flags Details | Formatted Diff | Diff
Patch (5.62 KB, patch)
2011-10-21 10:43 PDT, Joshua Bell
no flags Details | Formatted Diff | Diff
Patch (5.62 KB, patch)
2011-10-21 11:56 PDT, Joshua Bell
no flags Details | Formatted Diff | Diff
Patch (5.80 KB, patch)
2011-10-24 09:36 PDT, Joshua Bell
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Pilgrim (Google) 2011-04-14 16:28:50 PDT
Test from http://mxr.mozilla.org/mozilla2.0/source/dom/indexedDB/test/test_key_requirements.html?force=1#136 , taking an object store which was created with a keyPath and trying to add() a record with both the key in the first (value) object and in the second (key) argument should throw a DATA_ERR. This behavior is implemented by Mozilla and supported by spec text, namely http://dvcs.w3.org/hg/IndexedDB/raw-file/tip/Overview.html#widl-IDBObjectStore-add . WebKit does not throw any exception.
Comment 1 Mark Pilgrim (Google) 2011-04-14 16:29:40 PDT
Created attachment 89680 [details]
test case
Comment 2 Joshua Bell 2011-10-21 10:41:14 PDT
Created attachment 111982 [details]
Patch
Comment 3 Joshua Bell 2011-10-21 10:43:27 PDT
Created attachment 111984 [details]
Patch
Comment 4 Joshua Bell 2011-10-21 11:56:49 PDT
Created attachment 111999 [details]
Patch
Comment 5 Joshua Bell 2011-10-21 13:30:30 PDT
Sorry for the patch churn on this one. It should be ready for a look now.
Comment 6 Hans Wennborg 2011-10-24 02:43:26 PDT
Comment on attachment 111999 [details]
Patch

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

> Source/WebCore/storage/IDBObjectStoreBackendImpl.cpp:142
> +        || (!key && !autoIncrement && !hasKeyPath)) {

This is pretty hard to parse; can we split them into two if statements? One for the missing key case, and one for the "inline key and explicit key simultaneously" case?


LGTM besides that, thanks for fixing.
Comment 7 Joshua Bell 2011-10-24 09:36:55 PDT
Created attachment 112202 [details]
Patch
Comment 8 Joshua Bell 2011-10-24 09:38:21 PDT
Comment on attachment 112202 [details]
Patch

Split the "if" statement for readability per Hans.

Tony, can you give this a look?
Comment 9 WebKit Review Bot 2011-10-24 11:31:02 PDT
Comment on attachment 112202 [details]
Patch

Clearing flags on attachment: 112202

Committed r98258: <http://trac.webkit.org/changeset/98258>
Comment 10 WebKit Review Bot 2011-10-24 11:31:07 PDT
All reviewed patches have been landed.  Closing bug.