Opened 3 years ago

Closed 2 years ago

Last modified 7 months ago

#18619 closed defect (fixed)

TBB/ESR45 reports "InvalidStateError" in browser console

Reported by: arthuredelstein Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: ff45-esr, TorBrowserTeam201605R, tbb-6.0-must, tbb-no-uplift
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

An InvalidStateError appears in the browser console, unfortunately with no source filename or line number (only the word <unknown> for source). I tracked this error down to our setting the pref "dom.indexedDB.enabled" to false in 000-tor-browser.js. I haven't yet found where the error is being produced, but I presume it is a .jsm or .js file somewhere in the Firefox codebase, attempting to use an indexedDB.

Child Tickets

Change History (12)

comment:1 Changed 2 years ago by gk

Keywords: ff45-esr added

comment:2 Changed 2 years ago by gk

Parent ID: #15197

comment:3 Changed 2 years ago by gk

Keywords: TorBrowserTeam201605 added

Dragging into May to have it on our 6.0 radar.

comment:4 Changed 2 years ago by arthuredelstein

Keywords: TorBrowserTeam201605R added; TorBrowserTeam201605 removed
Status: newneeds_review

Here's a patch that attempts to fix the problem.

https://github.com/arthuredelstein/tor-browser/commit/18619+1
Hash 96e76624a5e354bb059782dbe1fdeb5f8345af4e

comment:5 in reply to:  4 ; Changed 2 years ago by mcs

Replying to arthuredelstein:

Here's a patch that attempts to fix the problem.

https://github.com/arthuredelstein/tor-browser/commit/18619+1
Hash 96e76624a5e354bb059782dbe1fdeb5f8345af4e

With this patch, an InvalidState error is no longer logged during browser startup. We still see one when we open the developer tools (Inspect Element). I don't know how difficult it would be to patch the underlying devtools/shared/async-storage.js code to avoid using IndexedDB when it has been disabled.

comment:6 in reply to:  5 ; Changed 2 years ago by arthuredelstein

Replying to mcs:

Replying to arthuredelstein:

Here's a patch that attempts to fix the problem.

https://github.com/arthuredelstein/tor-browser/commit/18619+1
Hash 96e76624a5e354bb059782dbe1fdeb5f8345af4e

With this patch, an InvalidState error is no longer logged during browser startup. We still see one when we open the developer tools (Inspect Element). I don't know how difficult it would be to patch the underlying devtools/shared/async-storage.js code to avoid using IndexedDB when it has been disabled.

Thanks for the review. That's a good suggestion. I wrote an in-memory implementation that is supposed to match the indexeedDB-based version:

https://github.com/arthuredelstein/tor-browser/commit/18619+2
Hash b875e25aade0323c85915e0c09eadaf44151524f

This seems to correctly the browser console input history during a browser session. It also appears to eliminate the InvalidStateErrors both as a result of browser startup, entering things in the browser console, or using Inspect Element.

comment:7 Changed 2 years ago by gk

Keywords: tbb-6.0-must added

comment:8 in reply to:  6 Changed 2 years ago by arthuredelstein

Replying to arthuredelstein:

This seems to correctly the browser console input history during a browser session. It also appears to eliminate the InvalidStateErrors both as a result of browser startup, entering things in the browser console, or using Inspect Element.

Also, I ran the test
./mach mochitest devtools/shared/tests/browser/browser_async_storage.js
with "dom.indexedDB.enabled" set to false, and all the tests pass.

comment:9 Changed 2 years ago by mcs

r=brade, r=mcs
Kathy and I think your code is fine and this solution is good as well. It does mean that some developer tools state will not be persistent, but at least this way it is stored while the devtools are open (and we avoid strange errors on the console).

comment:10 Changed 2 years ago by gk

Resolution: fixed
Status: needs_reviewclosed

Applied to tor-browser-45.1.0esr-6.0-1 (commit 847b5368b573508a66c7ba38a2148fe36289fe23).

comment:11 in reply to:  9 Changed 16 months ago by cypherpunks

Replying to mcs:

r=brade, r=mcs
Kathy and I think your code is fine and this solution is good as well. It does mean that some developer tools state will not be persistent, but at least this way it is stored while the devtools are open (and we avoid strange errors on the console).

Do we really need this non-clearable by NEWNYM database?
Does this patch pass https://people-mozilla.org/~spenades/test_cases/async_storage/ correctly?
Mozilla closed it as a duplicate of https://bugzilla.mozilla.org/show_bug.cgi?id=781982

comment:12 Changed 7 months ago by arthuredelstein

Keywords: tbb-no-uplift added

We no longer need this patch because we stopped setting dom.indexeddb.enabled to false in #23745. So I'm marking this as tbb-no-uplift and I opened #25215 to revert the patch. We'll keep https://bugzilla.mozilla.org/show_bug.cgi?id=1432994 open in case Mozilla wants to fix the problem.

Note: See TracTickets for help on using tickets.