Support opening new PWA windows when in fullscreen

[a-stewart]

The isStandalone() method used to determine if the app is running as a PWA doesn't work when the PWA is full screen (on Mac OS at least).

This is because, when the PWA is full screen, the display mode is full screen rather than standalone.

Unfortunately, we can't just also check for display-mode: full screen because this is also true when vscode is running in a regular browser tab but full screen.

Status	Window state	Display mode
PWA	fullscreen	fullscreen
PWA	windowed	standalone
Chromew Tab	fullscreen	fullscreen
Chromew Tab	windowed	browser

One possible solution could be to use both full screen and standalone initially but only trigger on a change to standalone or browser, not on a change from them.

display-mode before	display-mode after	display mode changed to standalone or browser?	value of isStandalone()
browser	fullscreen	no	false
browser	standalone	yes	true
standalone	fullscreen	no	true
standalone	browser	yes	false
fullscreen	standalone	yes	true
fullscreen	browser	yes	false

Eg, something like:

	const standaloneMatchMedia = window.matchMedia('(display-mode: standalone)');
	const browserMatchMedia = window.matchMedia('(display-mode: browser)');
	standalone = !browserMatchMedia.matches;       // Consider a full screen window on start a PWA
	// standalone = standaloneMatchMedia.matches;  // Don't consider a full screen window on start a PWA
	addMatchMediaChangeListener(standaloneMatchMedia, ({ matches }) => {
		standalone ||= matches;
	});
	addMatchMediaChangeListener(browserMatchMedia, ({ matches }) => {
		standalone &&= !matches;
	});

The downsides of this are.

1. Starting a browser tab whilst in full screen would incorrectly be considered as a PWA
2. Switching between browser and PWA whilst full screen would not work (though it doesn't at the moment either so no regression here)

We could shift the impact of 1 a bit, by not considering an instance as a PWA on startup if it is full screen.
So if you open standalone and go full screen it remains a PWA, but if you start in full screen (is that even possible) then it would think it is a browser tab.

Another option, would be to consider display-mode: full screen the same as display-mode: standalone. This would always result in the desired effect for a PWA, but would mean that when running in a chrome tab, actions like "New Window" would instead open in a new window (or the PWA if it exists), rather than a new tab in the current browser instance.

The ideal solution would be to distinguish between full screen in a browser tab and full screen in a PWA somehow, but I have not found a way in which that can be done yet.

[bpasero]

Open for help via PR. Thanks for the analysis.

[bpasero]

@a-stewart as original author of #143287 would you have a look at the suggested solution in #156424 ?

[a-stewart]

Yep, that solution looks good to me, in practice it works exactly the same as what is described above.

[vscodenpa]

This bug has been fixed in the latest release of VS Code Insiders!

@a-stewart, you can help us out by commenting /verified if things are now working as expected.

If things still don't seem right, please ensure you're on version 73fd3f1 of Insiders (today's or later - you can use Help: About in the command palette to check), and leave a comment letting us know what isn't working as expected.

Happy Coding!

[a-stewart]

Hm, it didn't seem to resolve the issue for me.

1. Go to insiders.vscode.dev (confirm it is using 73fd3f1)
2. Install the app
3. File > New Window - opens a new window in a PWA as expected
4. Go full screen
5. File > New Window - opens as a browser tab, not a new PWA

I need to call it a night tonight, but I'll investigate a bit more next week.

(There are no regressions in this work that I can tell, I expect that isStandalone is returning the correct value from what the code says, so I think there may be a second issue somewhere else.)

As an aside, is there a way to get source maps on insiders.vscode.dev - would be good to see if I can put some breakpoints in. If not I can fast forward this patch onto our fork and debug it there.

[bpasero]

Reopening and again, help wanted, so feel free to open a PR to fix this that is meaningfully small.

Sourcemaps work on insiders.vscode.dev out of the box, there is nothing specific to do. They just need a bit of loading time.

[pingren]

I expect that isStandalone is returning the correct value from what the code says, so I think there may be a second issue somewhere else.

There is a second issue. It seems that window.open won't open a new standalone window if the PWA is in macOS full-screen mode.

vscode/src/vs/code/browser/workbench/workbench.ts

Lines 381 to 382 in cb79d01

	if (isStandalone()) {
	result = window.open(targetHref, '_blank', 'toolbar=no'); // ensures to open another 'standalone' window!

try PWA demo from web.dev with this issue: https://mlearn-pwa-windows-basic.glitch.me/

[pingren]

Update

While window.open won't work for full-screen PWA, using command + N or clicking through macOS menu bar will work. So maybe we can to fix the issue with using macOS shortcut. It is out of the original issue scope. Shall I open another issue to address it?

[bpasero]

Can we not just immediately on startup run window.matchMedia('(display-mode: standalone)') and if that evaluates to true, keep it and never change it because you cannot really go back from PWA to not-PWA right?

And in the other case where you transition from a tab into a PWA, only there do the dynamic approach?

I feel this is a regression from https://github.com/microsoft/vscode/pull/143287/files#diff-43a30e8aea5c247b550412be5530c9fdda1e9b3010f229761507b6be6ef914a7 ?

[pingren]

Can we not just immediately on startup run window.matchMedia('(display-mode: standalone)') and if that evaluates to true, keep it and never change it because you cannot really go back from PWA to not-PWA right?

User COULD switch PWA to non-PWA:

From my testing above, window.open API (no matter using what parameters) in full-screen PWA will open a tab in the browser. Did it open a new standalone window in full-screen PWA before?

[bpasero]

Yeah I see, but then again: if we cannot fix it properly for PWA, I would rather have "Open in Chrome" behave with a bug, than loosing the PWA mode when opening a new window. So I would tolerate this issue more over the current one.

@a-stewart thoughts?

[pingren]

@bpasero Let me clarify and forgive me if I am too wordy:

• Add match media change listener in browser ts #143287 introduced dynamic isStandalone(), but it doesn't consider when PWA enters full-screen mode.

• Fix isStandalone when PWA entering fullscreen #156424 have already fixed dynamic isStandalone() issue introduced.

• @a-stewart tested the full-screen PWA still open new tab in the browser. It is caused by window.open does not work when PWA full-screen. It is very likely NEVER WORKING.

So the only thing we need to do is fixing this line to ensure it open another 'standalone' window, even for full-screen PWA

vscode/src/vs/code/browser/workbench/workbench.ts

Line 382 in cb79d01

result = window.open(targetHref, '_blank', 'toolbar=no'); // ensures to open another 'standalone' window!

[bpasero]

I am having a hard time to debug this, somehow devtools does not work in PWA for me. I am still not fully understanding if the isStandalone check is failing when in fullscreen or if really that code you mention fails. My gut feeling is that isStandalone is false and thus we call this instead:

vscode/src/vs/code/browser/workbench/workbench.ts

Line 384 in 5cf4a84

result = window.open(targetHref);

[pingren]

You could install this demo and try window.open(targetHref, '_blank', 'toolbar=no'): https://pwa-new-window.glitch.me/

Source code: https://glitch.com/edit/#!/pwa-new-window

[bpasero]

Yeah you are right, that code opens a new tab for me when in fullscreen mode, so maybe that is actually a Chrome bug?

[a-stewart]

Hi,

Sorry for being a bit slow getting back onto this. Yeah, from my testing a well, I agree with what pingren is saying - this is not a regression, but likely something that never worked.

I think with the example the pingren posted, raising this as a bug with the Chromium is probably be the best bet?

The two PRs that have already been mentioned are both still required as they are what ensure that window.open is called "correctly":

• Add match media change listener in browser ts #143287
• Fix isStandalone when PWA entering fullscreen #156424

[bpasero]

👍 feel free to report, some related issues:

• https://bugs.chromium.org/p/chromium/issues/detail?id=1281676&q=pwa%20fullscreen&can=2
• https://bugs.chromium.org/p/chromium/issues/detail?id=1273386&q=pwa%20fullscreen&can=2
• https://bugs.chromium.org/p/chromium/issues/detail?id=850465&q=pwa%20fullscreen&can=2

[a-stewart]

The second one looks close to the initial issue reported here that the second PR is working around - I'll follow that one.

For the specific issue we are now seeing, I have created https://bugs.chromium.org/p/chromium/issues/detail?id=1351346

[a-stewart]

Update on this - This should now be resolved in upstream Chromium, and will hopefully make its way to the Chrome auto-updated version soon.

https://chromiumdash.appspot.com/commits?commit=fbf5696bd186276df7b37f852592bd48efd5c606&platform=Mac

Looks like it should be included in 107.0.5290.0 with a stable release of October 27th (https://chromiumdash.appspot.com/schedule).

It looks like the dev branch of Chrome is on 107.0.5286.0 so hopefully, the fix should pull through on that soon as I can verify it.

I am not sure what the release schedule is for Edge, and how quickly that will pick up this fix.

[pingren]

Thank you for updates.

I am not sure what the release schedule is for Edge, and how quickly that will pick up this fix.

The trigger for Edge Beta and Stable major releases is an equivalent Chromium release.

[bpasero]

Anything to do here still?

[vscodenpa]

This issue has been closed automatically because it needs more information and has not had recent activity. See also our issue reporting guidelines.

Happy Coding!