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

incorrect position when restored from maximize-on-top-drag under Windows #7630 #7766

Merged
merged 1 commit into from Oct 31, 2016

Conversation

Projects
None yet
3 participants
@liusy182
Contributor

liusy182 commented Oct 27, 2016

Fix for #7630

Issues regarding restore/maximize/minimize in general are because default windows behaviour is overridden by handling WM_SIZE message and SetBounds() to last_normal_bounds_. It is difficult to keep last_normal_bounds_ up-to-date since there are many platform-specific scenarios need to be handled.

This fix is to remove overrides and rely on default windows behaviour. I have tested by switching between maximize / restore / minimize / fullscreen and it is working.

I do have a question though why was the override implemented in the first place. Was it to fix some other bugs?

@zcbenz

This comment has been minimized.

Show comment
Hide comment
@zcbenz

zcbenz Oct 31, 2016

Contributor

👍

Contributor

zcbenz commented Oct 31, 2016

👍

@zcbenz zcbenz merged commit 92f8c10 into electron:master Oct 31, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@zcbenz

This comment has been minimized.

Show comment
Hide comment
@zcbenz

zcbenz Oct 31, 2016

Contributor

I do have a question though why was the override implemented in the first place. Was it to fix some other bugs?

They were to fix the size events with Aero Snap: #2929.

Contributor

zcbenz commented Oct 31, 2016

I do have a question though why was the override implemented in the first place. Was it to fix some other bugs?

They were to fix the size events with Aero Snap: #2929.

@liusy182

This comment has been minimized.

Show comment
Hide comment
@liusy182

liusy182 Oct 31, 2016

Contributor

Thanks @zcbenz :)

Contributor

liusy182 commented Oct 31, 2016

Thanks @zcbenz :)

@simast

This comment has been minimized.

Show comment
Hide comment
@simast

simast Nov 6, 2016

Contributor

@liusy182 It seems this change (again) re-introduced the regression where if you minimize and restore browser window when minWidth is used - it is restored to a larger size. Please see #6596.

Contributor

simast commented Nov 6, 2016

@liusy182 It seems this change (again) re-introduced the regression where if you minimize and restore browser window when minWidth is used - it is restored to a larger size. Please see #6596.

@liusy182

This comment has been minimized.

Show comment
Hide comment
@liusy182

liusy182 Nov 8, 2016

Contributor

Sorry @simast I poked around but could not nail down the root cause yet. Maybe some expert can help on this.

Contributor

liusy182 commented Nov 8, 2016

Sorry @simast I poked around but could not nail down the root cause yet. Maybe some expert can help on this.

@simast

This comment has been minimized.

Show comment
Hide comment
@simast

simast Nov 12, 2016

Contributor

@liusy182 I have submitted a proper bug report as #7951 for this issue.

Contributor

simast commented Nov 12, 2016

@liusy182 I have submitted a proper bug report as #7951 for this issue.

@liusy182 liusy182 deleted the liusy182:window-size-restore branch Aug 18, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment