Closed
Bug 1105615
Opened 10 years ago
Closed 10 years ago
Make IDBObjectStoreParameters.keyPath a union
Categories
(Core :: Storage: IndexedDB, defect)
Tracking
()
RESOLVED
FIXED
mozilla37
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
Attachments
(1 file, 2 obsolete files)
8.81 KB,
patch
|
baku
:
review+
|
Details | Diff | Splinter Review |
There's a TODO in the IDL there, and I see no reason not to fix it...
![]() |
Assignee | |
Comment 1•10 years ago
|
||
The one thing to watch out for is zero-length arrays. As far as I can tell those used to throw from Parse() but now won't. IS that a problem?
Attachment #8529517 -
Flags: review?(amarchesini)
![]() |
Assignee | |
Updated•10 years ago
|
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
![]() |
Assignee | |
Comment 2•10 years ago
|
||
Attachment #8529573 -
Flags: review?(amarchesini)
![]() |
Assignee | |
Updated•10 years ago
|
Attachment #8529517 -
Attachment is obsolete: true
Attachment #8529517 -
Flags: review?(amarchesini)
![]() |
Assignee | |
Comment 3•10 years ago
|
||
> IS that a problem?
Looks like yes. We have tests for it, and the spec says empty array is not a valid keypath. I wonder why the Parse() overload that takes an array doesn't enforce that... So fixed in that updated patch.
![]() |
Assignee | |
Comment 4•10 years ago
|
||
Attachment #8529574 -
Flags: review?(amarchesini)
![]() |
Assignee | |
Updated•10 years ago
|
Attachment #8529573 -
Attachment is obsolete: true
Attachment #8529573 -
Flags: review?(amarchesini)
Comment 5•10 years ago
|
||
Comment on attachment 8529574 [details] [diff] [review]
Make IDBObjectStoreParameters.keyPath a union instead of using "any" for it
Review of attachment 8529574 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/indexedDB/KeyPath.cpp
@@ +266,5 @@
>
> + if (aValue.Value().IsString()) {
> + return Parse(aValue.Value().GetAsString(), aKeyPath);
> + }
> +
MOZ_ASSERT(aValue.Value().IsStringSequence()) ?
@@ +269,5 @@
> + }
> +
> + const Sequence<nsString>& seq = aValue.Value().GetAsStringSequence();
> + if (seq.Length() == 0) {
> + return NS_ERROR_DOM_SYNTAX_ERR;
Previously we returned NS_ERROR_FAILURE.
We return DOM_SYNTAX_ERR in CreateObject.
Attachment #8529574 -
Flags: review?(amarchesini) → review+
![]() |
Assignee | |
Comment 6•10 years ago
|
||
> MOZ_ASSERT(aValue.Value().IsStringSequence()) ?
Done.
> Previously we returned NS_ERROR_FAILURE.
> We return DOM_SYNTAX_ERR in CreateObject.
Ah, I see. Ok, switched back to NS_ERROR_FAILURE, since upstream fixes it up.
https://hg.mozilla.org/integration/mozilla-inbound/rev/a84e8a699159
Flags: in-testsuite?
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in
before you can comment on or make changes to this bug.
Description
•