chore: remove macos hittest workaround patch#50330
Merged
Merged
Conversation
CL:6574464 changed BridgedContentView::hitTest: to use GetHitTestResult(), which returns kRootView for any non-null, non-NativeViewHost view — causing BridgedContentView to absorb all web content mouse events. In BrowserWindow, content_view_ sits in front of the sibling WebContentsView and covers the full client area, so it was always found first, breaking all loadURL page interaction. Fix this by installing a ContentViewTargeterDelegate on content_view_ in NativeWindowMac::SetContentView that returns nullptr (instead of the view itself) when no children cover the target point. This makes GetHitTestResult return kOther, allowing hitTest: to fall through to [super hitTest:] and find RenderWidgetHostViewCocoa. This also removes the now-unnecessary chromium partial-revert patch that worked around the same issue.
01ef7ed to
eb82d53
Compare
ckerr
approved these changes
Mar 19, 2026
jkleinsc
approved these changes
Mar 19, 2026
Comment on lines
+119
to
+137
| // A ViewTargeterDelegate installed on content_view_ to make it event- | ||
| // transparent when no children cover the target point. | ||
| // | ||
| // In BrowserWindow, the WebContentsView is added as a sibling of content_view_ | ||
| // at a lower Z-order (behind it) so that user-added child views paint above | ||
| // the web content. However, views::ViewTargeterDelegate::TargetForRect | ||
| // iterates children front-to-back and returns the first child whose | ||
| // HitTestRect() is true. content_view_ covers the full window area, so the | ||
| // WebContentsView (and its NativeViewHost) is never reached. | ||
| // | ||
| // Chromium's BridgedContentView::hitTest: uses GetHitTestResult() to classify | ||
| // the result: NativeViewHost → kSubView (routes to RenderWidgetHostViewCocoa); | ||
| // anything else non-null → kRootView (BridgedContentView absorbs the event, | ||
| // breaking all web content interaction with loadURL); null → kOther (falls | ||
| // through to [super hitTest:] which finds RenderWidgetHostViewCocoa directly). | ||
| // | ||
| // By returning nullptr instead of root when no children cover the target rect, | ||
| // GetEventHandlerForPoint propagates null up the chain → GetHitTestResult | ||
| // returns kOther → hitTest: falls through to [super hitTest:] → NSView walk |
Member
There was a problem hiding this comment.
Thanks for the detailed explanation here!
|
No Release Notes |
Contributor
|
I was unable to backport this PR to "41-x-y" cleanly; |
Contributor
|
I have automatically backported this PR to "42-x-y", please check out #50374 |
5 tasks
Member
Note: This PR introduces a nullptr dereference regression fixed by #51586 . If/when we backport this PR to 41-x-y, we should pick up 51586 as well. |
3 tasks
MarshallOfSound
added a commit
that referenced
this pull request
May 14, 2026
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. #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.
VerteDinde
added a commit
that referenced
this pull request
May 14, 2026
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>
VerteDinde
added a commit
that referenced
this pull request
May 15, 2026
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>
Contributor
ckerr
pushed a commit
that referenced
this pull request
May 19, 2026
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>
jkleinsc
pushed a commit
that referenced
this pull request
May 27, 2026
* 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of Change
CL:6574464 changed
BridgedContentView::hitTest:to useGetHitTestResult(), which returns kRootView for any non-null, non-NativeViewHost view — causingBridgedContentViewto absorb all web content mouse events. In BrowserWindow,content_view_sits in front of the siblingWebContentsViewand covers the full client area, so it was always found first, breaking all loadURL page interaction.Fix this by installing a
ContentViewTargeterDelegateoncontent_view_inNativeWindowMac::SetContentViewthat returns nullptr (instead of the view itself) when no children cover the target point. This makesGetHitTestResultreturnkOther, allowinghitTest:to fall through to [super hitTest:] and findRenderWidgetHostViewCocoa. This also removes the now-unnecessary chromium partial-revert patch that worked around the same issue.Checklist
npm testpassesRelease Notes
Notes: none