Kalmat / PyWinCtl

Cross-Platform module to get info on and control windows on screen
Other
179 stars 19 forks source link

[Windows] pywinctl.getAllWindows() not getting all Windows #67

Closed Xenolphthalein closed 1 year ago

Xenolphthalein commented 1 year ago

Hello! First of all: Thanks for this great module! Let's get right to the Problem: The pywinctl.getAllWindows() method is not returning all open windows. I borrowed the code you use to determine if a window should be listed by pywinctl to create a simple debug script:

import time
import ctypes
from typing import List
from ctypes import wintypes
import win32gui
import win32con
import pywinctl

TYPE_CHECKING = True

class TITLEBARINFO(ctypes.Structure):
        if TYPE_CHECKING:
            cbSize: int
            rcTitleBar: wintypes.RECT
            rgstate: List[int]
            def __init__(
                self,
                cbSize: int = ...,
                rcTitleBar: wintypes.RECT = ...,
                rgstate: List[int] = ...
            ): ...

        _fields_ = [
            ("cbSize", wintypes.DWORD),
            ("rcTitleBar", wintypes.RECT),
            ("rgstate", wintypes.DWORD * 6)
        ]

time.sleep(3)

window = pywinctl.getActiveWindow()
title_info = TITLEBARINFO()
title_info.cbSize = ctypes.sizeof(title_info)
ctypes.windll.user32.GetTitleBarInfo(window.getHandle(), ctypes.byref(title_info))
isCloaked = ctypes.c_int(0)
DWMWA_CLOAKED = 14
ctypes.windll.dwmapi.DwmGetWindowAttribute(window.getHandle(), DWMWA_CLOAKED, ctypes.byref(isCloaked), ctypes.sizeof(isCloaked))
title = win32gui.GetWindowText(window.getHandle())

print('###### Active window: ######')
print(f"Active window: {window.title}")
print(f"Visible: {win32gui.IsWindowVisible(window.getHandle())}")
print(f"Cloaked: {isCloaked.value}")
print(f"Title: {title}")
print(f"TitleInfo.rgstate: {title_info.rgstate[0]}")
print(f"Bit AND: {title_info.rgstate[0] & win32con.STATE_SYSTEM_INVISIBLE}")

# Print all other window titles for the getAllWindows method:
print('###### Window list according to pywinctl: ######')
windows = pywinctl.getAllWindows()
for w in windows:
    print(w.title)

If i run this script and click on for example the Eclipse Installer application (to set it active and focus it) it outputs the following:

###### Active window: ######
Active window: Eclipse Installer
Visible: 1
Cloaked: 0
Title: Eclipse Installer
TitleInfo.rgstate: 1081344
Bit AND: 32768
###### Window list according to pywinctl: ######
test.py - pywinctl_test - Visual Studio Code
Task Manager

According to the code (https://github.com/Kalmat/PyWinCtl/blob/f78106e26b7689592c249cd6b19a7311391164cb/src/pywinctl/_pywinctl_win.py#L310) a window is only added to the returned handles if the bit AND operation is returning 0, but that seems to not be the case for the Eclipse Installer application even if it is focused and active.

I saw the stackoverflow issue that was linked in the code and i understand that it is easier if you don't have all the background process windows from apps showing up. But i think it would be better to let the programmers decide if they want to see them in the list or not.

So my proposed fix for this would be: A parameter or global flag you can set to disable this specific window filtering mechanism like pywinctl.windowFiltering = False or anything like that.

If you need any more information about this issue i am happy to help!

Kalmat commented 1 year ago

HI! Thank you so much for your feedback and help! I do really appreciate it.

First off, you are totally right. My current solution is missing windows that should be there. Thank you for pointing it out!

The windows returned by win32gui.EnumWindows() are annoying and somehow "dangerous" since it will fail in case you try to perform some actions on many of them; this is why I was trying to find a way to focus on the "real", user windows (as in the Alt-Tab switcher). Unfortunately, it seems there is no solution (you can see a huge number of questions regarding this issue just googling a little bit). So, following your advice, I will have to rollback to previous getAllWindows() version, not filtering the windows. The question here is how to implement this.

Your suggestion about a global flag is very sharp, but I have doubts on how to manage it in other platforms (on Linux and macOS this problem doesn't apply). Apart from this, I am not sure if the devs using the module will realize there is such a parameter (if they don't activate it and loop over all returned windows trying to do something on them, they should enclose it inside a try-catch block, or it will likely fail). Another alternative is to add that parameter as the input in the invokation of the functions retrieving windows objects like getAllWindows(), getAllTitles(), getAllWindowsWithTitle() and so on... This gives more "visibility" to the parameter (reducing the risk of crashes), but it's undeniably ugly, and it only applies to win32 platform (so, again, it should be added to other platforms too, but with no effect)...

Perhaps another alternative is to find a way to filter only those windows which may fail afterwards, but I can not imagine right now how to do that without performing any "intrussive" action (like minimize() or activate() or similar...)

What do you think? Any idea or opinion is more than welcome. This kind of discussions, having other perspectives, really help me a lot!

Xenolphthalein commented 1 year ago

I tested a little bit around and might have a different solution for the problem. But it comes with a catch, a catch of which i do not know if it will cause problems or not.

First to the main issue with the TITLEBARINFO approach: If you get the TITLEBARINFO with GetTitleBarInfo it contains the following values:

The code then proceeds to check if the titlebar element (index 0) is currently visible by AND'ing the state with the win32con.STATE_SYSTEM_INVISIBLE constant which contains 0x00008000.

The problem is that some application do not use the default window titlebar aka "chrome" and implement a better styled titlebar in their own client area. The normal applications titlebar is therefore set to STATE_SYSTEM_INVISIBLE, as well as all other components of the rgstate.

Now to the possible fix for this issue: Instead of checking if there is a title bar, we can check if the window is a tool window instead:

is_tool_window = False
WS_EX_TOOLWINDOW = 0x00000080 // https://learn.microsoft.com/de-de/windows/win32/winmsg/extended-window-styles
GWL_EXSTYLE = -20 // https://learn.microsoft.com/de-de/windows/win32/api/winuser/nf-winuser-getwindowlonga
extended_style = win32gui.GetWindowLong(hwnd, GWL_EXSTYLE) 
if (extended_style & WS_EX_TOOLWINDOW) != 0:
  is_tool_window = True

If you now filter out tool_windows you will most likely get all the windows sorted out which you don't want. The catch is if you really need a tool window you will not be able to find it in GetAllWindows. I did not test if the current code is/was able to find tool windows in the first place, so maybe there is no catch at all.

On the other hand if i only remove the TITLEBARINFO filter, there are 3 windows which are being additionally returned, those being, NVIDIA Geforce Overlay, Program Manager and BBar. The risk by including those seem relatively low.

But that's something for you to decide. :)

Kalmat commented 1 year ago

Wow! Really impressive! I will look into it as soon as I have a moment. Thank you!

Kalmat commented 1 year ago

Hi again! Sorry for the delay in replying. I have been testing with all combinations I've been able to figure out, but didn't reach a definite knowledge on potentital side effects... I think I will rollback to previous version (though too verbose and risky) because of two reasons:

I will try to warn users by all means, anyway, in order to avoid unwanted crashes.

Any other idea, comment or help is really appreciated. Thank you!

Kalmat commented 1 year ago

Hi! I have prepared a new version that hopefully solves this issue. If you want to give it a try, you can find it here. Anyway, I will upload it to PyPi in a few days, as soon as I finish all required, intensive tests.

Thank you again!