Repository navigation
fix: honor hasShadow: false and focusable for windows in app shims - #55632
Conversation
…55606) * fix: honor hasShadow: false for windows in app shims The option only reached a stored value, so these windows kept the system shadow. * docs: drop the app shim hasShadow note The constructor option now behaves as documented, so it needs no callout. * fix: honor focusable for windows in app shims focusable: false and win.setFocusable() had no effect on these windows, and win.isFocusable() always returned false. Co-authored-by: Felix Rieseberg <f@anthropic.com>
| expect(w.isVisible()).toBe(false); | ||
| }); | ||
|
|
||
| it('honors focusable for a window in a shim', async () => { |
There was a problem hiding this comment.
🔴 The new focus test will be flaky in CI. It needs OS-level focus but runs while other spec processes have windows on screen. spec/CLAUDE.md says such tests must be tagged serial. The test 'honors focusable for a window in a shim' awaits the window's 'focus' event and asserts isFocused(). Neither it nor the enclosing ifdescribe('app shims') suite at spec/api-app-shim.spec.ts:59 has { tags: ['serial'] }. Fix: change the line to it('honors focusable for a window in a shim', { tags: ['serial'] }, async () => {, as spec/api-web-contents-view.spec.ts:187 does.
Why this was flagged
The runner starts several Electron processes and gives each one spec file at a time (spec/CLAUDE.md:29-36). The new test calls w.focus() and then waits on once(w, 'focus') at spec/api-app-shim.spec.ts:175-179. It also asserts isFocused() is true at line 180. A window from another spec process can take key status at any moment, so the 'focus' event may never fire, or isFocused() may already be false again. The result is an intermittent timeout or assertion failure in CI. The earlier tests in this file do not depend on focus, and the suite at spec/api-app-shim.spec.ts:59 has no serial tag. Nothing else keeps this test away from the parallel phase.
Verification: The new test at spec/api-app-shim.spec.ts:166 is it('honors focusable for a window in a shim', async () => { with no tags. The test depends on OS-level focus. If another process's window takes activation or key status, the 'focus' event may never fire, which ends in a timeout, or isFocused() may already be false at line 180.
| if (!options.ValueOrDefault(options::kHasShadow, true)) | ||
| params.shadow_type = views::Widget::InitParams::ShadowType::kNone; |
There was a problem hiding this comment.
🟡 (optional) Apps that create a shim window with hasShadow: false and later call setHasShadow(true) now get no shadow, though hasShadow() returns true. Line 238 fixes the shadow when the window is created. SetHasShadow at native_window_mac_remote.mm:744-746 still only stores has_shadow_, so nothing can turn the shadow back on. On the base branch these windows always had a shadow, so that call matched what was on screen. Fix: send setHasShadow at runtime to the shim's NSWindow, for example with a new AppShim mojom call next to SetWindowButtonVisibility. Otherwise keep HasShadow() matching the creation-time state. Runtime toggling is pre-existing.
Why this was flagged
An app creates new BrowserWindow({ appShim: shim, hasShadow: false }) and later calls win.setHasShadow(true), which reaches NativeWindowMacRemote::SetHasShadow through BaseWindow. The new code at native_window_mac_remote.mm:237-238 sets params.shadow_type = ShadowType::kNone at creation. SetHasShadow (native_window_mac_remote.mm:744-746) only assigns has_shadow_ and never reaches the widget or the shim. The AppShim interface in shell/common/app_shim.mojom:113-161 has no shadow call either. So the window stays without a shadow, while HasShadow() (line 748-750) returns true. On the base branch the shadow option was ignored, so a window made with hasShadow: false kept its system shadow, and after setHasShadow(true) the reported value matched the screen. NativeWindow::InitFromOptions (native_window.cc:244) only syncs has_shadow_ at creation, so it does not help here. The in-process NativeWindowMac::SetHasShadow applies the change live.
Verification: nit. Trigger: a shim window is created with hasShadow: false and the app later calls setHasShadow(true). SetHasShadow at native_window_mac_remote.mm:744-746 is still just has_shadow_ = has_shadow;, so nothing can turn the shadow back on. On the base branch the shim window always had a shadow, so this particular order of calls happened to match what was on screen.
|
Release Notes Persisted
|
Backport of #55606
See that PR for details.
Notes: Fixed
hasShadow: falseandfocusablebeing ignored in macOS app shims.