Skip to content

fix: allow setsize to be called within a move or resize for preventDefault - #34843

Merged
zcbenz merged 1 commit into
electron:mainfrom
mesner:fork/34599-setsize-return
Jul 27, 2022
Merged

fix: allow setsize to be called within a move or resize for preventDefault#34843
zcbenz merged 1 commit into
electron:mainfrom
mesner:fork/34599-setsize-return

Conversation

@mesner

@mesner mesner commented Jul 7, 2022

Copy link
Copy Markdown
Contributor

Description of Change

Closes #34599
Refs #33288
Refs #33279

The prior work in #33279 to allow calling SetBounds during a resize or move (defined as time period between receiving a WM_MOVING or WM_SIZING--respectively--and WM_EXITSIZEMOVE WNDPROC message) prevented the ability to call preventDefault() within 'will-resize' or 'will-move' and then call SetBounds to change the size and position of the window immediately. After the PR, the window does not change bounds until the user ends the resize or move by releasing the mouse button, e.g. This PR simply removes the early-return statement to forward the SetBounds call to the widget, allowing for the bounds to change immediately if the movement or resizing has been cancelled through 'preventDefault' or after it has ended otherwise.

Tested with https://gist.github.com/6b6ea5cdaa1883656a8bd0bf89195283 and https://gist.github.com/meredith-ciq/a7ad120e84bd028a3fbd26b8f61aa45c

This is unfortunately not directly testable via spec, given that it's dependent on user-initiated movement.

Checklist

Release Notes

Notes: Fixed an issue in which calling setBounds() after e.preventDefault in a 'will-move' or 'will-resize' event wouldn't change the window's shape until the mouse button was released.

@welcome

welcome Bot commented Jul 7, 2022

Copy link
Copy Markdown

💖 Thanks for opening this pull request! 💖

We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix.

Examples of commit messages with semantic prefixes:

  • fix: don't overwrite prevent_default if default wasn't prevented
  • feat: add app.isPackaged() method
  • docs: app.isDefaultProtocolClient is now available on Linux

Things that will help get your PR across the finish line:

  • Follow the JavaScript, C++, and Python coding style.
  • Run npm run lint locally to catch formatting errors earlier.
  • Document any user-facing changes you've made following the documentation styleguide.
  • Include tests when adding/changing behavior.
  • Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

@electron-cation electron-cation Bot added the new-pr 🌱 PR opened recently label Jul 7, 2022
@codebytere
codebytere self-requested a review July 8, 2022 08:01
@mesner mesner changed the title fix: #34599 allow setsize to be called within a move or resize for preventDefault fix: allow setsize to be called within a move or resize for preventDefault Jul 11, 2022
@mesner
mesner marked this pull request as ready for review July 11, 2022 14:47
@electron-cation electron-cation Bot removed the new-pr 🌱 PR opened recently label Jul 18, 2022
@mesner

mesner commented Jul 19, 2022

Copy link
Copy Markdown
Contributor Author

@codebytere May I have some help identifying the next steps for this PR?

@zcbenz zcbenz added semver/patch backwards-compatible bug fixes target/18-x-y labels Jul 20, 2022

@zcbenz zcbenz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To conclude, #33288 assumes widget()->SetBounds(bounds) is always ignored by Chromium when window is moving or resizing, but #34599 says it is still working under some cases.

If I'm understanding it correctly then I'm good with this PR, apparently removing the early return won't affect the fix of #33288, and it will recover the old behavior for other cases.

@mesner

mesner commented Jul 20, 2022

Copy link
Copy Markdown
Contributor Author

@zcbenz Yes, your assessment is correct. Thanks!

@mesner

mesner commented Jul 26, 2022

Copy link
Copy Markdown
Contributor Author

@zcbenz What is needed for this to be merged? Sorry to pester but we are stuck on v17.0.0 until #34599 can be resolved.

@zcbenz

zcbenz commented Jul 27, 2022

Copy link
Copy Markdown
Contributor

I was waiting for another review, but this change is obvious enough and I'm going to just merge.

@zcbenz
zcbenz merged commit 9416091 into electron:main Jul 27, 2022
@welcome

welcome Bot commented Jul 27, 2022

Copy link
Copy Markdown

Congrats on merging your first pull request! 🎉🎉🎉

@release-clerk

release-clerk Bot commented Jul 27, 2022

Copy link
Copy Markdown

Release Notes Persisted

Fixed an issue in which calling setBounds() after e.preventDefault in a 'will-move' or 'will-resize' event wouldn't change the window's shape until the mouse button was released.

@trop

trop Bot commented Jul 27, 2022

Copy link
Copy Markdown
Contributor

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

@trop

trop Bot commented Jul 27, 2022

Copy link
Copy Markdown
Contributor

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

@trop

trop Bot commented Jul 27, 2022

Copy link
Copy Markdown
Contributor

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

schetle pushed a commit to schetle/electron that referenced this pull request Nov 3, 2022
…fault (electron#34843)

fix: electron#34599 allow setsize to be called within a move or resize for preventDefault
khalwa pushed a commit to solarwindscloud/electron that referenced this pull request Feb 22, 2023
…fault (electron#34843)

fix: electron#34599 allow setsize to be called within a move or resize for preventDefault
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.

[Bug]: setBounds calls during will-resize are no longer applied when preventing the default resize

2 participants