-
Notifications
You must be signed in to change notification settings - Fork 929
Fail more gracefully when opening IndexedDb fails in Firefox private browsing #5396
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: dc38ea4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Size Analysis Report |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Can you remove the Firefox mention from the error message since we don't know where else this occurs? You can leave it in the changelog though as it addresses a specific issue.
The thing is, IndexedDb works in Chrome incognito mode. So removing Firefox from the message IMO incorrectly implies that it is broken on Chrome too. Also, the hyperlink the message is to a bug report against Firefox for IndexedDb support in private browsing mode. I'm not sure what the best wording is but I'm open to suggestions. |
I would try to be more vague and remove the link as well. You can also list it as an example of why it could be broken - but it is likely not the only reason. It would confuse users that see this error that are not on Firefox. Browsers that support private browsing with IndexedDB wouldn't show this error. |
…StateError can presumably happen for other reasons as well
Done. I've changed the error message to the following:
|
'Firefox private browsing session, in which IndexedDb is not supported ' + | ||
'(https://goo.gle/3ymSNyf): ' + | ||
error | ||
'Unable to open an IndexedDb connection. This could be due to running in a ' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The official spelling seems to be IndexedDB.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Code.FAILED_PRECONDITION, | ||
'Unable to open an IndexedDb connection. This could be due to running in a ' + | ||
'private browsing session on a browser whose private browsing sessions do not ' + | ||
'support IndexedDb. (' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I think you should move the period after the parenthesis or use :
without parenthesis.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done (changed to ":").
Improve the obscure failure that occurs when attempting to open an IndexedDb connection in a Firefox private browsing session (which is a known limitation of Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=1639542).
Before this fix, the error that occurred in this scenario in v8.10.0 would be vague:
In v9.0.0 it was even worse because it mislead readers to suggest that it was related to multi-tab:
With this fix, the error message becomes more helpful by specifically calling out Firefox private browsing as a potential culprit:
Both before and after this fix, Firestore would continue to be functional, just without persistence.
Fixes #5377