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.

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

fix: make BrowserWindow#isFocused() return false when blur() is called on macOS #33734

Merged
merged 1 commit into from May 3, 2022
Merged

fix: make BrowserWindow#isFocused() return false when blur() is called on macOS #33734

merged 1 commit into from May 3, 2022

Conversation

RaisinTen
Copy link
Member

@RaisinTen RaisinTen commented Apr 12, 2022 •
edited

Description of Change

The isFocused() method on macOS works by checking if the selected
BrowserWindow is a key window. Unfortunately, this didn't work well
with blur() because it wasn't calling any macOS APIs that would change
the key status of the window. Hence, this changes the implementation of
blur() to call orderOut first, which removes the key
status of the window. Then when the orderBack function is called, it
moves the window to the back of its level in the screen list, without
changing the key window.

Fixes: #33732
Signed-off-by: Darshan Sen raisinten@gmail.com

Checklist

Release Notes

Notes: Fixed an issue where BrowserWindow#isFocused() was returning false when blur() was called on macOS.

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Apr 12, 2022
@RaisinTen RaisinTen mentioned this pull request Apr 12, 2022
Closed
3 tasks
@RaisinTen RaisinTen force-pushed the fix/make-BrowserWindow-isFocused-return-false-when-blur-is-called branch 3 times, most recently from a972eaf to c9e32e0 Compare April 12, 2022 13:37
@dsanders11
Copy link
Member

@RaisinTen, looks like the fixed test fails on Windows CI:

not ok 264 BrowserWindow module focus and visibility BrowserWindow.blur() removes focus from window
  AssertionError: expected true to equal false
      at Context.<anonymous> (electron\spec-main\api-browser-window-spec.ts:773:34)

So this may be broken on Windows as well and the faulty test was hiding it.

@RaisinTen RaisinTen force-pushed the fix/make-BrowserWindow-isFocused-return-false-when-blur-is-called branch from c9e32e0 to b5ab85e Compare April 13, 2022 05:56
@RaisinTen
Copy link
Member Author

RaisinTen commented Apr 13, 2022 •
edited

@dsanders11 thanks for noticing, updated the test to skip on Windows for now and added a reference to the open issue #20464.

@RaisinTen RaisinTen mentioned this pull request Apr 13, 2022
Open
3 tasks
@RaisinTen RaisinTen changed the title fix: make BrowserWindow#isFocused() return false when blur() is called fix: make BrowserWindow#isFocused() return false when blur() is called on macOS Apr 13, 2022
@RaisinTen RaisinTen force-pushed the fix/make-BrowserWindow-isFocused-return-false-when-blur-is-called branch 7 times, most recently from 5dd5164 to 0b1bc02 Compare April 13, 2022 11:49
@zcbenz zcbenz added semver/patch backwards-compatible bug fixes target/17-x-y labels Apr 14, 2022
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Apr 14, 2022
@RaisinTen RaisinTen force-pushed the fix/make-BrowserWindow-isFocused-return-false-when-blur-is-called branch 4 times, most recently from 2dd48a1 to 4d510ba Compare April 14, 2022 10:36
@RaisinTen
Copy link
Member Author

RaisinTen commented Apr 14, 2022 •
edited

Why is the Setup Goma step failing in the macOS builds?
Ah, depends on #33785, since CircleCI removed python2, rebasing.

@RaisinTen RaisinTen force-pushed the fix/make-BrowserWindow-isFocused-return-false-when-blur-is-called branch from 4d510ba to 1de51f5 Compare April 14, 2022 10:56
@RaisinTen
Copy link
Member Author

There was at least one passing macOS CI run for this change - https://app.circleci.com/pipelines/github/electron/electron/51968/workflows/8c551ade-118b-4fc7-9eff-fc55574bddff and the current failure is unrelated to this change. I think rerunning the macOS builds a couple of times will fix it. Could someone please do that (I don't have the permissions)?

Also, this is ready for review. :)

@RaisinTen RaisinTen force-pushed the fix/make-BrowserWindow-isFocused-return-false-when-blur-is-called branch from d9f60ff to 6294a79 Compare May 2, 2022 10:40
@RaisinTen
Copy link
Member Author

@zcbenz done!

@zcbenz
Copy link
Member

zcbenz commented May 3, 2022 •
edited

So it seems that the test only fails on forked PRs #34020, latest main branch has just disabled the test, rebasing this branch again should have the CI passing.

@RaisinTen
fix: make BrowserWindow#isFocused() return false when blur() is calle…
42b1876 
…d on macOS

The isFocused() method on macOS works by checking if the selected
BrowserWindow is a key window. Unfortunately, this didn't work well
with blur() because it wasn't calling any macOS APIs that would change
the key status of the window. Hence, this changes the implementation of
blur() to call orderOut first, which removes the key
status of the window. Then when the orderBack function is called, it
moves the window to the back of its level in the screen list, without
changing the key window.

Fixes: electron#33732
Signed-off-by: Darshan Sen <raisinten@gmail.com>
@RaisinTen RaisinTen force-pushed the fix/make-BrowserWindow-isFocused-return-false-when-blur-is-called branch from 6294a79 to 42b1876 Compare May 3, 2022 05:23
@RaisinTen
Copy link
Member Author

@zcbenz rebased, PTAL!

@electron electron deleted a comment May 3, 2022
@zcbenz zcbenz merged commit f887000 into electron:main May 3, 2022
@release-clerk
Copy link

release-clerk bot commented May 3, 2022

Release Notes Persisted

Fixed an issue where BrowserWindow#isFocused() was returning false when blur() was called on macOS.

This was referenced May 3, 2022
Merged
Merged
Merged
@trop
Copy link
Contributor

trop bot commented May 3, 2022

I have automatically backported this PR to "17-x-y", please check out #34029

@trop
Copy link
Contributor

trop bot commented May 3, 2022

I have automatically backported this PR to "18-x-y", please check out #34030

@trop
Copy link
Contributor

trop bot commented May 3, 2022

I have automatically backported this PR to "19-x-y", please check out #34031

@RaisinTen RaisinTen deleted the fix/make-BrowserWindow-isFocused-return-false-when-blur-is-called branch May 3, 2022 07:40
@commit-lint commit-lint bot mentioned this pull request Jul 5, 2022
Open
khalwa pushed a commit to solarwindscloud/electron that referenced this pull request Feb 22, 2023
@RaisinTen @khalwa
fix: make BrowserWindow#isFocused() return false when blur() is calle…
55f2f5c 
…d on macOS (electron#33734)

The isFocused() method on macOS works by checking if the selected
BrowserWindow is a key window. Unfortunately, this didn't work well
with blur() because it wasn't calling any macOS APIs that would change
the key status of the window. Hence, this changes the implementation of
blur() to call orderOut first, which removes the key
status of the window. Then when the orderBack function is called, it
moves the window to the back of its level in the screen list, without
changing the key window.

Fixes: electron#33732
Signed-off-by: Darshan Sen <raisinten@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zcbenz zcbenz zcbenz approved these changes

Assignees
No one assigned
Labels
semver/patch backwards-compatible bug fixes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: BrowserWindow: isFocused() returns true even after calling blur() on macOS
3 participants
@RaisinTen @dsanders11 @zcbenz