Skip to content

No longer override isAlive on UIA WordBrowseModeDocument#11885

Merged
michaelDCurran merged 3 commits into
nvaccess:masterfrom
LeonarddeR:isAlive
Dec 1, 2020
Merged

No longer override isAlive on UIA WordBrowseModeDocument#11885
michaelDCurran merged 3 commits into
nvaccess:masterfrom
LeonarddeR:isAlive

Conversation

@LeonarddeR

Copy link
Copy Markdown
Collaborator

Link to issue number:

#11741

Summary of the issue:

On WordBrowseModeDocument, isAlive was overridden in a way that it would always return True. This would block a clean up of the interceptor.

Description of how this pull request fixes the issue:

  1. No longer override isALive
  2. Always log at the error level when a tree interceptor couldn't be updated. Before, exception translated to debugWarning since the underlying was a COMError. If this was error from the start, we probably would have found this issue way earlier.

Testing performed:

Tested that #11741 no longer occurs. When the TI wasn't killed correctly, it generated an update error that caused speech to cancel somehow.

Known issues with pull request:

  1. I see no reason why the base isAlive wouldn't work. Before accepting this though, it would be helpful to know whether @michaelDCurran remembers something about this.
  2. It would be interesting to know why this underlying bug impacted cancellable speech, as I really don't know and therefore I'm not sure whether there's another cause for Cancellable speech: when going from one Explorer window to another, the title is discarded #11741.

Change log entry:

Probably not worth mentioning

@LeonarddeR LeonarddeR changed the title No longer override isAlive on WordBrowseModeDocument No longer override isAlive on UIA WordBrowseModeDocument Dec 1, 2020

@michaelDCurran michaelDCurran 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.

I think the reason why isAlive was originally overridden like that was simply just that UIABrowseMode and some of the other base classes did not yet exist when starting to write the UIA WordDocument support, and I think I just needed something that worked at the time.
Having tested this a bit myself, I believe what you have done is the right approach.
However, I have gone ahead and added exc_info=True to the log.error in api.py as it is important to be able to still see the actual traceback.

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.

3 participants