mozilla / fxa-content-server

DEPRECATED - Migrated to https://github.com/mozilla/fxa
Mozilla Public License 2.0
163 stars 120 forks source link

Discuss: remove crosstab.js dependency #3415

Closed shane-tomlinson closed 7 years ago

shane-tomlinson commented 8 years ago

Ref #3414

crosstab was useful when we first started FxA because it allows straight forward communication between two tabs within the same browser. Firefox implemented BroadcastChannel which serves the same purpose, without the use of a library.

crosstab's communication mechanism between two tabs relies on shuttling data via localStorage. If there are problems accessing localStorage [1][2][3][4] within crosstab, our startup process fails and the user can see a blank screen. If crosstab is able to read from but not write to localStorage, then an error is sent to Sentry every 3 seconds. Sentry reports > 5M instances of "main in setLocalStorageItem" types of errors, making it impossible to determine whether a large number of users see this problem or whether it's a small number of users who submit a large number of reports.

Removing crosstab will affect users of Firefox < 38 or users who signup to FxA from other browsers. The password reset flow from about:accounts will require users to return to the original tab to re-enter their credentials. Tab signin/signout state will not stay in sync. crosstab already disables itself for mobile users, so Fennec should not be affected. Removing crosstab will make for a marginally worse UX for a subset of users, but will result in less code to maintain and fewer code paths to worry about.

[1] - https://github.com/mozilla/fxa-content-server/issues/2969 [2] - https://github.com/mozilla/fxa-content-server/issues/2913 [3] - https://github.com/mozilla/fxa-content-server/issues/3414 [4] - https://bugzilla.mozilla.org/show_bug.cgi?id=1240238

cc @vladikoff, @ckarlof, @rfk, @philbooth, @zaach, @vbudhram, @ryanfeeley, @jrgm

shane-tomlinson commented 8 years ago

Removing crosstab will also allow us to stop maintaining and patching it for our own uses, e.g., #3416

philbooth commented 8 years ago

I say get rid of it!

vladikoff commented 8 years ago

related to https://github.com/mozilla/fxa-content-server/issues/3414

I have a feeling that firstrun starts so quickly that Firefox's localstorage apis are not properly initialized. While I agree with removing crosstab.js, I feel like we should also try to help fix the browser itself. Maybe suggest deploying a telemetry probe to look for "NS_ERROR_STORAGE_IOERR, NS_ERROR_FILE_NO_DEVICE_SPACE, NS_ERROR_FILE_CORRUPTED, NS_ERROR_STORAGE_CONSTRAINT, NS_ERROR_FAILURE, NS_ERROR_STORAGE_BUSY, NS_ERROR_OUT_OF_MEMORY," errors?

vladikoff commented 8 years ago

We also have 'NS_ERROR_STORAGE_CONSTRAINT' in https://bugzilla.mozilla.org/show_bug.cgi?id=1240238 . It happens too early for sentry to even catch that error.

shane-tomlinson commented 8 years ago

I have a feeling that firstrun starts so quickly that Firefox's localstorage apis are not properly initialized. While I agree with removing crosstab.js, I feel like we should also try to help fix the browser itself.

That's the exact thought I was trying to convey in this comment.

rfk commented 8 years ago

I support removing this as long as the experience on older browsers degrades gracefully. If the user finds they have to re-enter their password into one tab because we couldn't shuttle the key material across to it, that's probably OK. If the user finds themselves stuck and unable to proceed, that's obviously not.

One additional thing to check, is the "relier keys" functionality that we added for Hello, which IIRC does some cross-tab stuff to make sure that the webchannel message fires from the right tab in order to send the keys to the browser.

shane-tomlinson commented 8 years ago

One additional thing to check, is the "relier keys" functionality that we added for Hello, which IIRC does some cross-tab stuff to make sure that the webchannel message fires from the right tab in order to send the keys to the browser.

I am of the opinion that we should support back to the last LTS, if Hello works there w/o crosstab, and the experience for other reliers degrades well enough, rip it out.

ckarlof commented 8 years ago

If we really hate it, let's ballpark what % of FxA users use Firefox < 38. If it's low, let's remove it. Since most new signins/signups are coming from first run, I suspect our decision is going to be "burn it with fire".

vladikoff commented 8 years ago

from mtg: check old clients

eoger commented 8 years ago

Chrome will have BroadcastChannel API in its next release (54)