Bug 58598

Summary:

IndexedDB add should fail adding an inline and passed key simultaneously

Product:

WebKit

Reporter:

Mark Pilgrim (Google) <pilgrim>

Component:

New Bugs

Assignee:

Joshua Bell <jsbell>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

dgrogan, fishd, hans, jsbell, pilgrim, tony, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
test case	none
Patch	none
Patch	none
Patch	none
Patch	none

Mark Pilgrim (Google)

Reported 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.

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
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Mark Pilgrim (Google)

Comment 1 2011-04-14 16:29:40 PDT

Created attachment 89680 [details] test case

Joshua Bell

Comment 2 2011-10-21 10:41:14 PDT

Created attachment 111982 [details] Patch

Joshua Bell

Comment 3 2011-10-21 10:43:27 PDT

Created attachment 111984 [details] Patch

Joshua Bell

Comment 4 2011-10-21 11:56:49 PDT

Created attachment 111999 [details] Patch

Joshua Bell

Comment 5 2011-10-21 13:30:30 PDT

Sorry for the patch churn on this one. It should be ready for a look now.

Hans Wennborg

Comment 6 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.

Joshua Bell

Comment 7 2011-10-24 09:36:55 PDT

Created attachment 112202 [details] Patch

Joshua Bell

Comment 8 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?

WebKit Review Bot

Comment 9 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>

WebKit Review Bot

Comment 10 2011-10-24 11:31:07 PDT

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.