Bug 58467

Summary: IndexedDB createIndex should fail if options arg is invalid
Product: WebKit Reporter: Mark Pilgrim (Google) <pilgrim>
Component: New BugsAssignee: Eugene Girard <girard>
Status: RESOLVED FIXED    
Severity: Normal CC: dgrogan, fishd, girard, hans, jsbell, pilgrim, tony, webkit.review.bot
Priority: P3    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 58471    
Bug Blocks:    
Attachments:
Description Flags
test case #1: options arg is not an object
none
test case #2: options arg contains unknown attributes
none
Patch
none
Patch none

Description Mark Pilgrim (Google) 2011-04-13 12:06:53 PDT
Two possible ways the options argument can be invalid: it's not a JavaScript object, or the object contains attributes other than keyPath and autoIncrement. In each case, Mozilla throws but WebKit does not. Attaching tests for each case.
Comment 1 Mark Pilgrim (Google) 2011-04-13 12:07:20 PDT
Created attachment 89426 [details]
test case #1: options arg is not an object
Comment 2 Mark Pilgrim (Google) 2011-04-13 12:07:45 PDT
Created attachment 89427 [details]
test case #2: options arg contains unknown attributes
Comment 3 Mark Pilgrim (Google) 2011-04-13 12:22:26 PDT
Apologies, my original comment was slightly incorrect. The valid attributes for the options argument are unique and multirow. The test cases still properly demonstrate the problem of not throwing on unknown attributes.
Comment 4 Joshua Bell 2011-10-14 13:46:18 PDT
Test case #2 is no longer valid - IndexedDB spec has changed to use the (new) WebIDL dictionary type for the options object, which ignores unknown attributes - http://dev.w3.org/2006/webapi/WebIDL/#es-dictionary

Test case #1 is still valid; in Chrome 15 I can pass a string as the options argument and no exception is thrown. Per WebIDL: "If Type(V) is not Object, then throw a TypeError."
Comment 5 Eugene Girard 2012-02-07 08:14:04 PST
Created attachment 125846 [details]
Patch
Comment 6 Joshua Bell 2012-02-07 08:26:58 PST
Comment on attachment 125846 [details]
Patch

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

Test changes LGTM

> LayoutTests/ChangeLog:6
> +        https://bugs.webkit.org/show_bug.cgi?id=58467

Nit: Keep the specific URL under the bug subject, have the comment lines in a separate paragraph. Otherwise the commit queue may get confused.
Comment 7 Tony Chang 2012-02-07 10:22:36 PST
Comment on attachment 125846 [details]
Patch

As Joshua mentions, the bug URL should be directly below the bug title.  See the other ChangeLog entries as an example.
Comment 8 Eugene Girard 2012-02-07 10:40:07 PST
Created attachment 125875 [details]
Patch
Comment 9 Eugene Girard 2012-02-07 10:41:38 PST
(In reply to comment #7)
> (From update of attachment 125846 [details])
> As Joshua mentions, the bug URL should be directly below the bug title.  See the other ChangeLog entries as an example.

Thanks.  Fixed in this patch.
Comment 10 WebKit Review Bot 2012-02-07 12:13:53 PST
Comment on attachment 125875 [details]
Patch

Rejecting attachment 125875 [details] from commit-queue.

New failing tests:
perf/array-reverse.html
Full output: http://queues.webkit.org/results/11459034
Comment 11 WebKit Review Bot 2012-02-07 14:18:44 PST
Comment on attachment 125875 [details]
Patch

Clearing flags on attachment: 125875

Committed r106991: <http://trac.webkit.org/changeset/106991>
Comment 12 WebKit Review Bot 2012-02-07 14:18:49 PST
All reviewed patches have been landed.  Closing bug.