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

SnedMessage with WM_NCHITTEST returns HTCAPTION in the TABS #1979

Closed
RamonUnch opened this issue Jun 10, 2021 · 7 comments
Closed

SnedMessage with WM_NCHITTEST returns HTCAPTION in the TABS #1979

RamonUnch opened this issue Jun 10, 2021 · 7 comments

Comments

@RamonUnch
Copy link

Sumatra PDF is THE perfect PDF reader for Windows.
However I encountered an issue while developing the AltDrag software RamonUnch/AltSnap#67.

The problem comes from the fact that Sumatra returns HTCAPTION when the WM_NCHITTEST message is sent and the lParam points towards any of the tabs.

This HTCAPTION should be returned ONLY when lParam is pointing to the title bar outside of the tabs.
When lParam points into a tab, the HTCLIENT value should be returned.

The only other program with tabs I encountered having the same problem is MS Terminal and it will be fixed at some point microsoft/terminal#9443.

Could this be fixed for Sumatra PDF as well? this would also be potentially helpful for other accessibility tools that rely on the WM_NCHITTEST messaging.

@kjk
Copy link
Member

kjk commented Jun 10, 2021

I can reproduce the issue, will look into it.

@kjk
Copy link
Member

kjk commented Jun 10, 2021

I take it back. I was testing the wrong thing.

Which version of Sumatra did you test? I can't reproduce this in latest daily build https://www.sumatrapdfreader.org/dailybuilds

I've enabled "Lower windows by middle clicking on title bars" and was able to close tabs with Left Mouse.

I've also added logging to all handling of WM_NCHITTEST in sumatra and as far as I can tell, I return HTCLIENT for tabs.

Can you re-test with latest daily build?

@GitHubRulesOK
Copy link
Collaborator

@kjk
I may have misunderstood, so am open to correction (the hat with D is in my hand) but my take is that Mouse Middle Button is an option in Alt Drag which is Close Tab in SumatraPDF, so is there some conflict there.

@RamonUnch
Copy link
Author

RamonUnch commented Jun 11, 2021

I was not testing the latest daily build but a slightly older prerelease.
I downloaded the latest daily build 13423 (x64), removed the SumatraPDF-settings.txt file to have a clean profile.
And the problem still occurs... (Using Windows 10 20H2 x64)

image

I do not see the same than @kjk from the outside...
The tabs are in the title bar, maybe this causes a problem?
When check from AltDrag the WM_NCHITTEST it still returns 2 ie HTCAPTION. in the tab, the rest is correct, outside of the scroll bars that are marked as HTCLIENT, when I would expect them to be HTVSCROLL nad HTHSCROLL, but if they are not standard scroll bar then I guess they should be marked HTCLIENT.

The problem for AltDrag is only that when you middle click on a HTCAPTION area, the window is lowered. However when someone middle clicks on the Sumatra PDF tab this latter should be closed instead of lowering the whole window.

This causes no other problems than the Middle click tab closing, that can no longer work because AltDrag can not see the difference between the tab and the title bar, so AltDrag will lower the window instead of forwarding the click to Sumatra.

After searching in the code base I guess the issue is maybe here?

sumatrapdf/src/Tabs.cpp

Lines 502 to 511 in af0d18c

case WM_NCHITTEST: {
if (!tab->inTitlebar || hwnd == GetCapture()) {
return HTCLIENT;
}
POINT pt = {GET_X_LPARAM(lp), GET_Y_LPARAM(lp)};
ScreenToClient(hwnd, &pt);
if (-1 != tab->IndexFromPoint(pt.x, pt.y)) {
return HTCLIENT;
}
}

Or maybe here?

sumatrapdf/src/Caption.cpp

Lines 813 to 827 in af0d18c

case WM_NCHITTEST: {
// Provide hit testing for the caption.
int x = GET_X_LPARAM(lp);
int y = GET_Y_LPARAM(lp);
Point pt{x, y};
Rect rClient = MapRectToWindow(ClientRect(hwnd), hwnd, HWND_DESKTOP);
Rect rCaption = WindowRect(win->hwndCaption);
if (rClient.Contains(pt) && pt.y < rCaption.y + rCaption.dy) {
*callDef = false;
if (pt.y < rCaption.y) {
return HTTOP;
}
return HTCAPTION;
}
} break;

I was not very clear, I hope this helps.

@kjk
Copy link
Member

kjk commented Jun 11, 2021

@RamonUnch thanks for additional information.

I understand it now and can reproduce. With "Lower windows..." option turned on middle click no longer closes tabs.

I've added logging and I've noticed that on middle click sequence of messages is different with "Lower windows..." option turned on:

Without AltDrag "Lower windows...":

middle click down:
WM_NCHITTEST TabBarProc HTCLIENT

middle click up
WM_NCHITTEST WndProcCaption HTTRANSPARENT
WM_NCHITTEST CustomCaptionFrameProc HTCAPTION
WM_NCHITTEST WndProcCaption HTTRANSPARENT
WM_NCHITTEST CustomCaptionFrameProc HTCAPTION

With AltDrag "Lower windows...":

middle click down:
WM_NCHITTEST CustomCaptionFrameProc HTCAPTION

middle click up:
WM_NCHITTEST TabBarProc HTCLIENT

As you predicted, the issue is that we return HTCAPTION instead of HTCLIENT.

However, with "Lower windows..." option turned on, AltDrag sends a message to a different window so arguably Sumatra is returning the right response.

In Sumatra CustomCaptionFrameProc is a HWND representing the caption area and returns HTCLIENT / HTOP

TabBarProc is a child window of CustomCaptionFrameProc and return HTCLIENT / HTTRANSPARENT.

The issue is that AltDrag sends WM_NCHITTEST to parent of the window:

static int ActionNoAlt(POINT pt, WPARAM wParam)
...
        hwnd = GetAncestor(hwnd, GA_ROOT);
        if (blacklisted(hwnd, &BlkLst.Windows))
            return 0;

        DWORD area= HitTestTimeout(hwnd, pt.x, pt.y);

Is this a subtle bug in AltDrag i.e. GetAncestor() was only meant for blacklisting purposes and not to change the target HWND of WM_NCHITTEST?

Or is there a valid reason for sending WM_NCHITTEST to GetAncestor(hwnd)?

@RamonUnch
Copy link
Author

@kjk
Thanks very much for your investigation.
You are right, there are no good reasons to retrieve the parent window for the WM_NCHITTEST, I did this without thinking, not only for the blacklist but also because AltDrag always has to move and position the parent window at the end.

I will fix AltDrag then. I thought the problem was from Sumatra because other programs (such as firefox/chrome) would give the "correct" values from the ancestor hwnd. but I think you are right and there was nothing wrong with Sumatra.
So again, thanks for your precious time.

@RamonUnch
Copy link
Author

Well AltDrag is now fixed, I sent you a little donation for your efforts and your software.

RamonUnch added a commit to RamonUnch/altdrag-1 that referenced this issue Jun 17, 2021
On some windows such as Sumatra PDF, or Office 2010, by avoiding to get the ancestor window before the WM_NCHITTEST message.
SumatraPDF discussion: sumatrapdfreader/sumatrapdf#1979
I had to add the HitTestTimeout() function that checks the response of the WM_NCHITTEST on the pointed hwnd, if it returns HTTRANSPARENT, then I check the parent window until there is a parent or the HitTest returned something different than Transparent.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants