Repository navigation
feat: add Menu.setHighlightColors() for Windows - #55494
MarshallOfSound merged 4 commits into
Conversation
Lets apps set the background and text color of the hovered or keyboard-focused item in Views menus on Windows. The colors are skipped in high contrast and forced colors modes. Closes #54953 Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Co-authored-by: Ashley Hendrata <ashley.m.hendrata@gmail.com>
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Co-authored-by: Ashley Hendrata <ashley.m.hendrata@gmail.com>
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Co-authored-by: Ashley Hendrata <ashley.m.hendrata@gmail.com>
Light and dark colors are now set independently, with no fallback between modes, and validation errors name the mode (for example 'dark.background'). Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Co-authored-by: Ashley Hendrata <ashley.m.hendrata@gmail.com>
There was a problem hiding this comment.
I reviewed this backport and didn't find any bugs. It adds a new public API (Menu.setHighlightColors) to a release branch and hooks into the browser-wide ColorProvider pipeline, so a maintainer should still look at it. I reviewed it from a standalone clone, without the Chromium sources.
What was reviewed:
- Argument parsing in
Menu::SetHighlightColors: a non-object options value, a non-object or nulllight/dark, and non-string or invalid colors all throw before any state is stored, andnull/undefinedresets the colors to the defaults. AddMenuHighlightColorMixerskips high-contrast and forced-colors keys and picks the light or dark colors fromkey.color_mode. It is only registered on Windows, afterAddChromeColorMixers. The binding stays reachable on macOS and Linux, where it does nothing, which matches the docs.- The new files are listed in
filenames.gni. The docs have an API history block, and the vitest specs useexpectmatchers.
Extended reasoning...
The change adds a static Menu binding, a new shell/browser/ui/menu_highlight_colors.{h,cc} pair holding process-global colors, a Windows-only ColorProvider initializer in electron_browser_main_parts.cc, docs and specs. It touches no IPC, renderer-reachable, permission or other security-sensitive surface; the input is main-process JS only. I couldn't find a defect in the diff. I deferred because this is a new public feature going to a release branch, which usually needs maintainer sign-off. Also, from this checkout I can't check how ResetColorProviderCache and the mixer ordering behave upstream, or confirm that they actually affect the Views menu colors.
|
Release Notes Persisted
|
Backport of #55074
See that PR for details.
Notes: Added
Menu.setHighlightColors()to set menu highlight colors on Windows.