Closed
Bug 902565
Opened 11 years ago
Closed 10 years ago
this.done in cursor callback of naviagtor.getDeviceStorage("sdcard").enumerate is undefined
Categories
(Core :: DOM: Device Interfaces, defect)
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: nick, Unassigned)
References
Details
(Keywords: dev-doc-needed)
example: var sdcard = navigator.getDeviceStorage("sdcard"); var cursor = sdcard.enumerate(); cursor.onsuccess = function () { alert("this.done is: " + this.done); // this.done is: undefined }; I modified the MDN page, so that should be reverted when fixed. [1] [1] https://developer.mozilla.org/en-US/docs/Web/API/DeviceStorage.enumerate$history
Reporter | ||
Comment 1•11 years ago
|
||
Also note that I only tested this in 1.0.1.
Comment 2•11 years ago
|
||
Even Gaia code doesn't use .done but checks if .result is set: https://github.com/mozilla-b2g/gaia/blob/v1-train/apps/communications/contacts/js/utilities/sdcard.js?source=c#L62 Also no sign of using .done in the device storage tests in Gaia: https://github.com/mozilla-b2g/gaia/blob/v1-train/test_apps/ds-test/js/ds-test.js
Comment 3•11 years ago
|
||
Hi, I recently experienced the same problom when following code example found here: https://developer.mozilla.org/en-US/docs/WebAPI/Device_Storage?redirectlocale=en-US&redirectslug=WebAPI%2FDevice_Storage_API the solution to that example is to check whether file is not null and put "this.continue()" inside the null check. Please update the example as long as the problem exists.
Comment 4•11 years ago
|
||
Needinfo'ing Mounir and Jonas to ask whether they'd be aware of a regression. Maybe it's just a documentation issue?
Flags: needinfo?(mounir)
Flags: needinfo?(jonas)
Reporter | ||
Comment 5•11 years ago
|
||
David, I don't think it's a documentation issue if it's not used that way in Gaia (as per Harald's research in Comment #2).
Comment 6•11 years ago
|
||
Hmm. I just tried this on master, and it seems to be working for me. I modified ds-test.js around line 412 (in the onsuccess inside enumerateAll) so that it looked like: function onsuccess(e) { cursor.result = e.target.result; dump('onsuccess: this.done = ' + this.done + '\n'); dump('onsuccess: ds_cursor.done = ' + ds_cursor.done + '\n'); and when calling enumerateAll I see the following in my logcat: onsuccess: this.done = false onsuccess: ds_cursor.done = false for the cases when cursor.result is non-null and onsuccess: this.done = true onsuccess: ds_cursor.done = true when cursor.result is null. I wasn't using .done in ds-test.js because I wasn't aware of it. I'll file a bug to update ds-test.js to use .done
Reporter | ||
Comment 7•11 years ago
|
||
Dave, can you verify this in 1.0.1 and 1.1? It's an issue for developers if the API differs between versions and should be documented.
Comment 8•11 years ago
|
||
I filed bug 905294 to rework ds-test to use .done.
Comment 9•11 years ago
|
||
(In reply to Nick Desaulniers [:\n] from comment #7) > Dave, can you verify this in 1.0.1 and 1.1? It's an issue for developers if > the API differs between versions and should be documented. Sure will do.
Comment 10•11 years ago
|
||
FYI, I was using Geekphone Device with Boot2Gecko 1.0.1.0-prerelease and the simulator running Boot2Gecko 1.1.0.0-prerelease. Same issue on both.
Comment 11•11 years ago
|
||
(In reply to Nick Desaulniers [:\n] from comment #7) > Dave, can you verify this in 1.0.1 and 1.1? It's an issue for developers if > the API differs between versions and should be documented. It appears that bug 837865 modified DeviceStorage to use DOMCursor, and that particular bug only landed on master. I verified that this.done was undefined on 1.1 (I didn't bother confirming 1.0.1 since it should be the same).
Updated•11 years ago
|
Flags: needinfo?(mounir)
So it sounds like this is working as "expected" and that .done isn't implemented for v1.0.1/v1.1 and will only be available in v1.2. Certainly not great, but that's how it is with old code. Is this blocking any particular apps?
Flags: needinfo?(jonas)
Reporter | ||
Comment 13•11 years ago
|
||
No, I had written up a file browser app based on feedback and just had to work around it. The documentation should probably be reverted back with a note.
Reporter | ||
Comment 14•10 years ago
|
||
instead, one may put the call to this.continue() within a conditional check for this.result: https://github.com/nickdesaulniers/filebrowser/blob/eb49f30954c0d69c355bfab3bdbde453d5451894/myscript.js#L22-L45
Comment 15•10 years ago
|
||
In our code reviews we'd say that you should do this instead: if (!this.result) { return; } and then unindent the rest of the function by one level You shouldn't call this.continue once this.result returns null (since you're finished, it doesn't make sense to call continue). Alternatively you could do: if (!this.result) { this.done(); return; }
Reporter | ||
Comment 16•10 years ago
|
||
You also shouldn't invoke this.done if it's undefined. :P
Updated•10 years ago
|
Keywords: dev-doc-needed
Comment 17•10 years ago
|
||
Marking this as resolved works for me, since the behaviour observed appears to be intended. I also added dev-doc-needed so that we can document .done as a method to detect the end of the enumeration for FxOS 1.1 and newer.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•