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

fix: invoke app.dock.{hide|show} instead of duplicating logic #37599

Merged
merged 1 commit into from
Mar 20, 2023

Conversation

codebytere
Copy link
Member

@codebytere codebytere commented Mar 16, 2023

Description of Change

Closes #37487.

Fixes an issue with potential dock icon duplication on macOS by calling into our existing app.dock.show and app.dock.hide implementations, which are also subject to the duplication bugs but include workarounds to prevent them from happening.

Tested with https://gist.github.com/PsychoLlama/fcb2790372b6e4957beac9be7e543d3d to verify no duplicates occur.

Checklist

Release Notes

Notes: Fixes an issue with potential dock icon duplication on macOS.

@codebytere codebytere added semver/patch backwards-compatible bug fixes target/22-x-y PR should also be added to the "22-x-y" branch. target/23-x-y PR should also be added to the "23-x-y" branch. target/24-x-y PR should also be added to the "24-x-y" branch. labels Mar 16, 2023
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Mar 16, 2023
if (visibleOnFullScreen) {
[window_ setCanHide:NO];
TransformProcessType(&psn, kProcessTransformToUIElementApplication);
Browser::Get()->DockHide();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 on reusing the workarounds found in DockHide() / DockShow() here.

This diff has a subtle change: instead of calling NSWindow setCanHide:NO] for just window_, Browser::DockHide() calls it for everything in WindowList::GetWindows(). Is that OK / intentional here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'd say yes - imo the previous version (only doing it on window_) was incorrect

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Mar 17, 2023
@jkleinsc jkleinsc merged commit eb613ef into main Mar 20, 2023
@jkleinsc jkleinsc deleted the visible-on-all-workspaces-dock-struggles branch March 20, 2023 14:30
@release-clerk
Copy link

release-clerk bot commented Mar 20, 2023

Release Notes Persisted

Fixes an issue with potential dock icon duplication on macOS.

@trop
Copy link
Contributor

trop bot commented Mar 20, 2023

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

@trop trop bot added in-flight/22-x-y and removed target/22-x-y PR should also be added to the "22-x-y" branch. labels Mar 20, 2023
@trop
Copy link
Contributor

trop bot commented Mar 20, 2023

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

@trop
Copy link
Contributor

trop bot commented Mar 20, 2023

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

@trop trop bot added in-flight/23-x-y merged/24-x-y PR was merged to the "24-x-y" branch and removed target/23-x-y PR should also be added to the "23-x-y" branch. target/24-x-y PR should also be added to the "24-x-y" branch. in-flight/24-x-y labels Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged/22-x-y PR was merged to the "22-x-y" branch. merged/23-x-y PR was merged to the "23-x-y" branch. merged/24-x-y PR was merged to the "24-x-y" branch semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Restoring dock icon after creating multiple overlay windows creates duplicates
4 participants