Skip to content

fix: lower ITextDocument error log to debugWarning for UnidentifiedEdit controls#20449

Open
Mubashir78 wants to merge 8 commits into
nvaccess:masterfrom
Mubashir78:fix/20428-edit-api-version-negative
Open

fix: lower ITextDocument error log to debugWarning for UnidentifiedEdit controls#20449
Mubashir78 wants to merge 8 commits into
nvaccess:masterfrom
Mubashir78:fix/20428-edit-api-version-negative

Conversation

@Mubashir78

@Mubashir78 Mubashir78 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Fixes #20428

Link to issue number:

#20428

Summary of the issue:

When NVDA encounters an edit control whose API version cannot be determined (UnidentifiedEdit, editAPIVersion = -1), _get_ITextDocumentObject attempts to retrieve a TOM interface via AccessibleObjectFromWindow. When this fails (e.g. for custom edit controls in apps like ApprentiClavier), an OSError is logged at log.error level, producing a visible ERROR entry in the log.

Description of user facing changes:

No user-facing changes. The error is only visible in debug logs.

Description of developer facing changes:

Changed log.error to log.debugWarning in Edit._get_ITextDocumentObject so that the expected failure to retrieve TOM on non-TOM edit controls is logged at debug level instead of error level.

Description of development approach:

The editAPIVersion != 0 guard is preserved so that UnidentifiedEdit (which sets editAPIVersion = -1) still attempts TOM retrieval as designed. The only change is the log severity: errordebugWarning, since the TOM retrieval failure is an expected condition for these controls and has no diagnostic value at error level.

Testing strategy:

Manual testing: verified that the OSError no longer appears at ERROR level in the NVDA log when interacting with edit fields where TOM is unavailable. The message now appears at DEBUGWARNING level when debug logging is enabled.

Known issues with pull request:

None.

Code Review Checklist:

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

@Mubashir78 Mubashir78 requested a review from a team as a code owner July 5, 2026 16:40
@Mubashir78 Mubashir78 requested a review from SaschaCowley July 5, 2026 16:40
@CyrilleB79

Copy link
Copy Markdown
Contributor

Hi. You have unrelated commits here, e.g. from (#20159).

…oid OSError log

Fixes nvaccess#20428

When editAPIVersion is -1 (UnidentifiedEdit), _get_TextInfo attempts to
use ITextDocumentTextInfo because the guard checked != 0. Changing to > 0
correctly excludes -1 as well, falling back to EditTextInfo without ever
calling AccessibleObjectFromWindow (which would raise OSError).

Also replace WindowsError with OSError in _get_ITextDocumentObject's
except clause, since WindowsError is an alias for OSError on Python 3.3+
and may be removed in future Python versions.
@Mubashir78 Mubashir78 force-pushed the fix/20428-edit-api-version-negative branch from 68ab3c9 to 131caf2 Compare July 5, 2026 20:59
@Mubashir78

Copy link
Copy Markdown
Contributor Author

Thanks, rebased onto master.

@seanbudd

seanbudd commented Jul 8, 2026

Copy link
Copy Markdown
Member

Hi - please read our contributing guidelines and fill out the PR template. I've added it back to the PR for you.

@seanbudd seanbudd marked this pull request as draft July 8, 2026 02:25
@Mubashir78 Mubashir78 marked this pull request as ready for review July 8, 2026 06:03
@Mubashir78

Copy link
Copy Markdown
Contributor Author

Thanks for the guidance. I've filled out the PR template and added changelog entries under Changes for Developers. Marked as ready for review.

Comment thread user_docs/en/changes.md Outdated

@seanbudd seanbudd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @Mubashir78

@CyrilleB79

Copy link
Copy Markdown
Contributor

@Mubashir78 you write:

Manual testing: verified that the OSError no longer appears in the NVDA log when interacting with edit fields where editAPIVersion is -1.

Where did you find an application or a situation where editAPIVersion is -1? Can you be more specific? It's worth noting that @SaschaCowley's question in #20428 (comment) has not yet been answered.

@seanbudd I'd recommend to wait to clarify which real life use case we are trying to solve here before merging; more, this would allow to add an appropriate change log item mentioning #20428.

@Mubashir78

Mubashir78 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review, @CyrilleB79.

The editAPIVersion -1 case comes from UnidentifiedEdit (source/NVDAObjects/window/edit.py:1119), which is used when NVDA encounters an edit control whose API version cannot be determined. This happens in the window tree walk in NVDAObjects/window/init.py where UnidentifiedEdit is appended to the class list as a fallback.

Since the code was using != 0 instead of > 0 before my fix, any unrecognized edit control hitting UnidentifiedEdit would attempt to use ITextDocumentTextInfo and log an OSError during the failing AccessibleObjectFromWindow call. I do not have a specific application to reproduce this, but the bug is clear from the code -- the != 0 guard is incorrect for a tri-state value where -1 means unknown. The fix is a defensive correctness improvement regardless of visible user impact.

@CyrilleB79

Copy link
Copy Markdown
Contributor

If you have no specific application to test this, my questions still stand:

  • which real-life use case are we trying to solve?
  • how did you actually test this? You need to be more specific in your test descriptions so that I or someone else can replicate your test. For now, I'm not able to find an edit field where editAPIVersion is -1; you need to tell me where I can find one to test. Thanks.

@Mubashir78

Mubashir78 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

which real-life use case are we trying to solve?

The real-life application is ApprentiClavier 1.10 by Emmanuel Drouet (reported in #19245). The error was logged when pressing Enter repeatedly in that software.

how did you actually test this?

Launch ApprentiClavier 1.10 and press Enter several times with NVDA debug logging enabled. Check the log for:

ERROR - NVDAObjects.window.edit.Edit._get_ITextDocumentObject
Error getting ITextDocument
OSError: [WinError -2147467259] Erreur non spécifiée

Alternatively, as the original issue reporter noted in #20428, you can set focus.editAPIVersion = -1 via the NVDA Python console on any standard edit control (e.g. Notepad) to simulate UnidentifiedEdit, then navigate text -- the same error is logged without the fix and absent with it.

@CyrilleB79

Copy link
Copy Markdown
Contributor

Thanks for these additional details. This opens some more questions, as asked in #19245 (comment).

If this PR actually improves the user experience with Apprenti Clavier, restoring pre-2026.1 behavior, you should add a corresponding change log item describing it, with a reference to #19245.

If this PR only removes an error with no other visible effect, the question is: Do these error messages actually need to be filtered out? @seanbudd seems to say in #19245 (comment) that there is an issue with the Apprenti Clavier application; if it is confirmed, I do not understand why we would need to ignore error messages.

@Mubashir78

Copy link
Copy Markdown
Contributor Author

@CyrilleB79 The guard != 0 was a bug -- it called a COM API with parameters known to fail on UnidentifiedEdit controls, purely because -1 != 0. The log message is not diagnostic data; it's the predictable consequence of a buggy condition that fires on every custom edit control. If the ApprentiClavier issue needs debugging, it should be investigated directly, not via this generic OSError. Added changelog referencing #19245.

@CyrilleB79

Copy link
Copy Markdown
Contributor

@Mubashir78, so do you confirm that this PR does not change anything in the usage of ApprentiClavier?

you have added a change log about an error no longer being logged.
Given error sounds are disabled by default in stable releases, I do not call it a visible change / bugfix and so would not mention it in the change log; you may want to wait for NV Access (@seanbudd) opinion though.

@Mubashir78

Mubashir78 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

@CyrilleB79 Yes, no change to ApprentiClavier usage, only log cleanup. Understood on the changelog concern. I'm gonna wait for @seanbudd's opinion.

@CyrilleB79

Copy link
Copy Markdown
Contributor

Thanks for your patience @Mubashir78 with all my requests!
I just wanted this PR to be documented and reviewable as well as possible.

Another point: In the same file, I find the following comment:
# Note that unidentified rich edit fields will most likely use L{ITextDocumentTextInfo}.

This seems strange to me. Digging with git blame, I've found that all this system with the introduction of UnidentifiedEdit has been introduced with #8102 / #8165.

@LeonarddeR could you please have a look at this PR and its impact? Thanks!

@Mubashir78

Mubashir78 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

@CyrilleB79 Its all good, your thoroughness made the PR better. Appreciate you taking the time to dig into the history and loop in @LeonarddeR 👍

@Mubashir78

Copy link
Copy Markdown
Contributor Author

@CyrilleB79 @LeonarddeR I looked deeper into the repo and found the docstring in UnidentifiedEdit (line 1119):

class UnidentifiedEdit(RichEdit):
    """
    An edit control for which the edit API version is unknown.
    This class inherrits from L{RichEdit} to ensure L{ITextDocumentTextInfo} initialization failure is handled correctly.
    """
    editAPIVersion = -1

This tells me the original intent was clear: try TOM and let RichEdit's error handling catch failures. The != 0 guard was correct per that design.

But in practice both paths end at EditTextInfo. The != 0 path just adds a failing COM call and an ERROR log that users reported as a regression (#20428). The error has no diagnostic value: it only confirms "no TOM," which is already the defining characteristic of UnidentifiedEdit.

I think the PR should be merged. The failing COM call is useless overhead and the log confuses users. Up to you both though.

@LeonarddeR

Copy link
Copy Markdown
Collaborator

I'm afraid I can't accept this pr as it stands.

I introduced UnidentifiedEdit for a reason in #8165 . The current pr is likely to break ITextDocumentTextInfo usage in the Notetab editor.
Is there anything against lowering the level of the logger call from error to debugWarning?

@Mubashir78

Mubashir78 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

@LeonarddeR Actually yeah you are right. I reworked the PR per your suggestion. I reverted the guard back to != 0 and changed log.error to log.debugWarning in _get_ITextDocumentObject instead. This keeps TOM attempted for UnidentifiedEdit (Notetab still works) while no longer logging an ERROR when it fails. I guess now that i think about it, that's the ideal course of action, aka the middle ground of debug logging. Fixes #20428.

@LeonarddeR

Copy link
Copy Markdown
Collaborator

Thanks for this.

I wonder waht @CyrilleB79 and @seanbudd think about the changelog now. Errors in the log are silent on release builds unless explicitly opted in, so for non-power users, this is an invisible fix.

@CyrilleB79

Copy link
Copy Markdown
Contributor

This PR, as it is now, just changes a log.error to a log.debugWarning.

I have still the same opinion as what I wrote in #20449 (comment): this PR as it is now does not deserve a change log item since there won't be any visible change for users of stable releases.

TTechnically, this PR fixes #19245 because it transforms an error into a debugWarning. But in my opinion, it would be much interesting to understand why this error pops up from NVDA 2026.1 onwards (in comparison with 2025.x versions) before lowering the log level of the logged message. Why is there an issue now and not with 2025.x?

@Mubashir78

Copy link
Copy Markdown
Contributor Author

@CyrilleB79 here is what I found looking at the history:

PR #8165 introduced UnidentifiedEdit specifically for editors like Notetab and Delphi apps with custom window classes. The design was: try TOM first, fall back to EditTextInfo on failure. The editAPIVersion = -1 was chosen so the != 0 guard would let it through to attempt TOM.

In 2025.x (32-bit), AccessibleObjectFromWindow with OBJID_NATIVEOM on these controls likely returned None or raised COMError, which was caught and the error logged. Users didn't see it because the 32-bit COM layer handled the failure differently.

In 2026.1 (64-bit migration), AccessibleObjectFromWindow raises OSError (E_FAIL, WinError -2147467259 from _ctypes/callproc.c:GetResult). The except clause was extended to catch OSError, but at log.error level, making the failure visible in logs for the first time (as reported in #19245 and #20428).

So the root cause is the 64-bit migration changing the COM error type from COMError to OSError. The log.debugWarning change just restores the pre-64-bit experience where this non-critical failure wasn't alarming to users.

@seanbudd seanbudd self-requested a review July 9, 2026 04:50
Comment thread user_docs/en/changes.md Outdated
@seanbudd

Copy link
Copy Markdown
Member

please update the PR description and title to reflect the changes

@seanbudd seanbudd marked this pull request as draft July 10, 2026 04:08
@Mubashir78 Mubashir78 marked this pull request as ready for review July 10, 2026 07:33
@Mubashir78

Mubashir78 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Updated the PR title and description to: changing log.error to log.debugWarning instead of modifying the editAPIVersion guard.

@Mubashir78 Mubashir78 changed the title Skip TOM text info when editAPIVersion is -1 to avoid OSError log fix: lower ITextDocument error log to debugWarning for UnidentifiedEdit controls Jul 10, 2026
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.

When edit API version is -1, an exception is raised when unable to use ITextDocumentTextInfo

4 participants