Bug 58613 – IndexedDB put() should fail if second (key) parameter is null

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

[Mark Pilgrim (Google)]

http://dvcs.w3.org/hg/IndexedDB/raw-file/tip/Overview.html#widl-IDBObjectStore-put states that the second (key) parameter to put() is optional, but if specified it must not be null. Mozilla correctly throws in this case (test: http://mxr.mozilla.org/mozilla2.0/source/dom/indexedDB/test/test_key_requirements.html?force=1#204 ) but WebKit does not throw.

[Mark Pilgrim (Google)]

Created attachment 89699 [details]
test case

[Mark Pilgrim (Google)]

Created attachment 91466 [details]
Patch

[Mark Pilgrim (Google)]

Straightforward fix -- if key is not specified, the prpKey ends up as a null pointer in ::put(). However, if the key is specified but is null, the prpKey ends up as a valid IDBKey which has a null key type. As it happens, we need to be able to detect the difference between these cases (the key arg is optional but if specified must not be null), and we can! Yay.

There was one existing layouttest that needed tweaking, plus the new layouttest ported from Mozilla's test suite. All other existing layouttests pass, including 5 that use put() without the second (key) arg. (I know it's exactly 5 because my first attempt crashed these 5 tests.)

[Hans Wennborg]

(In reply to comment #3)
> Straightforward fix -- if key is not specified, the prpKey ends up as a null pointer in ::put(). However, if the key is specified but is null, the prpKey ends up as a valid IDBKey which has a null key type. As it happens, we need to be able to detect the difference between these cases (the key arg is optional but if specified must not be null), and we can! Yay.
> 
> There was one existing layouttest that needed tweaking, plus the new layouttest ported from Mozilla's test suite. All other existing layouttests pass, including 5 that use put() without the second (key) arg. (I know it's exactly 5 because my first attempt crashed these 5 tests.)

Thanks, Mark!

Unofficial r+.

[Tony Chang]

Comment on attachment 91466 [details]
Patch

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

r- so you can upload a new version that I can cq+

> Source/WebCore/ChangeLog:7
> +

Can you put some of the first paragraph of comment #3 here?

> LayoutTests/ChangeLog:11
> +        * storage/indexeddb/objectstore-basics.html:

You could add a one line description of why this needed tweaking here.

[Mark Pilgrim (Google)]

Created attachment 91517 [details]
Patch with added changelog comments

[WebKit Commit Bot]

Comment on attachment 91517 [details]
Patch with added changelog comments

Clearing flags on attachment: 91517

Committed r85234: <http://trac.webkit.org/changeset/85234>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Mark Pilgrim (Google)]

*** Bug 58611 has been marked as a duplicate of this bug. ***