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.

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

fix: non-client mouse events on WCO-enabled windows #32871

Merged
merged 1 commit into from Mar 8, 2022

Conversation

clavin
Copy link
Member

@clavin clavin commented Feb 12, 2022

Description of Change

My change in #32672 caused a regression where WCO-enabled windows (on Windows) could no longer be dragged. This is because in HWNDMessageHandler::HandleMouseEventInternal, HandleMouseEvent (which is what I patched in the previous PR) is called before HandleMouseInputForCaption (which is where the dragging happens I think, from testing). Instead of trying to patch in something to move the marking of the event as handled further down, I opted instead to try and fix the issue with LegacyRenderWidgetHostHWND.

This PR introduces a patch that does two things, particularly only for WCO-enabled windows right now:

  1. Correctly track non-client mouse events, not just regular mouse events. This is mostly just some boilerplate to include those messages.
  2. Instead of falling through to DefWindowProc using the parent window handle (GetParent()), it is instead called on its own window handle. This seems to avoid the issue with a mysterious WM_NCMOUSELEAVE message being generated by the DefWindowProc.
  3. (It also reverts my changes in the previous PR.)

This should be safe to do since the LegacyRenderWidgetHostHWND doesn't normally cover the non-client area in windows without hidden title bars (thus never reaching this code) and for frameless windows this is the same code path. Thus the change should only apply to WCO-enabled windows. I also think the code is still correct since it is the job of LegacyRenderWidgetHostHWND to forward messages to its parent window anyways.

I know this is a patch which we usually try to avoid those, however I don't really see an alternative for either possible solution without making a patch. I'll try to upstream this, but if that fails the only other way to remove this patch would be when LegacyRenderWidgetHostHWND is finally removed (see its header comments for more info on that; doesn't look very defined though).

Checklist

Release Notes

Notes: Fixed drag regions on WCO windows on Windows

@clavin clavin requested a review from a team as a code owner February 12, 2022 02:56
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Feb 12, 2022
@clavin clavin added the semver/patch backwards-compatible bug fixes label Feb 12, 2022
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Feb 13, 2022
@zcbenz zcbenz requested a review from a team as a code owner February 22, 2022 10:50
@clavin clavin force-pushed the clavin/fix-wco-drag branch 3 times, most recently from e7506e3 to 7ee6dda Compare March 1, 2022 22:21
@jkleinsc jkleinsc merged commit e41c3e9 into main Mar 8, 2022
@jkleinsc jkleinsc deleted the clavin/fix-wco-drag branch March 8, 2022 21:06
@release-clerk
Copy link

release-clerk bot commented Mar 8, 2022

Release Notes Persisted

Fixed drag regions on WCO windows on Windows

@trop
Copy link
Contributor

trop bot commented Mar 8, 2022

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

@trop trop bot added the in-flight/15-x-y label Mar 8, 2022
@trop trop bot removed the target/15-x-y label Mar 8, 2022
@trop
Copy link
Contributor

trop bot commented Mar 8, 2022

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

@trop
Copy link
Contributor

trop bot commented Mar 8, 2022

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

@trop
Copy link
Contributor

trop bot commented Mar 8, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants