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

Resize window area is 1 pixel in top right of screen #185249

Open
Tyriar opened this issue Jun 15, 2023 · 20 comments
Open

Resize window area is 1 pixel in top right of screen #185249

Tyriar opened this issue Jun 15, 2023 · 20 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug electron Issues and items related to Electron titlebar VS Code main title bar issues upstream Issue identified as 'upstream' component related (exists outside of VS Code) upstream-issue-linked This is an upstream issue that has been reported upstream windows VS Code on Windows issues
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Jun 15, 2023

I've noticed I naturally resize windows in the top right almost always, it's very difficult in VS Code as the hit area is a single pixel on Windows with the custom title bar.

image

@Tyriar Tyriar added the windows VS Code on Windows issues label Jun 15, 2023
@sbatten sbatten removed their assignment Aug 28, 2023
@sbatten
Copy link
Member

sbatten commented Aug 28, 2023

@deepak1556 the horizontal pixels left by WCO is not enough

@deepak1556 deepak1556 added upstream Issue identified as 'upstream' component related (exists outside of VS Code) good first issue Issues identified as good for first-time contributors electron Issues and items related to Electron upstream-issue-pending Issues that are caused by chromium but have not been reported due to pending minimal repro labels Aug 29, 2023
@Krytan
Copy link

Krytan commented Sep 1, 2023

@deepak1556 I would like to solve this problem, It will be my first open source contribution for this repository.
Do you mind if I start?

@deepak1556
Copy link
Collaborator

@Krytan thanks for your interest, first step would be to check if the issue repros in PWA application using the titlebar overlay feature

@mkos11
Copy link

mkos11 commented Oct 8, 2023

I would like to solve this problem, It will be my first open source contributi
on for this repository.
Do you mind if I start? but i dont know much i just fork on git hub that alll

@MrJithil
Copy link
Contributor

MrJithil commented Nov 6, 2023

Can I work on it?
CC: @mkos11

@Krytan
Copy link

Krytan commented Nov 6, 2023 via email

@MrJithil
Copy link
Contributor

MrJithil commented Nov 6, 2023

Thank you . Will wait for others shown interests to avoid the duplicate effort.

@joaomoreno joaomoreno assigned rzhao271 and unassigned deepak1556 Nov 6, 2023
@joaomoreno joaomoreno added this to the November 2023 milestone Nov 6, 2023
@joaomoreno joaomoreno added the bug Issue identified by VS Code Team member as probable bug label Nov 6, 2023
@rzhao271
Copy link
Contributor

rzhao271 commented Nov 7, 2023

This looks more like an upstream Electron issue.
I'm able to reproduce the issue with Electron Fiddle using Electron v25.9.3, v27.0.3, and v28.0.0-beta.3 by creating a frameless BrowserWindow with WCO enabled. I'm unable to reproduce the issue in Chrome or Edge.

Potentially related to electron/electron#27247 and #157197

@rzhao271 rzhao271 removed the good first issue Issues identified as good for first-time contributors label Nov 10, 2023
@rzhao271
Copy link
Contributor

rzhao271 commented Nov 10, 2023

This issue is related to electron/electron#27247, #157197, and #102023.

For frameless Electron windows, the cursor changes to a resize cursor only when it is on the border of the window, or a bit into the window.

For Electron windows with frames, as well as for Edge and Chrome windows, the cursor also changes to a resize cursor when the cursor is a bit to the right of the right border of the window, or a bit to the left of the left border of the window.

Therefore, the main issue doesn't have to do with the close button being too close to the border, but with the resize area not extending far enough from the window, ref #157197.

Upstream issue filed: electron/electron#40505

@rzhao271 rzhao271 added good first issue Issues identified as good for first-time contributors upstream-issue-linked This is an upstream issue that has been reported upstream and removed upstream-issue-pending Issues that are caused by chromium but have not been reported due to pending minimal repro labels Nov 10, 2023
@rzhao271 rzhao271 removed the good first issue Issues identified as good for first-time contributors label Nov 27, 2023
@rzhao271 rzhao271 removed this from the November 2023 milestone Nov 27, 2023
@rzhao271 rzhao271 added this to the December 2023 milestone Nov 27, 2023
@rzhao271 rzhao271 added the titlebar VS Code main title bar issues label Dec 7, 2023
@hotdogee
Copy link

hotdogee commented Dec 14, 2023

I have a working fix in electron, currently doing additional testing in Windows 10 and Windows 11 under multiple DPI settings to make sure the fix is good.

frameless-bug-vscode

@rzhao271
Copy link
Contributor

The question I have is why Electron's resize boundary doesn't extend outwards from the black frame.
If we can figure that out, then I believe the UX could improve for all Electron applications.

@hotdogee
Copy link

hotdogee commented Dec 14, 2023

The question I have is why Electron's resize boundary doesn't extend outwards from the black frame. If we can figure that out, then I believe the UX could improve for all Electron applications.

To be clear, are you expecting a technical (how to fix) or philosophical (should it be fixed) answer to the question?

@rzhao271
Copy link
Contributor

I was thinking of looking for a technical answer myself around next month, though you're welcome to also try and investigate.

@hotdogee
Copy link

hotdogee commented Dec 14, 2023

I was thinking of looking for a technical answer myself around next month, though you're welcome to also try and investigate.

I'm ready to open a pull request, just some final testing on multiple operation systems.
I spent the past week re-studying the Win32 API and reading Charles Petzold's Programming Windows before finally making the fix with as few changes to the existing code as possible.

@hotdogee
Copy link

I've been pixel peeping blownup screenshots of current versus fixed windows making sure every pixel is the same, I think I've got it.

2023-12-15.09-58-03.mp4

@rzhao271
Copy link
Contributor

Hi @hotdogee, it turns out I misunderstood your diagram.
It looks like your fix does fix the resize boundary issue, making my earlier question in #185249 (comment) redundant.
Feel free to submit a PR to the https://github.com/electron/electron repository when you're ready.
Thanks!

@rzhao271
Copy link
Contributor

Hi @hotdogee, when you're ready, could you put up a PR for your fix?

@rzhao271
Copy link
Contributor

rzhao271 commented Jan 23, 2024

Current findings

The resize area issue occurs when creating an Electron BrowserWindow with the frame property set to false. There is another property for BrowserWindow called thickFrame. The resize area issue is not fixed after setting thickFrame to true; it seems that the correct behaviour only occurs when frame itself is set to true.

Changing numeric frame-related constant values throughout the code doesn't seem to fix the issue. Commenting out !has_frame() if-statements does not fix the issue, either.

I created a minimal WinAPI window example in Visual Studio and realized that the WS_THICKFRAME window style allows us to get the resizing area behaviour we want. I have also confirmed that that style does not automatically add a full caption to a window.

I also confirmed with Spy++ that setting thickFrame to true while keeping frame set to false for an Electron BrowserWindow results in a window that does not have the WS_THICKFRAME style, which seems to explain why the resizing area differs between framed and frameless BrowserWindows in Electron, even when thickFrame is set to true.

Edit: Spy++ suggests that Edge and Explorer don't have WS_THICKFRAME either, yet those windows resize properly. I'll have to try out more styles later because I still think the issue has something to do with windows styles.

@deepak1556
Copy link
Collaborator

There is another property for BrowserWindow called thickFrame. The resize area issue is not fixed after setting thickFrame to true; it seems that the correct behaviour only occurs when frame itself is set to true.

ThickFrame option is true by default for frameless windows.

I also confirmed with Spy++ that setting thickFrame to true while keeping frame set to false for an Electron BrowserWindow results in a window that does not have the WS_THICKFRAME style

Hmm this contradicts https://github.com/electron/electron/blob/1300e83884595a3c89c24bd4d42f3a1bbbb6fe7d/shell/browser/native_window_views.cc#L365-L379, would be a bug if that style is not applied as the runtime expects it to be.

@rzhao271 rzhao271 modified the milestones: December / January 2024, February 2024 Jan 24, 2024
@rzhao271
Copy link
Contributor

rzhao271 commented Jan 30, 2024

I just realized that I've been using Spy++ incorrectly. Instead of placing the window finder crosshairs on the titlebar, I have been placing it on the "content window". As a result, WS_THICKFRAME never seemed to be showing up in the styles list.

I have confirmed that framed Electron applications as well as framed native applications such as Notepad and Paint have WS_THICKFRAME styles.

With Electron Fiddle, I made a minimal frameless Electron window with thickFrame set to true, but there was no titlebar to place Spy++ on. I placed the crosshairs in the "content window", and WS_THICKFRAME did not show up in the styles list. I also placed the crosshairs on the VS Code Insiders custom titlebar, and WS_THICKFRAME still did not show up. I have not tried setting thickFrame to true for VS Code.

Edit: I noticed I can just search through the list of windows that Spy++ sees in order to find the outer frame for VS Code windows. I have confirmed that that frame contains the WS_THICKFRAME style. I also confirmed that Electron has the same styles as well. Therefore, both windows are frameless windows with WS_THICKFRAME already.

@rzhao271 rzhao271 modified the milestones: February 2024, March 2024 Feb 15, 2024
@rzhao271 rzhao271 modified the milestones: March 2024, On Deck Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue identified by VS Code Team member as probable bug electron Issues and items related to Electron titlebar VS Code main title bar issues upstream Issue identified as 'upstream' component related (exists outside of VS Code) upstream-issue-linked This is an upstream issue that has been reported upstream windows VS Code on Windows issues
Projects
None yet
Development

No branches or pull requests

9 participants