content icon indicating copy to clipboard operation
content copied to clipboard

Issue with "IDBIndex.getAllKeys()": The article does not specify the form of the information returned in the result property of the request object

Open HelmZalee opened this issue 4 years ago • 8 comments

MDN URL: https://developer.mozilla.org/en-US/docs/Web/API/IDBIndex/getAllKeys

What information was incorrect, unhelpful, or incomplete?

"The getAllKeys() method of the IDBIndex interface instantly retrieves the primary keys of all objects inside the index, setting them as the result of the request object."

The article does not specify the form of the information returned in the result property of the request object.

It is an Array, which is what it should be according to w3c.github.io/IndexedDB/#ref-for-dom-idbindex-getallkeys%E2%91%A0. The word "array", however, does not appear in the article.

More importantly, FF and Chrome seem to behave very differently. FF seems to behave as the article describes. It returns an IDBRequest object that does have a result property that is an Array of keys.

Chrome, in contrast, seems to return an IDBRequest object unpopulated with a 'result' object, and with a 'readyState' property value 'pending'. Key values are not available "immediately". They are available only to the success event handler.

Specific section or headline?

The topmost headline: IDBIndex.getAllKeys().

What did you expect to see?

After trying to write this Issue clearly, I found that FF and Chrome do not seem to behave in the same way. The section cannot be written correctly without referring to specific browsers. That seems unfortunate!

Chrome returns an IDBRequest object that has no 'result' property, and a 'readyState' property with the value 'pending'. The keys seem to be available only as the result property of the event passed to the success handler assigned to the IDBRequest.

The behavior of FF is as the MDN article describes: the object returned by the getAllKeys method has a 'result' property that contains the keys, and a readyState property whose value is 'done'.

In writing this, i have looked at what the W3 spec (cited above) says. And looked again. It is no wonder that FF and Chrome don't behave the same way. In bending over backward to be clear and specific, W3 manages to be incomprehensible (to me, anyway).

Did you test this? If so, how?

Wrote HTML and js to define a simple db, create a couple of stores, one with an index, and perform operations on that db. Then I stepped through the code in both FF and Chrome.

MDN Content page report details
  • Folder: en-us/web/api/idbindex/getallkeys
  • MDN URL: https://developer.mozilla.org/en-US/docs/Web/API/IDBIndex/getAllKeys
  • GitHub URL: https://github.com/mdn/content/blob/main/files/en-us/web/api/idbindex/getallkeys/index.md
  • Last commit: https://github.com/mdn/content/commit/b0b00060b28f06c75c2174cddd42581a9d438df0
  • Document last modified: 2021-10-22T15:52:01.000Z

HelmZalee avatar Dec 20 '21 18:12 HelmZalee

@inexorabletash may be able to give us some guidance here

sideshowbarker avatar Dec 28 '21 22:12 sideshowbarker

All IDB queries return an IDBRequest object with a result and error property, but neither should have a value before an "error" or "success" event fires and readyState updates from "pending" to "done".

All queries are async and so "instantly" would be misleading.

If you're sampling the IDBRequest properties before the event fires, you're going to have a race between your code and the query completing.

inexorabletash avatar Dec 28 '21 22:12 inexorabletash

That said, bugs are possible. If you have sample code that demonstrates a behavior difference between browsers, please share it.

inexorabletash avatar Dec 28 '21 22:12 inexorabletash

All queries are async and so "instantly" would be misleading.

I opened #11554 with a fix which changes that part to:

The getAllKeys() method of the IDBIndex interface asynchronously retrieves the primary keys…

sideshowbarker avatar Dec 29 '21 00:12 sideshowbarker

Agree with everything inexorabletash said: "neither should have a value...". But...i thought it was an empty call stack that would allow states to change and events to fire on IDBReqest. Would it be necessary to assign a success handler? A good idea, but I thought not necesssary.

Anyway, here's a screenshot that illustrates that (a) getAllKeys has just returned an IDBRequest object and (b) the readyState property value is "done". (I did this 5 minutes ago, using FF 95.0.2 (64-bit). Relieved to see a recent update didn't make it go away, proving I had lost my mind.) image

If you promise not to laugh at my poor effort, I'll attach two files: idb.html and idb.js. Sure you can figure it out, but idb.js is expected to be in a directory "js" contained in the directory where idb.html is found. If it matters (don't think it does) this is all run locally using IIS in Window 10. And oops, just noticed the "builtIns.js", it is irrelevant.

"listNames" in idb.js manifests the issue. The html puts up a bunch of buttons for test cases. List stores, List courses or ListRptOps will run "listNames".

I look forward to learning something from this.

HelmZalee avatar Dec 29 '21 00:12 HelmZalee

I'll take a look when I'm back at work in a few weeks. Note that single stepping in the debugger may not be a good test, so verify without the debugger.

inexorabletash avatar Dec 29 '21 01:12 inexorabletash

I did investigate what happens w/ & w/o the debugger, in two environments. W/O the debugger, the code runs as it should: readyState is initially "pending".

As I said, the invalid readyState ('done') occurs only when single-stepping in the FF debugger. I added the "if" block after experimentally determining the thing ran differently in that mode. It took not a little time.

HelmZalee avatar Dec 29 '21 01:12 HelmZalee

@inexorabletash Any chance you have time now to take a look at this?

sideshowbarker avatar Nov 07 '22 06:11 sideshowbarker

Ah, sorry for leaving a "I'll take a look when I'm back..."

The reporter indicated the problem only occurred in FF when single-stepping with the debugger, and the change in 770bb44f36c4f1e9816118857cbf5338ada8f80a LGTM

I don't think there's anything actionable here?

inexorabletash avatar Nov 08 '22 00:11 inexorabletash

@inexorabletash OK, thanks — closing per https://github.com/mdn/content/issues/11349#issuecomment-1306414960 and previous comments from @HelmZalee

sideshowbarker avatar Nov 08 '22 03:11 sideshowbarker