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.

Sign up for GitHub

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: do not use CONTEXT_MENU flag for tray menu (reland) #24097

Merged
merged 2 commits into from Jun 18, 2020
Merged

fix: do not use CONTEXT_MENU flag for tray menu (reland) #24097

merged 2 commits into from Jun 18, 2020

Conversation

zcbenz
Copy link
Member

@zcbenz zcbenz commented Jun 12, 2020 •
edited

Close #11587, close #24049.

Relands #23843 with a fix for #24049.

By adding the WS_EX_TOOLWINDOW style, the menu window would no longer show in taskbar.

I'll file a bug and submit the patch to Chromium next week, I haven't figured out how to make Chrome browser show a status icon.

Notes: Fix tray menu showing in taskbar on Windows.

@zcbenz zcbenz requested a review from a team as a code owner June 12, 2020 12:42
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Jun 12, 2020
@zcbenz zcbenz changed the title fix: do not use CONTEXT_MENU flag for tray menu fix: do not use CONTEXT_MENU flag for tray menu (reland) Jun 12, 2020
MarshallOfSound
MarshallOfSound reviewed Jun 12, 2020
View reviewed changes
patches/chromium/remove_menu_window_task_item.patch Outdated
break;
case Widget::InitParams::TYPE_MENU:
*style |= WS_POPUP;
+ *ex_style |= WS_EX_TOOLWINDOW;
Copy link
Member

Choose a reason for hiding this comment

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

Is this safe, this looks like a well traversed function

Copy link
Member Author

Choose a reason for hiding this comment

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

This function is executed when calculating the styles of windows, so it is the correct place to add a style for a type of window.

My only concern is whether this would cause other side effects (I believe it won't according to the docs of this style), we will know once the patch gets reviewed in Chromium.

@deepak1556
Copy link
Member

I'll file a bug and submit the patch to Chromium next week, I haven't figured out how to make Chrome browser show a status icon.

@zcbenz as long as the exercised code path is same, you can create an issue with content_shell as an example. It is an accepted bug filing method when external developers can't make an example case with chrome.

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Jun 13, 2020
@zcbenz zcbenz force-pushed the fix-tray-accessebility branch 2 times, most recently from ddf24c1 to 808ee77 Compare June 15, 2020 05:16
@zcbenz
Copy link
Member Author

zcbenz commented Jun 15, 2020

Patch has been submitted to upstream: https://chromium-review.googlesource.com/c/chromium/src/+/2245941.

@zcbenz
Copy link
Member Author

zcbenz commented Jun 16, 2020

The added patch has been approved in upstream.

@zcbenz zcbenz requested a review from a team June 16, 2020 23:55
@deepak1556
Copy link
Member

@zcbenz can you update the patch to match upstream CL ?

@deepak1556
Copy link
Member

Failing test is unrelated, merging.

@deepak1556 deepak1556 merged commit 2376c45 into master Jun 18, 2020
@release-clerk
Copy link

release-clerk bot commented Jun 18, 2020

Release Notes Persisted

Fix tray menu showing in taskbar on Windows.

@deepak1556 deepak1556 deleted the fix-tray-accessebility branch June 18, 2020 04:21
@trop
Copy link
Contributor

trop bot commented Jun 18, 2020

I was unable to backport this PR to "8-x-y" cleanly;
you will need to perform this backport manually.

This was referenced Jun 18, 2020
Merged
Merged
@trop
Copy link
Contributor

trop bot commented Jun 18, 2020

I was unable to backport this PR to "7-3-x" cleanly;
you will need to perform this backport manually.

@trop
Copy link
Contributor

trop bot commented Jun 18, 2020

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

@trop trop bot removed the target/10-x-y label Jun 18, 2020
@trop
Copy link
Contributor

trop bot commented Jun 18, 2020

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

@zcbenz zcbenz mentioned this pull request Jun 18, 2020
Merged
@trop
Copy link
Contributor

trop bot commented Jun 18, 2020

@zcbenz has manually backported this PR to "8-x-y", please check out #24194

@zcbenz zcbenz mentioned this pull request Jun 18, 2020
Closed
@trop
Copy link
Contributor

trop bot commented Jun 18, 2020

@zcbenz has manually backported this PR to "7-3-x", please check out #24195

sentialx pushed a commit to sentialx/electron that referenced this pull request Jul 30, 2020
@zcbenz @sentialx
fix: do not use CONTEXT_MENU flag for tray menu (reland) (electron#24097
06678af 
)

* fix: do not use CONTEXT_MENU flag for tray menu

* fix: menu window should not appear in taskbar
sentialx pushed a commit to sentialx/electron that referenced this pull request Apr 8, 2021
@zcbenz @sentialx
fix: do not use CONTEXT_MENU flag for tray menu (reland) (electron#24097
25054ea 
)

* fix: do not use CONTEXT_MENU flag for tray menu

* fix: menu window should not appear in taskbar
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deepak1556 deepak1556 deepak1556 approved these changes

@MarshallOfSound MarshallOfSound MarshallOfSound approved these changes

@ckerr ckerr Awaiting requested review from ckerr

@codebytere codebytere Awaiting requested review from codebytere

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@zcbenz @deepak1556 @MarshallOfSound @codebytere