nvda icon indicating copy to clipboard operation
nvda copied to clipboard

Report virtual desktop switches in Windows 10

Open jcsteh opened this issue 4 years ago • 27 comments

Link to issue number:

Fixes #5641.

Summary of the issue:

In Windows 10, it is possible to have different virtual desktops, so as to switch between large groups of applications. To switch between desktops, press control+windows+left/rightArrow. NVDA however does not report when the desktop changes.

Description of how this pull request fixes the issue:

Based on reverted #8259.

Desktop switches can only be reported on Windows 10 May 2019 and above. Previous Windows 10 builds could experience delays and incorrect desktop names.

Implementation details:

  1. Added a Desktop IAccessible NVDAObject which is now used instead of the Desktop Window NVDAObject. This is used for api.getDesktopObject(). It is also used to retrieve the name and to catch nameChange events (see below). Querying of the name via IAccessible is disabled altogether before Windows 10 may 2019, in which case it is hard-coded like it was before this PR.
  2. Added eventHandler.handlePossibleDesktopNameChange function. This function checks to see if the Desktop's name has changed since the last time it was called. If so, then it reports the new desktop name. It fetches the desktop name using the Desktop IAccessible object.
  3. eventHandler.doPreGainFocus now calls handlePossibleDesktopNameChange, but only after any possible foreground event, so as to ensure that speech is not interrupted. In short this means that the desktop name will be announced before the new focus and its ancestry, but after speech is canceled due to a foreground change.
  4. handlePossibleDesktopNameChange is also called for nameChange events on the Desktop IAccessible object. However, NVDA delays the execution of handlePossibleDesktopNameChange in this case by 250 ms, to ensure that it executes after any other possible focus changes. Of course if a focus change did detect the desktop name change, then the delayed call from the name change event will simply do nothing. The reason for the call in the name change event is to handle the situations where the desktop changes but there is no focus event, such as when focused on the taskbar.

Changes from #8259:

  1. Don't allow the name property on the Desktop object to retrieve the name via IAccessible on earlier versions of Windows, since this could be triggered outside of handlePossibleDesktopNameChange, potentially causing problems.
  2. Use the Desktop IAccessible NVDAObject for api.getDesktopObject(). In #8259, a WindowRoot IAccessible NVDAObject was being used instead. This requires us to use getNVDAObjectFromEvent when setting the desktop object so we can specify OBJID_CLIENT.
  3. handlePossibleDesktopNameChange fetches the name via IAccessible instead of UIA. I think these are both the same, but we already have a Desktop IAccessible, so we may as well use it.

Testing performed:

Ran NVDA on Windows 10. Switched between multiple virtual desktops. Confirmed that the new desktop name was reported. This test was done both with focus in an application, and on the taskbar.

Also confirmed that if a desktop is renamed, the user specified name is reported instead of "Desktop 1", etc.

Known issues with pull request:

I don't have older builds of Windows 10 to test with. I believe this should address the issues that caused #8259 to be reverted, but I can't be certain.

Change log entry:

New features: - The name of the current virtual desktop on windows 10 is now reported when switching virtual desktops. (#5641)

jcsteh avatar Apr 19 '20 03:04 jcsteh

Hi, one test case to consider: what if virtual desktops are renamed (available starting Version 2004/May 2020 Update/build 19041)? As for testing on earlier builds, I can try on at least 1809. Thanks.

josephsl avatar Apr 19 '20 03:04 josephsl

Oh yeah, I tested that and forgot to note it in my testing scenarios. It works. I'll update.

jcsteh avatar Apr 19 '20 03:04 jcsteh

Hi,

This might pose a problem if the following happens:

Suppose a user running Version 2004 opens multiple virtual desktops. If two or more consecutive virtual desktops have the same name, desktop name will not be reported.

STR:

  1. Create two virtual desktops, one after the other.
  2. Open second to last virtual desktop.
  3. Press Windows+Tab to open Task View, then from virtual desktops list, focus on the second to last one.
  4. Press F2 to rename it.
  5. Back in Task View, move focus to the last virtual desktop.
  6. Press F2 to rename the focused virtual desktop, but give it the same name as second to last one and press Enter.
  7. Close Task View and switch between virtual desktops.

Expected: the names of last two virtual desktops are announced. Actual: no announcement due to equality checks.

Thanks.

josephsl avatar Apr 19 '20 03:04 josephsl

Interesting enough, desktop switches are reported for me on alpha versions of NVDA. How is that related to this pr?

LeonarddeR avatar Apr 30 '20 07:04 LeonarddeR

That's... curious. As in, you hear the desktop name: desktop 1, desktop 2, etc.? I don't follow how that would be happening. Do you have any add-ons that might be doing that? I believe Joseph's Win 10 Essentials supports it?

jcsteh avatar Apr 30 '20 07:04 jcsteh

I believe Joseph's Win 10 Essentials supports it?

Ah, gotcha. Never mind my words.

LeonarddeR avatar Apr 30 '20 09:04 LeonarddeR

hello @jcsteh This branch has conflicts that must be resolved Only those with Conflicting files source/eventHandler.py Can this problem be fixed? thanks

dpy013 avatar Jun 09 '20 21:06 dpy013

Hi @jcsteh Could you look into the comment by @josephsl

I think this would be a confusing edge case for users, is there anything that can be done about it? I'm going to mark this PR as a draft for now, please mark it as ready when this is resolved.

feerrenrut avatar Jun 25 '20 15:06 feerrenrut

Any updates on this PR?

codeofdusk avatar Sep 25 '20 16:09 codeofdusk

@jcsteh Do you have any new progress on this PR? Thank you

cary-rowen avatar Dec 07 '20 16:12 cary-rowen

I don't see how I can reasonably address https://github.com/nvaccess/nvda/pull/11031#issuecomment-616018671 without risking double speaking. Personally, I don't see a good use case for having two consecutive desktops with the same name and I'm not willing to risk double speaking for that edge case. Still, it's been noted that this is a requirement for getting this reviewed and I don't have a solution, so this is stalled.

jcsteh avatar Dec 07 '20 23:12 jcsteh

In what situations you've experienced nameChange firing more than once for a desktop switch? Assuming that it simply fires more than once for a single desktop switch perhaps we can simply threat nameChange with the same name which occurs in a short time as a duplicate. An alternative venue to investigate might be not to rely on IAccessibe/UIA at all and using VirtualDesktopNotificationService for detecting desktop change

lukaszgo1 avatar Dec 09 '20 17:12 lukaszgo1

The IVirtualDesktopManager can identify a desktop with a GUID, if you give it a window from that desktop. It does not seem to work on the Desktop window, but the foregroundwindow should work:

#include <ShObjIdl.h>
#include <comdef.h>
#include <iostream>

_COM_SMARTPTR_TYPEDEF(IVirtualDesktopManager, __uuidof(IVirtualDesktopManager));


int main()
{
    CoInitializeEx(NULL, COINIT_APARTMENTTHREADED | COINIT_DISABLE_OLE1DDE);
    {
        IVirtualDesktopManagerPtr pvdm;
        _com_error hr = CoCreateInstance(CLSID_VirtualDesktopManager, nullptr, CLSCTX_ALL, IID_PPV_ARGS(&pvdm));

        if (pvdm)
        {
            HWND hDesktop = GetForegroundWindow();
            assert(hDesktop);

            GUID desktopId;
            hr = pvdm->GetWindowDesktopId(hDesktop, &desktopId);
            if (SUCCEEDED(hr.Error()))
            {
                LPOLESTR desktopIdString;
                StringFromCLSID(desktopId, &desktopIdString);

                std::wcout << L"Desktop id: " << desktopIdString << std::endl;

                CoTaskMemFree(desktopIdString);
            }
            else
            {
                std::wcout << L"Failed to query desktop id: " << hr.ErrorMessage() << std::endl;
            }
        }
        else
        {
            std::wcout << L"Failed to create desktop manager: " << hr.ErrorMessage() << std::endl;
        }
    }
    CoUninitialize();
}

This reports a different guid for a different desktop.

There also seems to be an undocumented way to query desktops, but that might not be a good idea to use: https://www.cyberforum.ru/blogs/105416/blog3605.html

learn-more avatar Apr 12 '21 19:04 learn-more

Hi, what we are interested in is the earlier version of Windows 10 that this method will work reliably. Based on what Jamie and others observed, the approach should be reliable on Version 1903 and later – we use name change event handler for this. Thanks.

josephsl avatar Apr 12 '21 19:04 josephsl

Hi, what we are interested in is the earlier version of Windows 10 that this method will work reliably. Based on what Jamie and others observed, the approach should be reliable on Version 1903 and later – we use name change event handler for this. Thanks.

This page suggests that the IVirtualDesktopManager was added in 1511: https://abi-laboratory.pro/compatibility/Windows_8.1_to_Windows_10_1511_10586.494/x86_64/headers_diff/ole32.dll/diff.html

as for the undocumented methods, this version claims to work on 1607: https://github.com/MScholtes/VirtualDesktop/blob/master/VirtualDesktop1607.cs

learn-more avatar Apr 12 '21 20:04 learn-more

Hi, this means, for all practical purposes, Version 1507 is out. I wonder if the suggested approach would work when rearranging virtual desktops as in build 21343 and later. Thanks.

josephsl avatar Apr 12 '21 20:04 josephsl

Hi,

2021 update: given multiple requests to implement this PR, I think for the purposes of getting this merged ASAP, I think it would be best to take one of the following actions:

  1. Ignore my comment on duplicate desktop names. This works best if a purely announcement-only approach is taken. This will allow the PR to work on recent Windows 10 (and Windows 11) releases.
  2. Rethink the PR and simplify it. Currently an equality check is performed, but what if we just let NVDA announce the virtual desktop name only? This is useful on Windows 11 where you can rearrange virtual desktops from Task View.
  3. Take code from my add-on. This is perhaps the simplest solution - since Windows App Essentials add-on announces virtual desktop names, it would be better to take the add-on and put it inside the Desktop NVDA object.

Although the resulting work would be limited to Windows 10 Version 2004 (May 2020 Update) and later, at least we would have something.

Thanks.

josephsl avatar Sep 06 '21 12:09 josephsl

Hi,

After looking into IVirtualDesktopManager, a few things come to my mind:

  1. Suppose each virtual desktop uses a unique GUID. What happens if a Windows 11 user decides to rearrange virtual desktops? Will the GUI's follow the virtual desktop it is assigned to throughout its lifetime, including when the virtual desktop is rearranged? If so, this can cause NVDA to not announce virtual desktop switches at all if the GUID is the same when rearranged (same problem as name checks).
  2. If this PR is picked up, it must use newer NVDA API's such as winVersion.getWinVer() function and control types refactor.

Provided that different GUID's are tied to each virtual desktop, a possible solution is checking GUID's instead of names when announcing virtual desktop switches, allowing NvDA to deal with identically named virtual desktops placed next to each other (provided that GUID's are different). If the virtual desktop/GUID pair stays the same when the virtual desktop is rearranged, then we do need to think about the approach again as it can cause NVDA to not announce the newly rearranged virtual desktop (see above). Given the complexity, I think it would be best to just let NVDA announce all virtual desktop switches and refine desktop switch handler as we go i.e. remove equality check for now.

Also, I have created and updated Jamie's virtual desktop branch on my repo, which includes just the announcement portion, lint updates, and use of newer NVDA API.

Thanks.

josephsl avatar Sep 06 '21 14:09 josephsl

Hi,

Actually, take back my comment on announcement-only implementation: Jamie is correct in that NVDA will announce desktop name each time focus moves, so something other than desktop name should be used to compare virtual desktops. Also, Python thros attribute error as IAccessible handler is initialized way later, so when desktop object is initialized from core.main, it is incomplete.

Thanks.

josephsl avatar Sep 06 '21 14:09 josephsl

Hi,

2022 update: I'm looking into IVirtualDesktopManager route as a follow-up to this PR. For now may I suggest that we drop the duplicate desktop name requirement to get this PR merged ASAP?

CC @JCSTeh, I think it would be best if we can drop the duplicate desktop name requirement i.e. convert this from a draft.

Thanks.

josephsl avatar Jan 10 '22 20:01 josephsl

Hi,

By the way, Jamie, if you don't mind, could you resolve conflicts with master branch (ideally with Microsoft Detours commit added)?

Thanks.

josephsl avatar Jan 10 '22 20:01 josephsl

Hello @jcsteh. Would you be able to give some visibility on this PR as asked by @josephsl :

  • Would you accept to drop the unusual duplicate name requirement?
  • Do you still intend to work on this PR? (resolve conflicts) If yes, do you know when?

Sorry to ask this again, but this feature is regularly asked on mailing lists and it seems blocked only by a corner case. Many thanks.

CyrilleB79 avatar Jan 28 '22 22:01 CyrilleB79

I wasn't concerned about the duplicate name issue in the first place. I think it's a very rare corner case and I personally think we shouldn't let the perfect be the enemy of the good here. This got blocked because others were concerned (which is fair, regardless of my own opinion) and we didn't have a good solution for that.

Unfortunately, I don't have an ETA on when I'll be able to pick this up. It's been a long time since I worked on it, so there's a lot of context I'll need to re-acquire.

jcsteh avatar Jan 28 '22 22:01 jcsteh

@feerrenrut in https://github.com/nvaccess/nvda/pull/11031#issuecomment-649609542, you have asked to take into account the corner case of 2 desktops having the same name. Do you still have this expectation or would you accept to ignore this rare corner case for now and thus allow this PR to go forward?

Anyone else is still concerned by the corner case of 2 desktops having the same name?

Thanks.

CyrilleB79 avatar Jan 30 '22 21:01 CyrilleB79

@CyrilleB79 As long as this situation is considered and will be handled gracefully, IE not cause an error. A note in the userguide would also be good. I doubt this situation would be common.

feerrenrut avatar Feb 03 '22 04:02 feerrenrut

Given https://github.com/nvaccess/nvda/pull/11031#issuecomment-1025239053, I understand that the duplicate desktop name is not a requirement anymore. More specifically, it is expected that the desktop name be reported upon desktop change unless in the rare case of identical names for consecutive desktops; in this spcific case however it should not cause any error.

@jcsteh could you confirm that this PR fulfills this new expectation. If yes, do you still plan to submit it, i.e. resolving the merge conflicts with master branch? Just let us know, thanks. This feature would really be helpful in NVDA.

CyrilleB79 avatar Apr 26 '22 09:04 CyrilleB79

This PR (when it was written) did fulfill that expectation. As I noted above, though, unfortunately, I don't have an estimated time as to when I'll be able to pick this up. It's been a long time since I worked on it, so there's a lot of context I'll need to re-acquire. If someone wants to take over, I'm happy for that to happen. Otherwise, I'll try to look at it if I get some time.

jcsteh avatar Apr 26 '22 10:04 jcsteh

Hi,

September 2022 update: as mentioned in josephsl/wintenapps#77, I'm trying out a different approach thanks to the fact that it is CSRSS (client/server runtime subsystem) that raises a name change event when switching between virtual desktops, which eliminates the "same desktop name" problem I raised earlier. However, in order to bring this to core, NVDA's event handler must somehow recognize that the name change event comes from CSRSS's desktop object representation (note that CSRSS's process handle is 0/NULL as seen by NVDA) whenever handling foreground events, as CSRSS will raise name change event as soon as virtual desktops are switched before others can get a chance to raise focus events (tested with Windows App Essentials and Event Tracker add-ons).

Thanks.

josephsl avatar Sep 22 '22 17:09 josephsl

Hi all,

2023 update: I propose a reimagining of the approach - testing shows that CSRSS method works and is more effective than defining our own desktop object. This means event handler must be notified whenever CSRSS raises name change event. This also means only the newer Windows 10 releases will be supported (will test to pinpoint exactly when the change occurred), meaning Windows 11 will be supported automatically. As an alternative, I might go down the core module patching route via an add-on to experiment with event handler solution and bring the bulk of this PR forward (function names will remain the same but the internals might be different).

Thanks.

josephsl avatar Jan 16 '23 16:01 josephsl

Hi all,

Good news - the earliest release to support CSRSS-based virtual desktop name change event is Windows 10 Creators Update (build 15063), which is ancient by today's standards. Because older releases do not use CSRSS method, it is safe to proceed with CSRSS approach. Note that this means executable names must be checked every time event handler is entered.

Thanks.

josephsl avatar Jan 16 '23 17:01 josephsl