refactor: simplify content_view_ hit-test transparency on macOS by MarshallOfSound · Pull Request #51617 · electron/electron

MarshallOfSound · 2026-05-13T22:30:24Z

Description of Change

Replaces the fix for #51576 that landed in #51586 with a more scoped one. Same crash fix, much smaller surface.

Background

For BrowserWindow on macOS, the WebContentsView is added as a sibling of content_view_ at lower z-order so that user-added child views paint above web content. But content_view_ covers the whole window, so the default views hit-test always stops at content_view_ before reaching the WebContentsView. After Chromium CL:6574464 (M139) changed BridgedContentView::hitTest: to use GetHitTestResult(), any non-NativeViewHost hit result causes BridgedContentView to swallow the event — so all loadURL page interaction broke. #50330 worked around this by making ContentViewTargeterDelegate::TargetForRect return nullptr when no descendant of content_view_ covered the hit rect, so the result would propagate up as null → kOther → [super hitTest:] finds RenderWidgetHostViewCocoa.

Returning nullptr from ViewTargeterDelegate::TargetForRect violates its contract — for any in-bounds point it must resolve to a descendant or the root, never null (ui/views/view_targeter_delegate.h, ui/views/view.h). RootView::UpdateCursor and RootView::HandleMouseEnteredOrMoved both call GetEventHandlerForPoint() and dereference the result without a null check (ui/views/widget/root_view.cc:851-853, :864-871). When a right-click in a -webkit-app-region: drag region temporarily disables draggable regions (electron_ns_window.mm), the HTCAPTION early-exit no longer fires, the views path runs, and the nullptr SEGVs (#51576).

Why #51586 wasn't the right shape

#51586 fixed the crash, but did so by moving the transparency logic to a new targeter on RootViewMac that re-implements views::ViewTargeterDelegate::TargetForRect's child-walk loop so it can skip content_view_ and pick a sibling. That has several problems:

It duplicates ~25 lines of upstream logic that is exactly the part of views most likely to change (mirroring, layer transforms, visibility semantics, rounding mode). Any upstream change to that loop now needs to be ported by hand or the two will silently diverge.
It already has one divergence baked in. It uses View::ConvertRectToTarget(root, child, rect) (the gfx::Rect overload, which rounds inward via ToEnclosedRectIgnoringError), while the upstream loop converts to a gfx::RectF and rounds outward via ToEnclosingRect. For a 1×1 hit rect on a fractional-DPI display, those can disagree.
It only special-cases point-based targeting (UsePointBasedTargeting(rect)), so rect-based/touch targeting still falls through to the default — which returns content_view_ → kRootView — i.e. the original loadURL-interaction bug is still live for the non-point path. chore: remove macos hittest workaround patch #50330 covered both.
It leaks a macOS-only quirk into the platform-neutral surface. native_window.h (the cross-platform base) grew a content_view_hit_test_transparent_ flag + getter + setter that only native_window_mac.mm reads, and BrowserWindow's constructor (also platform-neutral) grew a set_content_view_hit_test_transparent(true) that does nothing on Windows/Linux.
Latent footgun: the return skipped_content_view ? skipped_content_view : root; fallback returns content_view_ → kRootView → swallowed — the exact behavior chore: remove macos hittest workaround patch #50330 was fixing. It's unreachable today only because WebContentsView always covers the window; if the view structure changes, this re-introduces the bug.
It conflates two responsibilities. "I am a transparent overlay" is a property of content_view_, not something its parent's targeter should have special knowledge of.

What this PR does instead

The views hit-test machinery already has a hook for "is this view opaque to events at this rect?" — ViewTargeterDelegate::DoesIntersectRect, which View::HitTestRect consults. The parent's default TargetForRect loop already checks child->HitTestRect() and skips children that say no (view_targeter_delegate.cc:61-63). So we can fix #50330 in place: keep the targeter on content_view_, keep the same install site in SetContentView, and override DoesIntersectRect to return false when no visible, processable child of content_view_ covers the rect. The parent's default loop then falls through to the WebContentsView sibling and resolves to its NativeViewHost — kSubView → routes to RenderWidgetHostViewCocoa — without ever returning nullptr from TargetForRect or duplicating the upstream walk.

This reverts the NativeWindow flag, the BrowserWindow constructor change, and RootViewMacTargeterDelegate. The net change vs the buggy #50330 state is a single method-body swap inside native_window_mac.mm.

Verification

Verified on macOS 26.4 (25E246), arm64, driving input with real OS-level CGEventPost events so the NSWindow → BridgedContentView::hitTest: → RootView::UpdateCursor → targeter path is actually exercised:

loadURL interaction (the chore: remove macos hittest workaround patch #50330 regression check): button click, input focus + typing, hover → pointingHand cursor, link click — all work.
Right-click in -webkit-app-region: drag (the Crash (SIGSEGV) on right-click in -webkit-app-region: drag on macOS 26 (Tahoe) #51576 crash check): right-click bar, mouse-move over bar, drag (window moved), Ctrl+click, right/left-click content — no SIGSEGV on any step.
Child WebContentsView over web content: clicking a WebContentsView overlaid on content_view_ and clicking the background page both route correctly; DoesIntersectRect returns true over the overlay child and false outside it.
api-browser-window-spec.ts -g "BrowserView|contentView|setContentView": 4/4. api-web-contents-view-spec.ts: 29/29. drag-region-spec.ts: 6/6 (8 skip, non-macOS).

Backports

41-x-y is not affected — chore: remove macos hittest workaround patch #50330 was never backported there; the original Chromium revert patch is still in place. (fix: don't return a nullptr from TargetForRect (#51586) #51611 was correctly closed unmerged.)
42-x-y / 43-x-y: the open fix: don't return a nullptr from TargetForRect #51586 backports (fix: don't return a nullptr from TargetForRect #51605, fix: don't return a nullptr from TargetForRect #51606) should be replaced with backports of this PR once it lands.

Checklist

PR description included
npm test passes
PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

Notes: none

Replace the RootViewMac-level TargetForRect override added in #51586 with a DoesIntersectRect override on content_view_'s own targeter, restoring the structure of #50330 but fixing its actual defect. The original bug (#51576) was that ContentViewTargeterDelegate::TargetForRect returned nullptr when no descendant covered the hit rect, violating the ViewTargeterDelegate contract. RootView::UpdateCursor and RootView::HandleMouseEnteredOrMoved dereference GetEventHandlerForPoint() without null checks, so the nullptr crashed with SIGSEGV when a right-click in a -webkit-app-region: drag region disabled the HTCAPTION early-exit. #51586 fixed the crash by moving the transparency logic to RootViewMac and re-implementing the parent targeter's child-walk loop so it could skip content_view_ and return a sibling instead. That re-implementation duplicates ~25 lines of upstream logic with a subtle rounding divergence (ToEnclosedRectIgnoringError vs ToEnclosingRect), and threads a new content_view_hit_test_transparent_ flag through the platform-neutral NativeWindow/BrowserWindow headers. The same fall-through can be achieved with the existing hit-test hook: override DoesIntersectRect on content_view_ to return false when no visible, processable child intersects the rect. The parent's default TargetForRect loop already skips children whose HitTestRect is false, so hit-testing naturally continues to the WebContentsView sibling and resolves to its NativeViewHost (kSubView) without ever returning nullptr from TargetForRect or re-implementing the walk. This reverts the NativeWindow flag, the BrowserWindow constructor change, and the RootViewMacTargeterDelegate, leaving the fix entirely in native_window_mac.mm at the same install site as #50330.

nmggithub

Tentatively approving this. Looks good, and it actually covers things I wasn't really happy about with my initial pass (duplication of logic, extra state, constructor modification). Thanks for the improvement!

release-clerk · 2026-05-14T07:18:55Z

No Release Notes

trop · 2026-05-14T07:19:01Z

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

trop · 2026-05-14T07:19:04Z

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

trop · 2026-05-14T20:22:19Z

@VerteDinde has manually backported this PR to "43-x-y", please check out #51625

trop · 2026-05-14T20:22:27Z

@VerteDinde has manually backported this PR to "42-x-y", please check out #51626

Backport of #51617 to 42-x-y. Replace the RootViewMac-level TargetForRect override added in #51586 with a DoesIntersectRect override on content_view_'s own targeter, restoring the structure of #50330 but fixing its actual defect. The original bug (#51576) was that ContentViewTargeterDelegate::TargetForRect returned nullptr when no descendant covered the hit rect, violating the ViewTargeterDelegate contract. RootView::UpdateCursor and RootView::HandleMouseEnteredOrMoved dereference GetEventHandlerForPoint() without null checks, so the nullptr crashed with SIGSEGV when a right-click in a -webkit-app-region: drag region disabled the HTCAPTION early-exit. #51586 fixed the crash by moving the transparency logic to RootViewMac and re-implementing the parent targeter's child-walk loop so it could skip content_view_ and return a sibling instead. That re-implementation duplicates ~25 lines of upstream logic with a subtle rounding divergence (ToEnclosedRectIgnoringError vs ToEnclosingRect), and threads a new content_view_hit_test_transparent_ flag through the platform-neutral NativeWindow/BrowserWindow headers. The same fall-through can be achieved with the existing hit-test hook: override DoesIntersectRect on content_view_ to return false when no visible, processable child intersects the rect. The parent's default TargetForRect loop already skips children whose HitTestRect is false, so hit-testing naturally continues to the WebContentsView sibling and resolves to its NativeViewHost (kSubView) without ever returning nullptr from TargetForRect or re-implementing the walk. This reverts the NativeWindow flag, the BrowserWindow constructor change, and the RootViewMacTargeterDelegate, leaving the fix entirely in native_window_mac.mm at the same install site as #50330. Co-authored-by: Claude <svc-devxp-claude@slack-corp.com>

Backport of #51617 to 43-x-y. Replace the RootViewMac-level TargetForRect override added in #51586 with a DoesIntersectRect override on content_view_'s own targeter, restoring the structure of #50330 but fixing its actual defect. The original bug (#51576) was that ContentViewTargeterDelegate::TargetForRect returned nullptr when no descendant covered the hit rect, violating the ViewTargeterDelegate contract. RootView::UpdateCursor and RootView::HandleMouseEnteredOrMoved dereference GetEventHandlerForPoint() without null checks, so the nullptr crashed with SIGSEGV when a right-click in a -webkit-app-region: drag region disabled the HTCAPTION early-exit. #51586 fixed the crash by moving the transparency logic to RootViewMac and re-implementing the parent targeter's child-walk loop so it could skip content_view_ and return a sibling instead. That re-implementation duplicates ~25 lines of upstream logic with a subtle rounding divergence (ToEnclosedRectIgnoringError vs ToEnclosingRect), and threads a new content_view_hit_test_transparent_ flag through the platform-neutral NativeWindow/BrowserWindow headers. The same fall-through can be achieved with the existing hit-test hook: override DoesIntersectRect on content_view_ to return false when no visible, processable child intersects the rect. The parent's default TargetForRect loop already skips children whose HitTestRect is false, so hit-testing naturally continues to the WebContentsView sibling and resolves to its NativeViewHost (kSubView) without ever returning nullptr from TargetForRect or re-implementing the walk. This reverts the NativeWindow flag, the BrowserWindow constructor change, and the RootViewMacTargeterDelegate, leaving the fix entirely in native_window_mac.mm at the same install site as #50330. Co-authored-by: Claude <svc-devxp-claude@slack-corp.com>

fix: simplify content_view_ hit-test transparency on macOS Replace the RootViewMac-level TargetForRect override added in #51586 with a DoesIntersectRect override on content_view_'s own targeter, restoring the structure of #50330 but fixing its actual defect. The original bug (#51576) was that ContentViewTargeterDelegate::TargetForRect returned nullptr when no descendant covered the hit rect, violating the ViewTargeterDelegate contract. RootView::UpdateCursor and RootView::HandleMouseEnteredOrMoved dereference GetEventHandlerForPoint() without null checks, so the nullptr crashed with SIGSEGV when a right-click in a -webkit-app-region: drag region disabled the HTCAPTION early-exit. re-implementing the parent targeter's child-walk loop so it could skip content_view_ and return a sibling instead. That re-implementation duplicates ~25 lines of upstream logic with a subtle rounding divergence (ToEnclosedRectIgnoringError vs ToEnclosingRect), and threads a new content_view_hit_test_transparent_ flag through the platform-neutral NativeWindow/BrowserWindow headers. The same fall-through can be achieved with the existing hit-test hook: override DoesIntersectRect on content_view_ to return false when no visible, processable child intersects the rect. The parent's default TargetForRect loop already skips children whose HitTestRect is false, so hit-testing naturally continues to the WebContentsView sibling and resolves to its NativeViewHost (kSubView) without ever returning nullptr from TargetForRect or re-implementing the walk. This reverts the NativeWindow flag, the BrowserWindow constructor change, and the RootViewMacTargeterDelegate, leaving the fix entirely in native_window_mac.mm at the same install site as #50330. Co-authored-by: GitHub Copilot <copilot@github.com>

* chore: remove macos hittest workaround patch Reverts the temporary Chromium patch and restores the original macOS hit-test workaround behavior in Electron. Co-authored-by: Charles Kerr <charles@charleskerr.com> * fix: do not return root from TargetForRect Restore the macOS hit-testing behavior that keeps BrowserWindow web contents interactive when content_view_ has no matching child. Co-authored-by: Charles Kerr <charles@charleskerr.com> * refactor: simplify content_view_ hit-test transparency on macOS (#51617) fix: simplify content_view_ hit-test transparency on macOS Replace the RootViewMac-level TargetForRect override added in #51586 with a DoesIntersectRect override on content_view_'s own targeter, restoring the structure of #50330 but fixing its actual defect. The original bug (#51576) was that ContentViewTargeterDelegate::TargetForRect returned nullptr when no descendant covered the hit rect, violating the ViewTargeterDelegate contract. RootView::UpdateCursor and RootView::HandleMouseEnteredOrMoved dereference GetEventHandlerForPoint() without null checks, so the nullptr crashed with SIGSEGV when a right-click in a -webkit-app-region: drag region disabled the HTCAPTION early-exit. re-implementing the parent targeter's child-walk loop so it could skip content_view_ and return a sibling instead. That re-implementation duplicates ~25 lines of upstream logic with a subtle rounding divergence (ToEnclosedRectIgnoringError vs ToEnclosingRect), and threads a new content_view_hit_test_transparent_ flag through the platform-neutral NativeWindow/BrowserWindow headers. The same fall-through can be achieved with the existing hit-test hook: override DoesIntersectRect on content_view_ to return false when no visible, processable child intersects the rect. The parent's default TargetForRect loop already skips children whose HitTestRect is false, so hit-testing naturally continues to the WebContentsView sibling and resolves to its NativeViewHost (kSubView) without ever returning nullptr from TargetForRect or re-implementing the walk. This reverts the NativeWindow flag, the BrowserWindow constructor change, and the RootViewMacTargeterDelegate, leaving the fix entirely in native_window_mac.mm at the same install site as #50330. Co-authored-by: GitHub Copilot <copilot@github.com> --------- Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com> Co-authored-by: Noah Gregory <nmggithub@electronjs.org> Co-authored-by: Samuel Attard <sam@electronjs.org> Co-authored-by: GitHub Copilot <copilot@github.com>

MarshallOfSound added semver/none target/42-x-y PR should also be added to the "42-x-y" branch. target/43-x-y PR should also be added to the "43-x-y" branch. labels May 13, 2026

electron-cation Bot added the new-pr 🌱 PR opened recently label May 13, 2026

MarshallOfSound requested review from codebytere and nmggithub May 13, 2026 22:32

VerteDinde approved these changes May 13, 2026

View reviewed changes

nmggithub approved these changes May 13, 2026

View reviewed changes

MarshallOfSound enabled auto-merge (squash) May 14, 2026 05:34

VerteDinde added the fast-track 🚅 Indicates that this PR is intended to bypass the 24 hour rule. Needs approval from Releases label May 14, 2026

electron-cation Bot removed the new-pr 🌱 PR opened recently label May 14, 2026

MarshallOfSound merged commit 9c2868e into main May 14, 2026
147 of 150 checks passed

MarshallOfSound deleted the fix/scoped-content-view-targeter branch May 14, 2026 07:18

trop Bot removed the target/43-x-y PR should also be added to the "43-x-y" branch. label May 14, 2026

trop Bot added needs-manual-bp/43-x-y needs-manual-bp/42-x-y and removed target/42-x-y PR should also be added to the "42-x-y" branch. labels May 14, 2026

VerteDinde mentioned this pull request May 14, 2026

refactor: simplify content_view_ hit-test transparency on macOS #51625

Merged

VerteDinde mentioned this pull request May 14, 2026

refactor: simplify content_view_ hit-test transparency on macOS #51626

Merged

trop Bot added in-flight/43-x-y and removed needs-manual-bp/43-x-y labels May 14, 2026

trop Bot added in-flight/42-x-y and removed needs-manual-bp/42-x-y labels May 14, 2026

trop Bot added merged/42-x-y PR was merged to the "42-x-y" branch. and removed in-flight/42-x-y labels May 14, 2026

trop Bot added merged/43-x-y PR was merged to the "43-x-y" branch. and removed in-flight/43-x-y labels May 15, 2026

nmggithub mentioned this pull request May 19, 2026

chore: remove macos hittest workaround patch #51699

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: simplify content_view_ hit-test transparency on macOS#51617

refactor: simplify content_view_ hit-test transparency on macOS#51617
MarshallOfSound merged 1 commit into
mainfrom
fix/scoped-content-view-targeter

MarshallOfSound commented May 13, 2026

Uh oh!

nmggithub left a comment •

edited

Loading

Uh oh!

Uh oh!

release-clerk Bot commented May 14, 2026

Uh oh!

trop Bot commented May 14, 2026

Uh oh!

trop Bot commented May 14, 2026

Uh oh!

trop Bot commented May 14, 2026

Uh oh!

trop Bot commented May 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

MarshallOfSound commented May 13, 2026

Description of Change

Background

Why #51586 wasn't the right shape

What this PR does instead

Verification

Backports

Checklist

Release Notes

Uh oh!

nmggithub left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

release-clerk Bot commented May 14, 2026

Uh oh!

trop Bot commented May 14, 2026

Uh oh!

trop Bot commented May 14, 2026

Uh oh!

trop Bot commented May 14, 2026

Uh oh!

trop Bot commented May 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nmggithub left a comment •

edited

Loading