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

WM_NCHITTEST returns HTCAPTION for client area and tabs #8795

Open
chaohershi opened this issue Jan 15, 2021 · 4 comments
Open

WM_NCHITTEST returns HTCAPTION for client area and tabs #8795

chaohershi opened this issue Jan 15, 2021 · 4 comments
Labels
Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal.
Milestone

Comments

@chaohershi
Copy link

Description of the new feature/enhancement

Please consider placing the tabs into the client area instead of the title bar. Right now the tabs are in the correct place only with setting "showTabsInTitlebar": false,. This will cause problems with softwares that utilize the title bars.

Snipaste_2021-01-14_22-36-17
Default settings

Snipaste_2021-01-14_22-32-35
"showTabsInTitlebar": false,

Proposed technical implementation details (optional)

Take a look at Edge browser. Edge used to place everything in the title bar as well, but now it has been fixed.

Snipaste_2021-01-14_22-39-10

@chaohershi chaohershi added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Jan 15, 2021
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Jan 15, 2021
@zadjii-msft
Copy link
Member

Well, that's certainly an interesting one. Thanks for finding this!

@zadjii-msft zadjii-msft added Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. and removed Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. labels Jan 15, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jan 15, 2021
@zadjii-msft zadjii-msft added this to the Terminal v2.0 milestone Jan 15, 2021
@DHowett DHowett changed the title Place the tabs into the client area instead of the title bar WM_NCHITTEST returns HTCAPTION for client area and tabs Jan 17, 2021
@DHowett
Copy link
Member

DHowett commented Jan 17, 2021

I've updated the title to more accurately represent what is happening.

Offending code is in NonClientIslandWindow.cpp, NonClientIslandWindow::_OnNcHitTest.

@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jan 22, 2021
@RamonUnch
Copy link

I did encounter this problem while developping the AltDrag software.
It could be nice if it could be fixed. It forces me to treat Windows Terminal as a spetial case
I will infiorm my users that setting the "showTabsInTitlebar": false, fixes the problem.

@zadjii-msft
Copy link
Member

Notes from a mail thread

What they're doing is not really supported (or at least not expected by any of this newer input magic code), and I think you only get the proper behavior if you're actual input rather than some random process calling WindowFromPoint + SendMessage(WM_NCHITTEST). I agree though you could probably make your implementation return "the right thing" even though the OS would never ask you for those regions.

So I'll probably need to roll a scratch test app to be able to investigate this fully.

@zadjii-msft zadjii-msft removed their assignment Sep 1, 2021
ghost pushed a commit that referenced this issue Nov 29, 2021
Adds snap layout support to the Terminal's maximize button. This PR is
full of BODGY, so brace yourselves.

Big thanks to Chris Swan in #11134 for building the prototype.
I don't believe this solves #8795, because XAML islands can't get
nchittest messages

- The window procedure for the drag bar forwards clicks on its client
  area to its parent as non-client clicks.
- BODGY: It also _manually_ handles the caption buttons. They exist in
  the titlebar, and work reasonably well with just XAML, if the drag bar
  isn't covering them.
- However, to get snap layout support, we need to actually return
  `HTMAXBUTTON` where the maximize button is. If the drag bar doesn't
  cover the caption buttons, then the core input site (which takes up
  the entirety of the XAML island) will steal the `WM_NCHITTEST` before
  we get a chance to handle it.
- So, the drag bar covers the caption buttons, and manually handles
  hovering and pressing them when needed. This gives the impression that
  they're getting input as they normally would, even if they're not
  _really_ getting input via XAML.
- We also need to manually display the button tooltips now, because XAML
  doesn't know when they've been hovered for long enough. Hence, the
  `_displayToolTip` `ThrottledFuncTrailing`

## Validation
Minimized, maximized, restored down, hovered the buttons slowly, moved
the mouse over them quickly, they feel the same as before. But now with
snap layouts appearing.

## TODO!
* [x] I'm working on getting the ToolTips on the caption buttons back. Alas, I needed a demo of this _today_, so I'll fix that tomorrow morning.
* [x] mild concern: I should probably test Win 10 to make sure there wasn't weird changes to the message loop in win11 that means this is broken on win10.
* [x] I think I used the wrong issue number for tons of my comments throughout this PR. Double check that. Should be #9443, not #9447. 

Closes #9443
I thought this took care of #8587 ~as a bonus, because I was here, and the fix is _now_ trivial~, but looking at the latest commit that regressed.

Co-authored-by: Chris Swan <[email protected]>
DHowett pushed a commit that referenced this issue Dec 13, 2021
Adds snap layout support to the Terminal's maximize button. This PR is
full of BODGY, so brace yourselves.

Big thanks to Chris Swan in #11134 for building the prototype.
I don't believe this solves #8795, because XAML islands can't get
nchittest messages

- The window procedure for the drag bar forwards clicks on its client
  area to its parent as non-client clicks.
- BODGY: It also _manually_ handles the caption buttons. They exist in
  the titlebar, and work reasonably well with just XAML, if the drag bar
  isn't covering them.
- However, to get snap layout support, we need to actually return
  `HTMAXBUTTON` where the maximize button is. If the drag bar doesn't
  cover the caption buttons, then the core input site (which takes up
  the entirety of the XAML island) will steal the `WM_NCHITTEST` before
  we get a chance to handle it.
- So, the drag bar covers the caption buttons, and manually handles
  hovering and pressing them when needed. This gives the impression that
  they're getting input as they normally would, even if they're not
  _really_ getting input via XAML.
- We also need to manually display the button tooltips now, because XAML
  doesn't know when they've been hovered for long enough. Hence, the
  `_displayToolTip` `ThrottledFuncTrailing`

## Validation
Minimized, maximized, restored down, hovered the buttons slowly, moved
the mouse over them quickly, they feel the same as before. But now with
snap layouts appearing.

## TODO!
* [x] I'm working on getting the ToolTips on the caption buttons back. Alas, I needed a demo of this _today_, so I'll fix that tomorrow morning.
* [x] mild concern: I should probably test Win 10 to make sure there wasn't weird changes to the message loop in win11 that means this is broken on win10.
* [x] I think I used the wrong issue number for tons of my comments throughout this PR. Double check that. Should be #9443, not #9447.

Closes #9443
I thought this took care of #8587 ~as a bonus, because I was here, and the fix is _now_ trivial~, but looking at the latest commit that regressed.

Co-authored-by: Chris Swan <[email protected]>
(cherry picked from commit f2ebb21)
@zadjii-msft zadjii-msft modified the milestones: Terminal Backlog, Backlog Jan 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal.
Projects
None yet
Development

No branches or pull requests

4 participants