1268380 - Exception Precedence of ​ SyntaxError in IDBObjectStore.createIndex() is Inconsistent to the one in IDB Spec v2.

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect)

Description

[Bevis Tseng[:bevis][:btseng](Exited)]

The priority of SyntaxError in IDBObjectStore.createIndex() is lower than ConstraintError as specified in IDB Spec v2 [1].
However, in gecko, SyntaxError is the first one to be validated in IDBObjectStore.createIndex()[2][3].

[1] http://w3c.github.io/IndexedDB/#dom-idbobjectstore-createindex
[2] https://hg.mozilla.org/mozilla-central/annotate/fc15477ce628599519cb0055f52cc195d640dc94/dom/indexedDB/IDBObjectStore.cpp#l1827
[3] https://hg.mozilla.org/mozilla-central/annotate/fc15477ce628599519cb0055f52cc195d640dc94/dom/indexedDB/IDBObjectStore.cpp#l1846

Comment 1

[Bevis Tseng[:bevis][:btseng](Exited)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1cc33e803eb3

Comment 2

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v1) Patch: Lower the precedence of SyntaxError next to ConstraintError in IDBObjectStore::CreateIndex().

Unify 2 IDBObjectStore::createIndex() methods with |(DOMString or sequence<DOMString>) keyPath| to check the syntax of keyPath after validating index name.

Comment 3

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #2)
> Created attachment 8746953 [details] [diff] [review]
> (v1) Patch: Lower the precedence of SyntaxError next to ConstraintError in
> IDBObjectStore::CreateIndex().
> 
> Unify 2 IDBObjectStore::createIndex() methods with |(DOMString or
> sequence<DOMString>) keyPath| to check the syntax of keyPath after
> validating index name.

DataStore was removed in Bug 1261009, so we could discard the change in DataStoreDB.cpp.

Comment 4

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

Comment on attachment 8746953 [details] [diff] [review]
(v1) Patch: Lower the precedence of SyntaxError next to ConstraintError in IDBObjectStore::CreateIndex().

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

Can you add a web-platform-test?  (Or maybe ask jsbell if Google has one already?)

Comment 5

[Bevis Tseng[:bevis][:btseng](Exited)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a3e5f29969a1

Comment 6

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v2) Part 1: Lower the precedence of SyntaxError next to ConstraintError in IDBObjectStore::CreateIndex().

Discard the change of DataStoreDB.cpp done in v1.

Comment 7

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v2) Part 2: Add Test Coverage in Web Platform Tests.

Add Web Platform Test for the Exception Precedence in IDBObjectStore::createIndex().

Comment 8

[Bevis Tseng[:bevis][:btseng](Exited)]

try server result looks fine:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1e82e24855ad

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d231a8a96580
https://hg.mozilla.org/integration/mozilla-inbound/rev/d13e15bf4369

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d231a8a96580
https://hg.mozilla.org/mozilla-central/rev/d13e15bf4369