769880 - Make createIndex throw InvalidAccessError instead of NotSupportedError

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect)

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Patch to fix

Based on discussion on mailing list. Though there is some risk that this will be reverted it seems more likely to stick.

Patch also includes some additional tests and some test cleanup.

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 638076 [details] [diff] [review]
Patch to fix

Review of attachment 638076 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/indexedDB/test/unit/test_create_index.js
@@ +51,5 @@
>      }
> +    ok(ex, "createIndex with array keyPath and multiEntry should throw");
> +    is(ex.name, "InvalidAccessError", "should throw right exception");
> +    ok(ex instanceof DOMException, "should throw right exception");
> +    is(ex.code, DOMException.INVALID_ACCESS_ERR, "should throw right exception");

Put the tests in the catch, and put a ok(false) at the end of the try block.  That way if this fails the test won't throw an exception and terminate at ex.name.

::: dom/indexedDB/test/unit/test_create_objectStore.js
@@ +109,5 @@
> +  }
> +  ok(ex, "createObjectStore with empty keyPath and autoIncrement should throw");
> +  is(ex.name, "InvalidAccessError", "should throw right exception");
> +  ok(ex instanceof DOMException, "should throw right exception");
> +  is(ex.code, DOMException.INVALID_ACCESS_ERR, "should throw right exception");

Here too.

@@ +122,5 @@
> +  }
> +  ok(ex, "createObjectStore with array keyPath and autoIncrement should throw");
> +  is(ex.name, "InvalidAccessError", "should throw right exception");
> +  ok(ex instanceof DOMException, "should throw right exception");
> +  is(ex.code, DOMException.INVALID_ACCESS_ERR, "should throw right exception");

Here too.

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

But it makes the code flow less obvious. And I'd have to insert both ok(true) in the catch branch and ok(false) in the try branch.

I'd prefer to keep it as-is.

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

It would be consistent with the other tests too.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Kyle, does your r+ stand with the patch as-is? I'd rather not change the tests as I prefer the current pattern and would prefer to see more rather than fewer tests written this way.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 638076 [details] [diff] [review]
Patch to fix

Resetting review request to get clarification.

Comment 6

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 638076 [details] [diff] [review]
Patch to fix

Review of attachment 638076 [details] [diff] [review]:
-----------------------------------------------------------------

I told you on IRC that I'd go along with it, despite my distaste for that pattern.

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6a870e68438b

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6a870e68438b