Bug 58614 - IndexedDB delete() should fail if key is null
Summary: IndexedDB delete() should fail if key is null
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P3 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-14 17:42 PDT by Mark Pilgrim (Google)
Modified: 2011-04-27 21:12 PDT (History)
5 users (show)

See Also:


Attachments
test case (1.66 KB, text/html)
2011-04-14 17:43 PDT, Mark Pilgrim (Google)
no flags Details
patch with changelog and layouttest (5.48 KB, patch)
2011-04-26 06:50 PDT, Mark Pilgrim (Google)
tony: review+
Details | Formatted Diff | Diff
nits addressed (5.54 KB, patch)
2011-04-26 17:04 PDT, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff
rebase webkit directory (5.20 KB, patch)
2011-04-26 19:35 PDT, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff
nits addressed once again (regression in previous patch) (5.26 KB, patch)
2011-04-27 08:28 PDT, Mark Pilgrim (Google)
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 17:42:52 PDT
http://dvcs.w3.org/hg/IndexedDB/raw-file/tip/Overview.html#widl-IDBObjectStore-delete states "If the key parameter is not a valid key this method throws a DATA_ERR exception." Then it goes on to say that key can not be null. Mozilla correctly throws in this case, but WebKit does not throw.
Comment 1 Mark Pilgrim (Google) 2011-04-14 17:43:41 PDT
Created attachment 89702 [details]
test case
Comment 2 Mark Pilgrim (Google) 2011-04-26 06:50:05 PDT
Created attachment 91097 [details]
patch with changelog and layouttest

patch to check if key is null in IDBObjectStoreBackendImpl.cpp::deleteFunction
Comment 3 David Grogan 2011-04-26 15:45:49 PDT
Comment on attachment 91097 [details]
patch with changelog and layouttest

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

Code LG, just nits.

> third_party/WebKit/Source/WebCore/ChangeLog:5
> +        IndexedDB object store delete should fail if key is null

make more obvious that you're referring to an object store deleting an entry, not an entire object store being deleted. e.g. myObjectStore.delete(null)

> third_party/WebKit/LayoutTests/storage/indexeddb/mozilla/key-requirements-delete-null-key.html:45
> +function cleanDatabase()

Change to something not misleading, e.g. cleanDatabaseAndCreateObjectStore or just inSetVersion
Comment 4 Tony Chang 2011-04-26 16:10:54 PDT
Comment on attachment 91097 [details]
patch with changelog and layouttest

Feel free to upload a patch addressing David's nits and I will cq+ it.
Comment 5 Mark Pilgrim (Google) 2011-04-26 17:04:33 PDT
Created attachment 91188 [details]
nits addressed
Comment 6 David Grogan 2011-04-26 17:15:16 PDT
Comment on attachment 91188 [details]
nits addressed

LGTM
Comment 7 Mark Pilgrim (Google) 2011-04-26 19:35:20 PDT
Created attachment 91212 [details]
rebase webkit directory
Comment 8 Mark Pilgrim (Google) 2011-04-27 08:28:32 PDT
Created attachment 91284 [details]
nits addressed once again (regression in previous patch)
Comment 9 WebKit Commit Bot 2011-04-27 21:12:08 PDT
Comment on attachment 91284 [details]
nits addressed once again (regression in previous patch)

Clearing flags on attachment: 91284

Committed r85145: <http://trac.webkit.org/changeset/85145>
Comment 10 WebKit Commit Bot 2011-04-27 21:12:13 PDT
All reviewed patches have been landed.  Closing bug.