fix: fix browser focus issue on MacOS #24892

lmiller1990 · 2022-11-30T02:21:35Z

Closes App window does not respond to mouse events in window focus race condition #21743

User facing changelog

Fix a MacOS specific bug where the browser would not be correctly focused when launched.

Additional details

For reasons unknown, if you quickly jiggle your mouse back and forth when launching Chrome or Firefox browsers on MacOS, the browser would not be correctly focused. You can see this below in the first video - I jiggle my mouse, and then I need to click the search bar a bunch of times to get the browser to properly focus.

Note this also happens in Cypress 9 on the latest MacOS version. I reproduced it here: #21743 (comment)

There was a work around which was to switch to any other window, then switch back, and it would work as expected. I don't know why this fixes the issue, but i does. I was able to do the same thing programmatically, but simply showing and immediately hiding the electron app in code.

Bug

bug.mov

Fix

fix.mov

Steps to test

Note: You need to use MacOS to test this.

Reproduce the issue (explained above).
Get this branch.
Try to reproduce it - you shouldn't be able to.

I see no practical way to write an automated test around this behavior. 🤔

How has the user experience changed?

The launched browser is now correctly focused.

PR Tasks

[na] Have tests been added/updated?
Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
[na] Has a PR for user-facing changes been opened in cypress-documentation?
[na] Have API changes been updated in the type definitions?

cypress-bot · 2022-11-30T02:21:37Z

Thanks for taking the time to open a PR!

Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

CLAassistant · 2022-11-30T02:21:41Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ lmiller1990
❌ =

= seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

lmiller1990 · 2022-11-30T02:23:51Z

packages/server/lib/browsers/index.ts

@@ -130,15 +131,15 @@ export = {
    return this.getBrowserInstance()
  },

-  async open (browser: Browser, options: BrowserLaunchOpts, automation: Automation, ctx): Promise<BrowserInstance | null> {
+  async open (browser: Browser, options: BrowserLaunchOpts, automation: Automation, ctx?: DataContext): Promise<BrowserInstance | null> {


This is probably NOT allowed to be nullable, but I'm not really sure, so I just made it nullable for now (previously it was any, so at least this is more tightly typed).

…us-issue

lmiller1990 · 2022-11-30T03:19:21Z

packages/server/lib/browsers/index.ts

+    // work around for a focus MacOS specific issue where the
+    // browser is not correctly focused when launched.
+    // https://github.com/cypress-io/cypress/issues/21743
+    ctx?.actions.browser.resetFocusIfMacOS()


This is the """fix""".

cypress · 2022-11-30T03:20:19Z

Test summary

46548 • 1 • 4326 • 0 • 22

Run details


Project	cypress
Status	Failed
Commit	`08bf460`
Started	Nov 30, 2022 5:34 AM
Ended	Nov 30, 2022 5:52 AM
Duration	17:37 💡
OS	Linux Debian -
Browser	Multiple

View run in Cypress Dashboard ➡️

Failures

	cypress/e2e/e2e/origin/navigation.cy.ts		1
1	... > establishes foobar spec bridge

Flakiness

	commands/xhr.cy.js	1
1	src/cy/commands/xhr > abort > aborts xhrs currently in flight
	e2e/origin/cookie_behavior.cy.ts	4
1	... > same site / cross origin > XMLHttpRequest > sets cookie on same-site request if withCredentials is true, and attaches to same-site request if withCredentials is true
2	... > same site / cross origin > fetch > sets same-site cookies if "include" credentials option is specified from request, but does not attach same-site cookies to request by default (same-origin)
3	... > same site / cross origin > XMLHttpRequest > sets cookie on same-site request if withCredentials is true, and attaches to same-site request if withCredentials is true
4	... > same site / cross origin > fetch > sets same-site cookies if "include" credentials option is specified from request, but does not attach same-site cookies to request by default (same-origin)
This comment includes only the first 5 flaky tests. See all 22 flaky tests in the Cypress Dashboard.

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

mike-plummer

Works well for me., just one small nitpick Nice job tracking down something that works for this, and thanks for adding the additional typings throughout 💯

mike-plummer · 2022-11-30T15:11:05Z

packages/data-context/src/actions/BrowserActions.ts

@@ -44,6 +44,14 @@ export class BrowserActions {
    await this.browserApi.focusActiveBrowserWindow()
  }

+  resetFocusIfMacOS () {


/nit: Personal preference, I would put the "this is a workaround" and GH issue link on the function as JSDoc

astone123 · 2022-11-30T15:47:07Z

I wasn't able to reproduce the issue on this branch 👍🏻

One thing that I'm seeing now though, is that once I launch Chrome from the launchpad, I can't find the launchpad window anymore... it is nowhere to be found. I see the Cy icon in my dock, but when I ask it to show me where the launchpad window is, it says that there aren't any windows.

@mike-plummer are you also seeing this? https://www.loom.com/share/f411c08995134eeea27e6e1ba7ed0dec

I think I see the same issue happen in the videos in the PR description. If you look closely you can see the launchpad window disappear once Chrome launches in the fixed video

mike-plummer · 2022-11-30T17:47:50Z

I wasn't able to reproduce the issue on this branch 👍🏻

One thing that I'm seeing now though, is that once I launch Chrome from the launchpad, I can't find the launchpad window anymore... it is nowhere to be found. I see the Cy icon in my dock, but when I ask it to show me where the launchpad window is, it says that there aren't any windows.

@mike-plummer are you also seeing this? loom.com/share/f411c08995134eeea27e6e1ba7ed0dec

I think I see the same issue happen in the videos in the PR description. If you look closely you can see the launchpad window disappear once Chrome launches in the fixed video

@astone123 Confirmed, I see this too. Didn't notice it in my earlier testing. If I update BrowserActions to do a show/hide/show in the resetFocusIfMacOS then everything seems to work as expected. Do you see the same?

resetFocusIfMacOS () {
    if (this.ctx.util.isMac) {
      this.ctx.actions.electron.showBrowserWindow()
      this.ctx.actions.electron.hideBrowserWindow()
      this.ctx.actions.electron.showBrowserWindow()
      this.focusActiveBrowserWindow().catch(this.ctx.onError)
    }
  }

astone123 · 2022-11-30T19:04:43Z

@mike-plummer can confirm that this works, only thing is that the Launchpad window flashes over top of the browser for a split second after it launches... I wonder if that's something that we're willing to live with to fix this issue on MacOS? 🤔

lmiller1990 · 2022-11-30T20:41:11Z

@mike-plummer can confirm that this works, only thing is that the Launchpad window flashes over top of the browser for a split second after it launches... I wonder if that's something that we're willing to live with to fix this issue on MacOS? 🤔

Yeah I had considered this but I don't think it feels great... I didn't realize hide() prevents the electron app from refocusing. Hm.

lmiller1990 · 2022-11-30T22:34:42Z

This is actually a bug in macos - I don't think we can "properly" fix it here, without the "flicker".

I logged an issue in electron: electron/electron#36506

lmiller1990 · 2022-12-01T03:15:35Z

I am going to close this for now - we can revisit once we decide what we'd like to do, but none of the proposed solutions seems appropriate.

= and others added 4 commits November 29, 2022 16:10

chore: some style improvements

3939d11

chore: hacky fix

a1fe8a6

conditionally refocus browser on macos

8ab812d

add comment

2c62fd5

lmiller1990 commented Nov 30, 2022

View reviewed changes

lmiller1990 added 3 commits November 30, 2022 12:24

revert code change run ci

986b1a8

Merge remote-tracking branch 'origin/develop' into lmiller/fixing-foc…

67e55ad

…us-issue

fix types

8cb462a

lmiller1990 marked this pull request as ready for review November 30, 2022 03:16

lmiller1990 requested review from a team and brian-mann November 30, 2022 03:18

lmiller1990 commented Nov 30, 2022

View reviewed changes

fix typescript

08bf460

mike-plummer approved these changes Nov 30, 2022

View reviewed changes

lmiller1990 closed this Dec 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: fix browser focus issue on MacOS #24892

fix: fix browser focus issue on MacOS #24892

lmiller1990 commented Nov 30, 2022 •

edited

Loading

cypress-bot bot commented Nov 30, 2022

CLAassistant commented Nov 30, 2022

lmiller1990 Nov 30, 2022

lmiller1990 Nov 30, 2022

cypress bot commented Nov 30, 2022 •

edited

Loading

mike-plummer left a comment

mike-plummer Nov 30, 2022

astone123 commented Nov 30, 2022 •

edited

Loading

mike-plummer commented Nov 30, 2022 •

edited

Loading

astone123 commented Nov 30, 2022

lmiller1990 commented Nov 30, 2022

lmiller1990 commented Nov 30, 2022

lmiller1990 commented Dec 1, 2022

fix: fix browser focus issue on MacOS #24892

fix: fix browser focus issue on MacOS #24892

Conversation

lmiller1990 commented Nov 30, 2022 • edited Loading

User facing changelog

Additional details

Bug

Fix

Steps to test

How has the user experience changed?

PR Tasks

cypress-bot bot commented Nov 30, 2022

CLAassistant commented Nov 30, 2022

lmiller1990 Nov 30, 2022

Choose a reason for hiding this comment

lmiller1990 Nov 30, 2022

Choose a reason for hiding this comment

cypress bot commented Nov 30, 2022 • edited Loading

Test summary

Run details

Failures

Flakiness

mike-plummer left a comment

Choose a reason for hiding this comment

mike-plummer Nov 30, 2022

Choose a reason for hiding this comment

astone123 commented Nov 30, 2022 • edited Loading

mike-plummer commented Nov 30, 2022 • edited Loading

astone123 commented Nov 30, 2022

lmiller1990 commented Nov 30, 2022

lmiller1990 commented Nov 30, 2022

lmiller1990 commented Dec 1, 2022

lmiller1990 commented Nov 30, 2022 •

edited

Loading

cypress bot commented Nov 30, 2022 •

edited

Loading

astone123 commented Nov 30, 2022 •

edited

Loading

mike-plummer commented Nov 30, 2022 •

edited

Loading