Closed Bug 1409641 Opened 2 years ago Closed Last year

Fix [JavaScript Warning: "ReferenceError: reference to undefined property "result"" {file: "resource://gre/modules/IndexedDBHelper.jsm" line: 170}] - replace |txn.result| with |event.target.result|

Categories

(Core :: Storage: IndexedDB, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: jorgk-bmo, Assigned: tt)

References

Details

Attachments

(1 file, 1 obsolete file)

From Bug 1399746 comment #68:

(In reply to Jorg K (GMT+2) from Bug 1399746 comment #30)
> Marco and Andrew, should that JS error be fixed?
> [JavaScript Warning: "ReferenceError: reference to undefined property
> "result"" {file: "resource://gre/modules/IndexedDBHelper.jsm" line: 170}]

IDBTransaction doesn't have a result property, I guess |txn.result| should be replaced with |event.target.result|
Component: Storage → DOM: IndexedDB
Product: Toolkit → Core
This doesn't sound urgent to me, Shawn may be able to fix this at a commensurate time frame. Feel free to correct me if I am wrong.
Flags: needinfo?(shuang)
Priority: -- → P3
Assignee: nobody → shuang
Flags: needinfo?(shuang)
Janv: can you give this another brief look? I don't believe this is urgent either (p3) but I would like it double checked as Shawn is no longer on it.
Flags: needinfo?(jvarga)
(In reply to Jorg K (GMT+2) from comment #0)
> From Bug 1399746 comment #68:
> 
> (In reply to Jorg K (GMT+2) from Bug 1399746 comment #30)
> > Marco and Andrew, should that JS error be fixed?
> > [JavaScript Warning: "ReferenceError: reference to undefined property
> > "result"" {file: "resource://gre/modules/IndexedDBHelper.jsm" line: 170}]
> 
> IDBTransaction doesn't have a result property, I guess |txn.result| should
> be replaced with |event.target.result|

Right, see:
https://dxr.mozilla.org/mozilla-central/rev/4e56a2f51ad739ca52046723448f3129a58f1666/dom/base/IndexedDBHelper.jsm

Tom, feel free to send a patch for this, I'll review it.
Flags: needinfo?(jvarga) → needinfo?(shes050117)
Will do but keep the ni as a notification as usual. :)
Assignee: nobody → shes050117
Per discussing with Jan, there seems that arguments for function oncomplete() [1] and onabort()[2] aren't defined in the current spec. This file dom/base/IndexedDBHelper.jsm, however, expects to get results from a transaction oncomplete event.

Therefore, this patch modifies IndexedDBHelper to make the callback inside get the result property only when it's defined. Otherwise, it gets a callback with nothing. Furthermore, this patch also changes the value for the failureCb since it should get an error from the transaction instead of the event.target.

[1] https://w3c.github.io/IndexedDB/#dom-idbtransaction-oncomplete
[2] https://w3c.github.io/IndexedDB/#dom-idbtransaction-onabort
Flags: needinfo?(shes050117)
Attachment #9001641 - Flags: review?(jvarga)
Comment on attachment 9001641 [details] [diff] [review]
Bug 1409641: Get the result only when it's defined in IDBtranscation.

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

::: dom/base/IndexedDBHelper.jsm
@@ +175,3 @@
>          if (DEBUG) debug("Caught error on transaction");
>          /*
>           * event.target.error may be null

This needs an update too. Or just remove the comment.

@@ +175,4 @@
>          if (DEBUG) debug("Caught error on transaction");
>          /*
>           * event.target.error may be null
>           * if txn was aborted by calling txn.abort()

Hm, I think txn.error shouldn't be null when we this abort handler is called.
You can remove the comment I think.
Attachment #9001641 - Flags: review?(jvarga) → review+
Address the comment
Attachment #9001641 - Attachment is obsolete: true
Attachment #9001671 - Flags: review+
Pushed by shes050117@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f0e23762df5
Get the result only when it's defined in IDBtranscation. r=janv
https://hg.mozilla.org/mozilla-central/rev/3f0e23762df5
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Blocks: 1484615
You need to log in before you can comment on or make changes to this bug.