{DataStoreTask|DataStoreChangeEvent}.id and DataStoreTask.data has to be nullable

RESOLVED FIXED in mozilla32

Status

()

defect
RESOLVED FIXED
5 years ago
3 months ago

People

(Reporter: airpingu, Assigned: baku)

Tracking

Trunk
mozilla32
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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

5 years ago
any? data seems wrong. You should remove it from the spec. any can be 'null'.
Attachment #8411777 - Flags: review?(gene.lian)
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+
Btw, the W3C spec has been fixed (don't need to add "?" for any).
Assignee

Comment 4

5 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));
https://hg.mozilla.org/mozilla-central/rev/f3a57f0d89b0
Assignee: nobody → amarchesini
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.