Closed
Bug 1000837
Opened 10 years ago
Closed 10 years ago
{DataStoreTask|DataStoreChangeEvent}.id and DataStoreTask.data has to be nullable
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: airpingu, Assigned: baku)
References
Details
Attachments
(1 file)
10.96 KB,
patch
|
airpingu
:
review+
|
Details | Diff | Splinter Review |
This happens when DataStoreOperation is "clear" or "done". Hope to align this to the W3C version at: http://airpingu.github.io/data-store-api/index.html
Assignee | ||
Comment 1•10 years ago
|
||
any? data seems wrong. You should remove it from the spec. any can be 'null'.
Attachment #8411777 -
Flags: review?(gene.lian)
Reporter | ||
Comment 2•10 years ago
|
||
Comment on attachment 8411777 [details] [diff] [review] nullable.patch Review of attachment 8411777 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/datastore/DataStoreImpl.jsm @@ +274,5 @@ > self._db.clearRevisions(aRevisionStore, > function() { > debug("Revisions cleared"); > > + self.addRevision(aRevisionStore, null, REVISION_VOID, This fixes the "clear" path. Where is for "done"? ::: dom/webidl/DataStore.webidl @@ +102,5 @@ > dictionary DataStoreTask { > DOMString revisionId; > > DataStoreOperation operation; > + DataStoreKey? id; Would you please add a simple comment here to explain when it could be nullable? // When |operation| is "clear" or "done", this must return null. ::: dom/webidl/DataStoreChangeEvent.webidl @@ +5,5 @@ > */ > > dictionary DataStoreChangeEventInit : EventInit { > DOMString revisionId = ""; > + DataStoreKey? id = 0; Since we want to use null to represent invalid value, can this be just initialized as null? I think null is better than 0, if we never defined 0 as default void value in our spec. Please correct me if I'm wrong. @@ +14,5 @@ > [Func="Navigator::HasDataStoreSupport", > Constructor(DOMString type, optional DataStoreChangeEventInit eventInitDict)] > interface DataStoreChangeEvent : Event { > readonly attribute DOMString revisionId; > + readonly attribute DataStoreKey? id; Ditto. Add a comment for this.
Attachment #8411777 -
Flags: review?(gene.lian) → review+
Reporter | ||
Comment 3•10 years ago
|
||
Btw, the W3C spec has been fixed (don't need to add "?" for any).
Assignee | ||
Comment 4•10 years ago
|
||
> > + self.addRevision(aRevisionStore, null, REVISION_VOID,
in DataStoreCursorImpl.cpp:
- aResolve(Cu.cloneInto({ revisionId: this._revision.revisionId,
- operation: 'done' }, this._window));
+ aResolve(this.createTask('done', null, this._revision.revisionId, null));
Assignee | ||
Comment 5•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f3a57f0d89b0
https://hg.mozilla.org/mozilla-central/rev/f3a57f0d89b0
Assignee: nobody → amarchesini
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•