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

[Bug]: Window resizes when setting resizable: false #31233

Open
3 tasks done
CliffReimers opened this issue Oct 1, 2021 · 12 comments
Open
3 tasks done

[Bug]: Window resizes when setting resizable: false #31233

CliffReimers opened this issue Oct 1, 2021 · 12 comments

Comments

@CliffReimers
Copy link

Preflight Checklist

Electron Version

13.5.1

What operating system are you using?

Windows

Operating System Version

Windows 10 21H1

What arch are you using?

x64

Last Known Working Electron version

13.4.0

Expected Behavior

Window should keep size when calling setResizable(false).

Actual Behavior

Window (and content size) increases by 10px when calling setResizable(false).

Testcase Gist URL

https://gist.github.com/CliffReimers/cfc4784579006b79cef11c34c3a042b5

Additional Information

Probably caused by #31078.

@codebytere
Copy link
Member

cc @CezaryKulakowski

@CezaryKulakowski
Copy link
Contributor

CezaryKulakowski commented Oct 8, 2021

I can confirm that #31078 causes the problem. For some reason window gets event WM_SIZE after thick frame is enabled/disabled. I'd created simple test app and I checked that it's not a standard winapi's behaviour so Electron must somehow invoke the resize. I'm trying to figure out where it happens.

@CezaryKulakowski
Copy link
Contributor

CezaryKulakowski commented Oct 25, 2021

This bug is not really a regression from #31078. The problem was also present before this change landed but it was not so clearly visible: we wasn't updating the Windows' cache so it was necessary to force the redraw by e.g. minimizing and restoring the window to observe the resize.

There are three issues here:

  1. Problem described in this ticket, i.e. noticable window resize when setResizable(true/false) is called. I don't think this is really an issue: note that window size doesn't change, only content's size does. It happens because we disable resizing by removing flag WS_THICKFRAME from window's style. Removing this flag removes sizing box placed around the window which is 5 pixels wide and which is invisible (you can observe it by moving mouse pointer outside the wndow just a bit: although pointer is not over the window anymore you can still resize it and you won't focus window which is beneath our window when you click the mouse button). As window's bounds don't change this 5 pixels are replaced with content and it looks like window was resized. If you decide this is really an issue which needs to be fixed (rather than expected behaviour which we can document) we can change it by not changing WS_THICKFRAME at all: window resize still won't be possible as we set maximum and minimum size of the window to the current size when we disable the resize (more about this is (2)). The only problem is that mouse pointer would still change to resize arrow (i..e. <->) when mouse pointer is placed at the edge of the window but I think it can be fixed by proper handling of WM_SETCURSOR.
  2. When after window resizing is disabled and we force WM_GETMINMAXINFO to be generated (e.g. by minimizing and restoring the window) it's being shrinken by 10 pixels (it's another problem than described in (1) as in this case window's size is actually changed and it can be also reproduced on builds without fix: update Windows' cache after changing window's style #31078). It happens because we first set minimum and maximum size of the window to the current size and after that we make a call to SetCanResize which removes WS_THICKFRAME from the window's style. When window is moved (or minimized and restored) WM_GETMINMAXINFO is generated and its chromium's handler bases on min/max values which we set earlier. The problem is that we set values valid for window with different style (removing WS_THICKFRAME removes sizing box placed around the window which is 5 pixels wide) and not for the current one and it results in window being shrinked. Following app illustrates the problem:
const {app, BrowserWindow} = require('electron')
const path = require('path')

function createWindow () {
  const mainWindow = new BrowserWindow({
    x: 100,
    y: 100,
    width: 800,
    height: 600
  })
  mainWindow.removeMenu();
  let resizable = true;
  setInterval(() => {
    resizable = !resizable;
    mainWindow.setResizable(resizable);
    mainWindow.minimize();
    mainWindow.restore();
    console.log(`window : ${mainWindow.getSize()}`);
    console.log(`content: ${mainWindow.getContentSize()}\n`);
  }, 1000);
}

app.whenReady().then(() => {
  createWindow()
})

Every time when resize is disabled and window is minimized and restored it's size is shrinken by 10 pixels and after few itearations only titlebar remains. This can be fixed by changing order of setting size constraints and setting window's style via call to SetCanResize. We could also don't set minimum and maximum size of the window on Windows as it seems we do it because on Linux there is no "resizable" property of a window and we have to set both the minimum and maximum size to the window size to disable the resize.

Following commit fixes the problem:
CezaryKulakowski@722adf4

  1. When thickFrame is set to false in window's options thick frame will be added to the window when resizing is disabled and enabled again. There is a guard which I believe was added to avoid this:
#if defined(OS_WIN)
  if (has_frame() && thick_frame_)
    FlipWindowStyle(GetAcceleratedWidget(), resizable, WS_THICKFRAME);
#endif

but WS_THICKFRAME will be added by chromium after our call to SetCanResize. I don't know if this is a problem we want to address.

If we would like to fix all three issues I would suggest to:

  1. Do not enable/disable resizing by adding/removing WS_THICKFRAME but only by setting minimum and maximum size as we do it on Linux.
  2. Add handling for WM_SETCURSOR which would prevent mouse pointer from being changed when resizing is disabled and pointer is placed on the edge of the window.

Please let me know what you think about it.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 5, 2022

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. If you have any new additional information—in particular, if this is still reproducible in the latest version of Electron or in the beta—please include it with your comment!

@github-actions github-actions bot added the stale label Oct 5, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2022

This issue has been closed due to inactivity, and will not be monitored. If this is a bug and you can reproduce this issue on a supported version of Electron please open a new issue and include instructions for reproducing the issue.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 6, 2022
@pushkin-
Copy link

This is still a problem with Electron 21

@nornagon nornagon reopened this Nov 28, 2022
@github-actions github-actions bot removed the stale label Nov 29, 2022
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. If you have any new additional information—in particular, if this is still reproducible in the latest version of Electron or in the beta—please include it with your comment!

@github-actions github-actions bot added the stale label Feb 28, 2023
@pushkin-
Copy link

repros in v23

@github-actions github-actions bot removed the stale label Mar 1, 2023
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. If you have any new additional information—in particular, if this is still reproducible in the latest version of Electron or in the beta—please include it with your comment!

@github-actions github-actions bot added the stale label May 31, 2023
@pushkin-
Copy link

bump

@github-actions github-actions bot removed the stale label Jun 1, 2023
@electron-issue-triage
Copy link

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. If you have any new additional information—in particular, if this is still reproducible in the latest version of Electron or in the beta—please include it with your comment!

@pushkin-
Copy link

repros in v26

@codebytere codebytere added status/confirmed A maintainer reproduced the bug or agreed with the feature 25-x-y 26-x-y 27-x-y component/BrowserWindow and removed 14-x-y 15-x-y 21-x-y bug/regression ↩️ A new version of Electron broke something labels Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Unsorted Items
Status: 👀 Unsorted Items
Status: 👍 Does Not Block Stable
Development

No branches or pull requests

5 participants