Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Remove LocalStorage synchronization on storage events in Safari #8408

Merged
merged 4 commits into from
Aug 6, 2024
Merged

Remove LocalStorage synchronization on storage events in Safari #8408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ee6def9
Remove LocalStorage synchronization on storage events in Safari
dlarocque Aug 2, 2024
0a74735
Remove unused imports
dlarocque Aug 6, 2024
c9e8bb6
Run formatter
dlarocque Aug 6, 2024
7d069bc
Add changeset
dlarocque Aug 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/mighty-shirts-pump.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@firebase/auth': patch
---

Remove localStorage synchronization on storage events in Safari iframes. See [GitHub PR #8408](https://github.com/firebase/firebase-js-sdk/pull/8408).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI the changeset tool automatically links to the PR that the changeset was added in when creating the Version Packages PR, but this does make the copy-paste into release notes easier.

37 changes: 1 addition & 36 deletions packages/auth/src/platform_browser/persistence/local_storage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,7 @@

import { Persistence } from '../../model/public_types';

import { getUA } from '@firebase/util';
import {
_isSafari,
_isIOS,
_isIframe,
_isMobileBrowser,
_isIE10
} from '../../core/util/browser';
import { _isMobileBrowser, _isIE10 } from '../../core/util/browser';
import {
PersistenceInternal as InternalPersistence,
PersistenceType,
Expand All @@ -33,11 +26,6 @@ import {
} from '../../core/persistence';
import { BrowserPersistenceClass } from './browser';

function _iframeCannotSyncWebStorage(): boolean {
const ua = getUA();
return _isSafari(ua) || _isIOS(ua);
}

// The polling period in case events are not supported
export const _POLLING_INTERVAL_MS = 1000;

Expand All @@ -64,9 +52,6 @@ class BrowserLocalPersistence
// eslint-disable-next-line @typescript-eslint/no-explicit-any
private pollTimer: any | null = null;

// Safari or iOS browser and embedded in an iframe.
private readonly safariLocalStorageNotSynced =
_iframeCannotSyncWebStorage() && _isIframe();
// Whether to use polling instead of depending on window events
private readonly fallbackToPolling = _isMobileBrowser();
readonly _shouldAllowMigration = true;
Expand Down Expand Up @@ -112,26 +97,6 @@ class BrowserLocalPersistence
this.stopPolling();
}

// Safari embedded iframe. Storage event will trigger with the delta
// changes but no changes will be applied to the iframe localStorage.
if (this.safariLocalStorageNotSynced) {
// Get current iframe page value.
const storedValue = this.storage.getItem(key);
// Value not synchronized, synchronize manually.
if (event.newValue !== storedValue) {
if (event.newValue !== null) {
// Value changed from current value.
this.storage.setItem(key, event.newValue);
} else {
// Current value deleted.
this.storage.removeItem(key);
}
} else if (this.localCache[key] === event.newValue && !poll) {
// Already detected and processed, do not trigger listeners again.
return;
}
}

const triggerListeners = (): void => {
// Keep local map up to date in case storage event is triggered before
// poll.
Expand Down
Loading