Skip to content

indexeddbblob: Fix test not triggering, update metadata - #2608

Open
Xenonym wants to merge 2 commits into
Modernizr:masterfrom
Xenonym:fix/indexeddbblob-not-triggering
Open

Xenonym wants to merge 2 commits into
Modernizr:masterfrom
Xenonym:fix/indexeddbblob-not-triggering

Conversation

@Xenonym

@Xenonym Xenonym commented Oct 5, 2020 •

Copy link
Copy Markdown
Contributor
  • indexeddbblob: Fix test never triggering

    The indexedbblob test never triggers, even on platforms that support IndexedDB or blobs in IndexedDB databases.

    This is because in 164e757, indexeddb was changed to an async test, but indexeddbblob still assumes that indexeddb test is synchronous. Therefore, the indexeddb check is never true and indexedbblob never triggers.

    The onsuccess / onerror / exception handlers also assign the test result to a shared variable before calling addTest(), which results in the test result being always false as the variable will not be updated synchronously before addTest().

    Let's:

    • wrap the indexeddbblob test in a on('indexeddb.deletedatabase') listener, so that it will trigger when indexeddb and indexeddb.deletedatabase pass
    • call addTest() in the onsuccess / onerror / exception handlers instead of assigning to a shared variable
  • indexeddbblob: Add missing metadata properties

    • add storage tag
    • mark as async

The `indexedbblob` test never triggers, even on platforms that support
IndexedDB or blobs in IndexedDB databases.

This is because in 164e757, `indexeddb` was changed to an async test,
but `indexeddbblob` still assumes that `indexeddb` test is synchronous.
Therefore, the `indexeddb` check is never true and `indexedbblob` never
triggers.

The `onsuccess` / `onerror` / exception handlers also assign the test
result to a shared variable before calling `addTest()`, which results
in the test result being always false as the variable will not be
updated synchronously before `addTest()`.

Let's:

* wrap the `indexeddbblob` test in a `on('indexeddb.deletedatabase')`
listener, so that it will trigger when `indexeddb` and
`indexeddb.deletedatabase` pass

* call `addTest()` in the `onsuccess` / `onerror` / exception handlers
instead of assigning to a shared variable
* add `storage` tag

* mark as async
@rejas

rejas commented Oct 6, 2020

Copy link
Copy Markdown
Member

Thx @Xenonym that sounds very reasonable. What do you think @patrickkettner sicne you wrote the original tests?

@Xenonym

Xenonym commented Oct 26, 2020 •

Copy link
Copy Markdown
Contributor Author

@rejas

Would you be so kind as to label this PR hacktoberfest-accepted or approve this PR if this is not going to be merged before Oct 31? 😃

@Monib007

Monib007 commented Oct 9, 2023

Copy link
Copy Markdown

hey could you assign me this task and let me contribute?

@rejas

rejas commented Oct 13, 2023

Copy link
Copy Markdown
Member

hey could you assign me this task and let me contribute?

could you look for another issue. This PR is stale and not currently worked but still somebody else worked on it.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants