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

Fix-up of #9843: Fixed an error preventing a failed copy operation from being reported [NVDA 2020.4beta1] #11959

Merged
merged 2 commits into from
Dec 22, 2020

Conversation

CyrilleB79
Copy link
Collaborator

PR made against beta branch since the PR it is fixing is not yet part of a stable release.
Cc @feerrenrut

Link to issue number:

None
Fix-up of #9843.

Summary of the issue:

When using api.copyToClip with new argument notify=True, if any call to the clipboard API fails, "Unable to copy" message should be reported since the merge of #9843. This is however not the case.
It was discovered during my computer stress tests (for unrelated reasons). The stress tests consists in keeping NVDA+T pressed during a long time (30 seconds or one minute) in MS Word.
After a while, the clipboard is overflowed by API operations and become inaccessible. The following error is reported in the log:

IO - inputCore.executeGesture (11:15:19.230) - winInputHook (11196):
Input: kb(desktop):NVDA+t
ERROR - scriptHandler.executeScript (11:15:19.190) - MainThread (13740):
error executing script: <bound method GlobalCommands.script_title of <globalCommands.GlobalCommands object at 0x06ADD6F0>> with gesture 'NVDA+t'
Traceback (most recent call last):
File "api.pyc", line 317, in copyToClip
File "contextlib.pyc", line 112, in __enter__
File "winUser.pyc", line 758, in openClipboard
PermissionError: [WinError 5] Accès refusé.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
File "scriptHandler.pyc", line 208, in executeScript
File "globalCommands.pyc", line 1689, in script_title
File "api.pyc", line 321, in copyToClip
TypeError: catching classes that do not inherit from BaseException is not allowed

As per #9843, "Unable to copy" should be reported in this case and no error should occur.

Description of how this pull request fixes the issue:

In api.copyToClip, caught OSError instead of unappropriated ctypes.WinError which is a function.

I have seen in NVDA code (probably Python 2 legacy code) that we use WinError in this case.
Anyway, Python doc tells us to use OSError:

ctypes.WinError(code=None, descr=None)
Windows only: this function is probably the worst-named thing in ctypes. It creates an instance of OSError. If code is not specified, GetLastError is called to determine the error code. If descr is not specified, FormatError() is called to get a textual description of the error.
Changed in version 3.3: An instance of WindowsError used to be created.

In any case, WinError and OSError are the same.

Testing performed:

Tested with the same stress test that the failing copy is reported. Since many messages are reported and interrupted, I have heard only the beginning of the message that differs from the message reporting a successful copy. I have then checked this message in the log.

Known issues with pull request:

None

Change log entry:

Not needed since #9843 is not yet part of a stable release.

Cc @JulienCochuyt (author of #9843).

@lukaszgo1
Copy link
Contributor

@CyrilleB79 there are some unneeded commits here.

@CyrilleB79
Copy link
Collaborator Author

@CyrilleB79 there are some unneeded commits here.

Thanks @lukaszgo1. The unneeded commits were coming from the rebase onto beta that I had not correctly done.
I have fixed it now and force-pushed the new branch.

@JulienCochuyt
Copy link
Collaborator

@CyrilleB79: I indeed carried over the previous construct without realizing it was wrong. Good catch, thanks!

Copy link
Contributor

@feerrenrut feerrenrut left a comment

Choose a reason for hiding this comment

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

Thanks @CyrilleB79

@feerrenrut feerrenrut merged commit 4a451d0 into nvaccess:beta Dec 22, 2020
@nvaccessAuto nvaccessAuto added this to the 2021.1 milestone Dec 22, 2020
@feerrenrut feerrenrut modified the milestones: 2021.1, 2020.4 Dec 22, 2020
@CyrilleB79 CyrilleB79 deleted the ctypesWinError branch December 27, 2020 22:05
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.

None yet

5 participants