Fix modernizr breakage for TBB/ESR52

Our patch for #16528 (moved) doesn't work with electrolysis enabled, so we need a new patch for TBB/ESR52.

added TorBrowserTeam201703R component::applications/tor browser ff52-esr owner::tbb-team priority::medium resolution::fixed severity::normal sponsor::4 status::closed tbb-7.0-must tbb-no-uplift type::defect labels

Trac:
Keywords: tbb-52esr deleted, ff52esr added

Trac:
Keywords: ff52esr deleted, ff52-esr added

Trac:
Keywords: N/A deleted, TorBrowserTeam201702 added

This is Sponsor4 work

Trac:
Sponsor: N/A to Sponsor4

Here's a patch for review: https://github.com/arthuredelstein/tor-browser/tree/21308

To explain this patch a little:

In private browsing mode, calling

var req = window.indexedDB.open("anydbname")

returns without throwing an error, but results in

req.name = "InvalidStateError".

Whereas dom.indexeddb.enabled = false, var req = window.indexedDB.open("anydbname") throws an InvalidStateError instead.

The Modernizr script fails to handle the latter case. So this patch changes the behavior of the code when dom.indexedb.enabled = false so that no error is thrown, but instead window.indexedDB = null. This is correctly handled by Modernizr, and also matches the convention where the use of a JS API is preceding by checking for its presence, e.g.:

if (window.indexedDB) {
  var req = indexedDB.open(...)
  // More indexedDB usage
}

I also tested this with Twitter and it appears to get it working properly again.

Trac:
Keywords: TorBrowserTeam201702 deleted, TorBrowserTeam201702R added
Status: new to needs_review

r=brade, r=mcs This looks good to us. Nice work! We tested with the Modernizr script (using our own little test page) and the right thing seemed to happen.

Replying to mcs:

r=brade, r=mcs This looks good to us. Nice work! We tested with the Modernizr script (using our own little test page) and the right thing seemed to happen.

Thanks for reviewing this! I added a small mochitest to make sure it's working correctly. https://github.com/arthuredelstein/tor-browser/commit/21308+2 I will this add patch to the latest #20680 (moved) branch.

Replying to arthuredelstein:

Our patch for #16528 (moved) doesn't work with electrolysis enabled, so we need a new patch for TBB/ESR52.

Hm. Do you happen to know why that's the case? Because what we do with the patch we currently have is just disabling the pref checks and relying solely on private browsing mode (PBM) checks. And surely, IndexedDB is still disabled in PBM. Thus, those checks should still work as-is.

Second point, I assume the patch in comment:5 works as well with e10s disabled? Because it is still unclear right now how stable e10s will be in a Tor Browser ESR52. We might need to disable it or enable it later.

Trac:
Status: needs_review to needs_information

No February anymore.

Trac:
Keywords: TorBrowserTeam201702R deleted, TorBrowserTeam201703R added

Trac:
Keywords: N/A deleted, tbb-7.0-must added

Replying to gk:

Replying to arthuredelstein:

Our patch for #16528 (moved) doesn't work with electrolysis enabled, so we need a new patch for TBB/ESR52.

Hm. Do you happen to know why that's the case? Because what we do with the patch we currently have is just disabling the pref checks and relying solely on private browsing mode (PBM) checks. And surely, IndexedDB is still disabled in PBM. Thus, those checks should still work as-is.

The old patch was causing crashes with e10s. I don't know why.

Second point, I assume the patch in comment:5 works as well with e10s disabled? Because it is still unclear right now how stable e10s will be in a Tor Browser ESR52. We might need to disable it or enable it later.

In my hands it seems to be working OK with e10s enabled or disabled.

Trac:
Status: needs_information to needs_review

Okay, could you pick that on top of your up-to-date 20680 branch and close this ticket then?

Replying to gk:

Okay, could you pick that on top of your up-to-date 20680 branch and close this ticket then?

Done. The corresponding Mozilla bug is at https://bugzilla.mozilla.org/1192643

Trac:
Status: needs_review to closed
Resolution: N/A to fixed

For the record: in #23745 (moved) we reverted this patch because it was no longer needed. So we also won't need to uplift it.

Parent was: #20680 (moved)

Trac:
Parent: #20680 (moved) to N/A
Keywords: N/A deleted, tbb-no-uplift added

closed

mentioned in issue #23745 (moved)

moved to tpo/applications/tor-browser#21308 (closed)