fix: cp-12.15.0 Prevent fullscreen UI from opening every startup #31332

Gudahtt · 2025-03-26T20:23:33Z

Description

Prevent tab from opening upon every browser startup.

A recent PR updated the logic for how we determine whether the extension was just installed or not; we used to check state, but now we use the runtime.onInstalled event instead to store a flag in session storage indicating whether the extension was just installed.

Unfortunately this PR had a couple of bugs; the condition using this flag was accidentally reversed, and the read from session storage was never successful because of a race condition (the write had not finished yet).

To address both problems, the logic for detecting first install was refactored to use a global variable instead of session storage. Globals can be accessed and written synchronously, preventing any possibility of a race condition.

The solution was complicated by the fact that MV3 builds must add an onInstalled listener in the root service worker module, otherwise the listener is never called (we tested this experimentally). It was also unclear whether the listener would always run before or after the background.js script loading, and we need to trigger an action in background.js on install. But we've accounted for both possibilities in this solution.

Related issues

Fixes: #30924

Manual testing steps

Create a production-like build (yarn dist and yarn dist:mv2), or download the builds from the metamaskbot comment
Test that the window opens only on first install, not on later browser restarts.

Screenshots/Recordings

N/A

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
I've completed the PR template to the best of my ability
I’ve included tests if applicable
I’ve documented my code using JSDoc format if applicable
I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Pre-merge reviewer checklist

I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Gudahtt · 2025-03-27T21:53:35Z

I haven't written any tests for this yet. This is a bit trick to test because we can't really unit test these integrations with the extension/service worker API. We won't know that the integration works without a real browser. And we can't test this using a standard E2E test build because the behavior we're testing (opening a new tab on install) is disabled.

We should be able to write an E2E test that uses a production build though, similar to the vault decryption build. Given the urgency of fixing this problem, I'd prefer to use manual testing for now. I'll follow up with the E2E test in a later PR.

app/scripts/background.js

Gudahtt · 2025-03-27T22:03:19Z

app/scripts/background.js

    addAppInstalledEvent();
    platform.openExtensionInBrowser();
  }
-  onNavigateToTab();


I'm not sure why this function was here, it's not really related to the install event. This was being called irrespective of whether this session was the first after install as well, which is confusing.

I've moved it down to initBackground, so it should get called as part of initialization the same way it always was before.

davidmurdoch

Looks great! Solution is a maybe bit non-idiomatic, but still much easier to reason about than the async storage complexity.

Left only some nits/questions, nothing that must be changed.

I'll give it a few runs locally tomorrow before give it the ✅

app/scripts/app-init.js

davidmurdoch · 2025-03-28T00:17:36Z

app/scripts/background.js

+  // This condition is for when `background.js` was loaded before the `onInstalled` listener was
+  // called.
+} else {
+  globalThis.__metamaskTriggerOnInstall = () => onInstall();


app/scripts/app-init.js

Gudahtt · 2025-03-28T11:31:50Z

Not quite sure why the "App Installed Event" E2E tests are failing.

app/scripts/app-init.js

metamaskbot · 2025-03-28T18:47:49Z

Builds ready [7e06ea8]

builds: chrome, firefox
builds (flask): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
bundle size: Bundle Size Stats
user-actions-benchmark: User Actions Stats
storybook: Storybook
typescript migration: Dashboard
all CircleCI artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6, 7, 8
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12
- content-script: 0
- offscreen: 0

UI Startup Metrics (1191 ± 58 ms)

Platform	BuildType	Page	Metric	Mean (ms)	Min (ms)	Max (ms)	Std Dev (ms)	P 75 (ms)	P 95 (ms)
Chrome	Browserify	Home	uiStartup	1191	1073	1343	58	1237	1296
			load	1040	942	1221	58	953	991
			domContentLoaded	1035	938	1216	57	956	992
			domInteractive	16	13	35	4	15	27
			firstPaint	771	76	1220	397	249	977
			backgroundConnect	9	6	45	4	9	10
			firstReactRender	19	14	41	4	19	28
			getState	11	4	33	7	6	8
			initialActions	0	0	1	0	0	1
			loadScripts	819	728	1004	57	850	924
			setupStore	7	5	13	2	7	8
	Webpack	Home	uiStartup	933	753	1201	78	944	971
			load	796	611	919	58	829	870
			domContentLoaded	789	592	911	59	825	864
			domInteractive	15	11	41	7	13	34
			firstPaint	437	60	875	339	819	863
			backgroundConnect	16	10	47	8	14	39
			firstReactRender	14	12	28	4	13	25
			getState	6	4	13	2	6	8
			initialActions	0	0	1	0	0	1
			loadScripts	787	582	895	58	823	857
			setupStore	7	5	17	2	7	8
Firefox	Browserify	Home	uiStartup	1405	1221	1962	145	1432	1753
			load	1266	1080	1807	141	1309	1574
			domContentLoaded	1266	1080	1806	141	1309	1574
			domInteractive	101	37	163	28	84	97
			firstPaint	NaN	NaN	NaN	NaN	NaN	NaN
			backgroundConnect	24	17	54	7	25	40
			firstReactRender	23	19	51	5	25	31
			getState	7	4	30	3	7	8
			initialActions	0	0	1	0	0	1
			loadScripts	1244	1062	1783	139	1288	1546
			setupStore	6	3	18	2	6	7
	Webpack	Home	uiStartup	1007	846	1751	161	911	973
			load	883	737	1495	144	831	977
			domContentLoaded	883	736	1494	144	830	977
			domInteractive	115	36	173	25	144	90
			firstPaint	NaN	NaN	NaN	NaN	NaN	NaN
			backgroundConnect	21	14	74	9	22	36
			firstReactRender	19	16	40	3	19	25
			getState	10	4	106	13	8	9
			initialActions	0	0	1	0	0	1
			loadScripts	865	722	1464	140	824	983
			setupStore	8	5	37	4	7	8

Bundle size diffs [🚀 Bundle size reduced!]

background: -404 Bytes (-0.01%)
ui: 0 Bytes (0%)
common: 0 Bytes (0%)

metamaskbot · 2025-03-28T19:49:34Z

Builds ready [18e2d1b]

builds: chrome, firefox
builds (flask): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
bundle size: Bundle Size Stats
user-actions-benchmark: User Actions Stats
storybook: Storybook
typescript migration: Dashboard
all CircleCI artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6, 7, 8
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12
- content-script: 0
- offscreen: 0

UI Startup Metrics (1172 ± 54 ms)

Platform	BuildType	Page	Metric	Mean (ms)	Min (ms)	Max (ms)	Std Dev (ms)	P 75 (ms)	P 95 (ms)
Chrome	Browserify	Home	uiStartup	1172	1057	1329	54	1206	1259
			load	1021	931	1165	50	954	992
			domContentLoaded	1014	928	1157	51	950	990
			domInteractive	15	12	32	4	15	26
			firstPaint	723	92	1165	399	247	981
			backgroundConnect	11	6	62	9	9	10
			firstReactRender	19	13	46	5	20	29
			getState	11	4	35	7	6	8
			initialActions	0	0	4	0	0	1
			loadScripts	800	700	951	52	835	884
			setupStore	7	5	13	2	7	9
	Webpack	Home	uiStartup	987	819	1293	83	982	995
			load	841	696	962	49	861	935
			domContentLoaded	836	692	950	48	854	924
			domInteractive	16	12	44	7	14	35
			firstPaint	511	58	906	348	852	868
			backgroundConnect	16	11	42	8	15	40
			firstReactRender	14	12	29	3	14	25
			getState	7	4	12	1	8	8
			initialActions	0	0	1	0	0	1
			loadScripts	833	691	939	47	852	914
			setupStore	7	5	16	2	7	9
Firefox	Browserify	Home	uiStartup	1372	1188	1929	152	1400	1782
			load	1234	1063	1790	146	1268	1623
			domContentLoaded	1233	1063	1790	146	1267	1623
			domInteractive	108	39	180	24	81	96
			firstPaint	NaN	NaN	NaN	NaN	NaN	NaN
			backgroundConnect	24	16	152	14	25	35
			firstReactRender	22	18	51	5	24	29
			getState	7	4	23	3	7	8
			initialActions	0	0	1	0	0	1
			loadScripts	1211	1044	1767	143	1249	1598
			setupStore	6	4	27	3	6	7
	Webpack	Home	uiStartup	968	827	1573	160	883	982
			load	849	723	1334	141	808	936
			domContentLoaded	848	723	1334	141	808	935
			domInteractive	115	34	197	23	176	96
			firstPaint	NaN	NaN	NaN	NaN	NaN	NaN
			backgroundConnect	20	13	82	11	22	33
			firstReactRender	18	16	27	2	18	24
			getState	9	4	85	13	7	85
			initialActions	0	0	1	0	0	1
			loadScripts	832	709	1310	138	795	925
			setupStore	8	5	59	8	7	8

Bundle size diffs [🚀 Bundle size reduced!]

background: -404 Bytes (-0.01%)
ui: 0 Bytes (0%)
common: 0 Bytes (0%)

davidmurdoch

On Firefox I was automatically greeted with the "Welcome back" login screen, not the "Let's get started" screen, in fullscreen after installing the extension as a "Temporary Add-on".

My hunch is that this is because I had previously installed it this way, so this was a new install, but firefox persisted the database from the last time I installed MM as a Temporary add-on.

If I manually remove MM (instead of waiting for it to expire and get automatically removed by FF) and then install it again it opens in full screen with the "Let's get started" screen, as expected.

I think this behavior is fine, it is a change from the original way we did this (check if we had written to storage before), so I just wanted to mention it.

Gudahtt · 2025-03-31T16:38:37Z

E2E test added in #31435

metamaskbot added the team-wallet-framework label Mar 26, 2025

Gudahtt force-pushed the fix-first-time-install-check branch 2 times, most recently from 5578d50 to 79237e9 Compare March 27, 2025 21:33

Gudahtt force-pushed the fix-first-time-install-check branch from 79237e9 to d4713a6 Compare March 27, 2025 21:57

Gudahtt commented Mar 27, 2025

View reviewed changes

app/scripts/background.js Show resolved Hide resolved

Gudahtt commented Mar 27, 2025

View reviewed changes

davidmurdoch reviewed Mar 28, 2025

View reviewed changes

Gudahtt force-pushed the fix-first-time-install-check branch from 2f3b1e3 to ac3a801 Compare March 28, 2025 16:50

Gudahtt changed the title ~~fix: Prevent fullscreen UI from opening every startup~~ fix: cp-12.15.0 Prevent fullscreen UI from opening every startup Mar 28, 2025

davidmurdoch reviewed Mar 28, 2025

View reviewed changes

app/scripts/app-init.js Outdated Show resolved Hide resolved

Gudahtt added 11 commits March 28, 2025 15:43

WIP

4f047d5

Use globals rather than session storage to track first install

4cb7863

Add comments, cleanup global namespace

deef539

Add missing onNavigate step to background init

a4f850c

Prevent opening tab on install in e2e tests

1893fd8

Remove duplicate event and obsolete comment

5f88287

Remove install listener after install event

769b7e1

ADd type declaration for globals

57b406b

Improve comments

75e4743

Migrate globals to stateHooks to pass through to root container

6c277ee

Improve JSDoc type specificity

7e06ea8

Gudahtt force-pushed the fix-first-time-install-check branch from 006de3c to 7e06ea8 Compare March 28, 2025 18:13

Fix lint error by annotating stateHooks assignment with type

18e2d1b

Gudahtt marked this pull request as ready for review March 28, 2025 18:45

davidmurdoch approved these changes Mar 28, 2025

View reviewed changes

Gudahtt enabled auto-merge March 28, 2025 21:13

HowardBraham approved these changes Mar 28, 2025

View reviewed changes

Gudahtt added this pull request to the merge queue Mar 28, 2025

Merged via the queue into main with commit 108263d Mar 28, 2025
149 checks passed

Gudahtt deleted the fix-first-time-install-check branch March 28, 2025 23:08

github-actions bot locked and limited conversation to collaborators Mar 28, 2025

metamaskbot added the release-12.17.0 Issue or pull request that will be included in release 12.17.0 label Mar 28, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: cp-12.15.0 Prevent fullscreen UI from opening every startup #31332

fix: cp-12.15.0 Prevent fullscreen UI from opening every startup #31332

Uh oh!

Gudahtt commented Mar 26, 2025 •

edited

Loading

Uh oh!

Gudahtt commented Mar 27, 2025

Uh oh!

Uh oh!

Gudahtt Mar 27, 2025 •

edited

Loading

Uh oh!

davidmurdoch left a comment

Uh oh!

Uh oh!

davidmurdoch Mar 28, 2025

Uh oh!

Uh oh!

Gudahtt commented Mar 28, 2025

Uh oh!

Uh oh!

metamaskbot commented Mar 28, 2025

Uh oh!

metamaskbot commented Mar 28, 2025

Uh oh!

davidmurdoch left a comment

Uh oh!

Uh oh!

Gudahtt commented Mar 31, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

fix: cp-12.15.0 Prevent fullscreen UI from opening every startup #31332

fix: cp-12.15.0 Prevent fullscreen UI from opening every startup #31332

Uh oh!

Conversation

Gudahtt commented Mar 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues

Manual testing steps

Screenshots/Recordings

Pre-merge author checklist

Pre-merge reviewer checklist

Uh oh!

Gudahtt commented Mar 27, 2025

Uh oh!

Uh oh!

Gudahtt Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidmurdoch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

davidmurdoch Mar 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Gudahtt commented Mar 28, 2025

Uh oh!

Uh oh!

metamaskbot commented Mar 28, 2025

Uh oh!

metamaskbot commented Mar 28, 2025

Uh oh!

davidmurdoch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Gudahtt commented Mar 31, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Gudahtt commented Mar 26, 2025 •

edited

Loading

Gudahtt Mar 27, 2025 •

edited

Loading