fix: window focus for second instance on macOS

[deepak1556]

Since 8.3.0 the correct way to steal focus has been changed, since we no longer carry the patch in the internal build https://domoreexp.visualstudio.com/Teamspace/_git/electron-build?path=%2Fpatches%2Fcustomization%2Felectron%2Ffeat_allow_app_to_steal_focus_on_macos.patch&version=GBrelease%2F7-2-x&_a=contents

ref electron/electron#23574

[bpasero]

I think this is probably obsolete now that we went back to Electron 7, but feel free to create this PR again from the electron-8.x branch.

Feedback: if we drop our internal patch, we need to check any user of BrowserWindow.focus() to use app.focus instead, otherwise we cause regressions, no?

[deepak1556]

Converted to draft until E8 update.

Feedback: if we drop our internal patch, we need to check any user of BrowserWindow.focus() to use app.focus instead, otherwise we cause regressions, no?

Do you mean other consumers of vscode build flavor ?

There is an explicit mention about it in the internal release notes.

[bpasero]

@deepak1556

Do you mean other consumers of vscode build flavor ?

I was not clear: my understanding is that we will drop this patch we have for the old behaviour since we can now use app.focus({ steal: true });, but if we do that, any place where we call BrowserWindow.focus() today, we need to also call app.focus({ steal: true }); right?

[deepak1556]

Is there any case other than the second instance launch where we rely on Browserwindow.focus to bring the app into focus ? This hack is only needed for such use case.

If the app is already in focus and we just want to switch focus between windows then BrowserWIndow.focus is sufficient.

[bpasero]

@deepak1556 well, we would have to check case by case, but for example hitting a debug breakpoint I think also triggers this in debug land?

[deepak1556]

We always let the app take focus when launching second instance, previously it was only covering a single case.

[bpasero]

Sorry, what does that mean?

[deepak1556]

without this fix the behavior is,

code-insiders abc/ -> will get focus
code-insiders . -> will not get focus

which is a regression from how it behaved with E7

[bpasero]

@deepak1556 I think to make progress on this PR you will have to go through all the calls to window focus and then come up with a proposal if this new flag is needed or not. This includes:

• any direct calls to BrowserWindow#focus()
• any calls to electron-main/window.ts#focus()
• any calls to IElectronService#focusWindow()

Does that make sense?

[bpasero]

Continues in #102011