Skip to content

Add significantly more debug logging to UIAHandler #14256

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

Merged
merged 8 commits into from
Oct 23, 2022
Merged

Conversation

michaelDCurran
Copy link
Member

Link to issue number:

None.

Summary of the issue:

Although NVDA has a specific uIA debug logging category, this does not really enable all that many more log messages a part from some debug warnings. thus it is almost useless to ask for a user's log with this flag enabled.

Description of user facing changes

Setting NVDA's log level to debug, and enabling the UIA debug logging category in NVDA's advanced settings panel, will now produce significantly more logging for UIA. This includes logging when all UIA event handler methods are called, along with their parameters, and logging for utility utility methods such as checking if a window or element is native UIA.

Description of development approach

Added a bunch of methods to UIAHandler for generating a string representation for debugging, for things such as UIA elements, property IDs, event IDs, notification types etc.
Added conditional debug logging to all UIA event handler methods, plus isUIAWindow, isNativeUIAElement and getNearestWindowhandle.

Testing strategy:

Ran NVDA for a day or so, performing general tasks, using multiple apps that are native UIA.
Used the new debug logging to aide in investigating QT issues.

Known issues with pull request:

None known.

Change log entries:

New features
Changes
Bug fixes
For Developers

  • the UIA debug logging category when enabled now produces significantly more logging for UIA event handlers and utilities.

Code Review Checklist:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • API is compatible with existing add-ons.
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • Security precautions taken.

@michaelDCurran michaelDCurran requested a review from a team as a code owner October 17, 2022 00:21
@josephsl
Copy link
Collaborator

josephsl commented Oct 17, 2022 via email

@@ -1292,6 +1302,11 @@ def __init__(self,windowHandle=None,UIAElement=None,initialUIACachedPropertyIDs=
UIACachedWindowHandle=UIAElement.cachedNativeWindowHandle
self.UIAIsWindowElement=bool(UIACachedWindowHandle)
if not windowHandle:
if UIAHandler._isDebug():
log.debug(
f"No windowHandle for UIA NvDAObject. "
Copy link
Member

@seanbudd seanbudd Oct 17, 2022

Choose a reason for hiding this comment

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

Fix capitalization of NVDA

Suggested change
f"No windowHandle for UIA NvDAObject. "
"No windowHandle for UIA NVDAObject. "

Copy link
Member

Choose a reason for hiding this comment

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

this is still an issue, same with the unnecessary f-strings.
However, as the remaining changes are trivial, I am approving in the mean-time

@seanbudd
Copy link
Member

Mostly trivial changes remaining

michaelDCurran and others added 2 commits October 17, 2022 11:05
Co-authored-by: Sean Budd <sean@nvaccess.org>
@AppVeyorBot
Copy link

See test results for failed build of commit a634d7edaa

@michaelDCurran
Copy link
Member Author

@seanbudd I've changed all unnecessary f-strings to regular strings, and also added a missing isDebug argument to the call to _isUIAWindowHelper. So another review please.

@AppVeyorBot
Copy link

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • PASS: Lint check.
  • Build execution time has reached the maximum allowed time for your plan (60 minutes).

See test results for failed build of commit 536be0f087

@AppVeyorBot
Copy link

See test results for failed build of commit b1238b2a9e

@michaelDCurran michaelDCurran merged commit 12803e9 into master Oct 23, 2022
@michaelDCurran michaelDCurran deleted the UIADebugging branch October 23, 2022 03:18
@nvaccessAuto nvaccessAuto added this to the 2023.1 milestone Oct 23, 2022
michaelDCurran added a commit that referenced this pull request Oct 31, 2022
…ot be created. (#14314)

Fixes regression introduced in pr #14256

Summary of the issue:
Sometimes a UIA focus event handler can't create an NVDAObject for the element being focused. Normally, in this situation, the event would be silently dropped.
However, with the introduction of extra debugging for UIA events in pr #14256, a return was accidently removed, which meant that the shouldAllowUIAFocusEvent attribute tried to be looed up on the None (failed NVDAObject). This caused a noisy traceback in the log at level error.

Description of user facing changes
A noisy error traceback is no longer produced when NVDA cannot handle certain broken UIA focus events.

Description of development approach
Make sure to return if instantiating the NVDAObject produces None.
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.

5 participants