Fix adobe reader int/str to NoneType comparisons as well as other identified cases - #9881
Conversation
|
I can test it too.
|
|
This specific bug has been fixed. However after pressing CTRL+Page Down error sound is played with each key press, NVDA needs to be killed, and the following is in the log multiple times: |
|
Ugh, another one. Makes sense to fix this as part of this one as well.
|
|
Thanks for reporting again @lukaszgo1. Could you have another try? |
|
This time error is as follows: |
|
Ugh, my bad. Just did another push. |
|
Now it works. Thanks @LeonarddeR |
feerrenrut
left a comment
There was a problem hiding this comment.
All looks good, but a question about logging the caught exception.
| try: | ||
| return normalizeStdName(self.pdDomNode.GetStdName())[0] | ||
| except (AttributeError, LookupError, COMError): | ||
| except (AttributeError, LookupError, COMError, TypeError): |
There was a problem hiding this comment.
This means that it will hide any other TypeErrors. I think this should instead log at Debug / Debug warning.
| return normalizeStdName(self.pdDomNode.GetStdName())[0] | ||
| except (AttributeError, LookupError, COMError, TypeError): | ||
| pass | ||
| log.debugWarning("Could not get role for AcrobatNode using normalizeStdName", exc_info=True) |
There was a problem hiding this comment.
Will this be noisy? Are regular LookupError or AttributeError exceptions expected?
There was a problem hiding this comment.
Hmm, good point. Do you want me to only log this for type errors? I'd say that this could still be noisy though. May be I should avoid raising the typeerror altogether.
Link to issue number:
Fixes #9859
Summary of the issue:
Reading documents in Adobe Reader raises a TypeError
Description of how this pull request fixes the issue:
Fixed this int to NoneType comparison, identified some others while at it. The following commands where used:
I did not touch comparisons for IAccessibleChildID, as I'm 99% sure that this will never be None.
Testing performed:
T.b.d. @lukaszgo1 would you be able to test your case with Acrobat?
Known issues with pull request:
one
Change log entry:
None