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

UI Automation in Windows Console: only enable UIA when available #9650

Merged
merged 5 commits into from
Jun 5, 2019

Conversation

codeofdusk
Copy link
Contributor

Link to issue number:

Split from #9646 (builds on #9614).

Summary of the issue:

Currently, in consoles with UI Automation enabled:

  • If the advanced setting "force UI Automation in the Windows Console" is enabled on a system without a UIA console (such as Windows versions below Windows 10 1709 or those with the "use legacy console" option selected), the console is completely inaccessible.

Description of how this pull request fixes the issue:

The legacy console code has been moved to NVDAObjects.IAccessible.winConsoleLegacy, and the check for consoles has been moved from NVDAObjects.window.findOverlayClasses to NVDAObjects.IAccessible.findOverlayClasses. Since NVDA prefers UIA over MSAA, UIA is used when enabled and available and legacy is used otherwise.

Testing performed:

Tested all combinations of the "use legacy console" and UIA checkboxes on Windows 10 versions 1803 and 1903.

Known issues with pull request:

See #9646 for all UIA console known issues to date.

Change log entry:

None.

@codeofdusk
Copy link
Contributor Author

@LeonarddeR
Copy link
Collaborator

The legacy console code has been moved to NVDAObjects.IAccessible.winConsoleLegacy.

Why do you consider this necessary? This breaks all add-ons that somehow use NVDAObjects.window.winConsole.

and the check for consoles has been moved from NVDAObjects.window.findOverlayClasses to NVDAObjects.IAccessible.findOverlayClasses.

This change makes much sense.

source/core.py Outdated
import winConsoleHandler
log.debug("Initializing winConsole support")
winConsoleHandler.initialize()
import winConsoleHandlerLegacy
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you rename this as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid confusion (in anticipation of UIA becoming the default).

@michaelDCurran
Copy link
Member

michaelDCurran commented May 31, 2019 via email

Copy link
Member

@michaelDCurran michaelDCurran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again like with NVDAObjects.window.winConsole, can you please provide modules for winConsoleHandler and winCon with warnings when imported?
At the top of each you can just import everything from their respective new module. E.g for wincon:

from winCon import *

@@ -528,6 +528,9 @@ def findOverlayClasses(self,clsList):
elif windowClassName.startswith("Chrome_"):
from . import chromium
chromium.findExtraOverlayClasses(self, clsList)
elif windowClassName == "ConsoleWindowClass":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than placing the code directly here, you can just place an entry in the _staticMap directory in this file. But as well as the window class, also check for an MSAA role of ROLE_SYSTEM_CLIENT. This will ensure that the subclass is only used for the actual client of the window, and not the system menu or scrollbars etc.
Something like:

("ConsoleWindowClass",oleacc.ROLE_SYSTEM_CLIENT):"winConsoleLegacy.WinConsoleLegacy",

import api
import core

class winConsoleLegacy(Terminal, EditableTextWithoutAutoSelectDetection, IAccessible):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is a class, please use a capital W, I.e. WinConsoleLegacy.

# Copyright (C) 2019 Bill Dengler

import warnings
from ..IAccessible.winConsoleLegacy import winConsoleLegacy as WinConsole
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When capitalizing WinConsoleLegacy, don't forget to change it here.

from ..IAccessible.winConsoleLegacy import winConsoleLegacy as WinConsole

warnings.warn("NVDAObjects.window.winConsole is deprecated. Use NVDAObjects.IAccessible.winConsoleLegacy or NVDAObjects.UIA.winConsoleUIA instead",
DeprecationWarning, stacklevel=3)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a blank line at the end of this file.

@codeofdusk codeofdusk force-pushed the cmduia2-move-legacy branch from 79c2458 to c47f165 Compare June 3, 2019 16:17
@codeofdusk
Copy link
Contributor Author

@michaelDCurran Interestingly, this one merged without conflicts!

@michaelDCurran
Copy link
Member

like @LeonarddeR I am a little concirned having so many modules renamed, even though of course we do have backwards compatibility wrapper modules with deprecation warnings.
We are trying to keep changes on master fairly small, at least as far as breaking add-ons is concirned.
I think that it might be better if we don't rename winCon and winConsoleHandler. And rather than moving all of NVDAObjects.window.winConsole to NVDAObjects.IAccessible.winConsoleLegacy, we just create an NVDAObjects.IAccessible.winConsole which contains one class which is WinConsole that inherits from both NVDAObjects.window.winConsole.WinConsole and IAccessible. Very much like what I did with winWord.
Failing this, we could merge this pr straight to threshold, making the required renames, but in that case we wouldn't have to worry about the backwards compatibility modules.
But then master would not be able to take advantage of UIAWinConsole working instantly after turning it on in the Advanced settings pannel.
I'd like to get an opinion from @LeonarddeR on this.
Sorry for not being as clear on this previously.

@LeonarddeR
Copy link
Collaborator

LeonarddeR commented Jun 5, 2019 via email

@codeofdusk
Copy link
Contributor Author

@LeonarddeR @michaelDCurran Is this better?

@michaelDCurran michaelDCurran merged commit d1db21a into nvaccess:master Jun 5, 2019
@nvaccessAuto nvaccessAuto added this to the 2019.3 milestone Jun 5, 2019
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

Successfully merging this pull request may close these issues.

4 participants