Skip to content

Windows 10 Mail: ensure browse mode is used in the reading pane. - #12320

Closed
michaelDCurran wants to merge 2 commits into
masterfrom
i12117
Closed

Windows 10 Mail: ensure browse mode is used in the reading pane.#12320
michaelDCurran wants to merge 2 commits into
masterfrom
i12117

Conversation

@michaelDCurran

@michaelDCurran michaelDCurran commented Apr 21, 2021

Copy link
Copy Markdown
Member

Link to issue number:

Fixes #12117

Summary of the issue:

In Windows 10 Mail, a Microsoft Word document control is used to display content of received emails and emails currently being composed. In NVDA 2020.4, NVDA would use browse mode for reading emails, but not for writing emails.
However, after merging of pr #12051 browse mode is no longer used by default when reading emails. This is because the base Microsoft Word document NVDAObject now creates a TreeInterceptor all the time, but set to focus mode, so that elements list is always available in Microsoft Word.
But as the hxMail implementation assumed browse mode would be available for the TreeInterceptor always, and only created the TreeInterceptor in the reading pane, Windows 10 mail ended up getting no treeInterceptor for writing email (ok) but for reading email it got a treeInterceptor but set to focus mode (not okay).

Description of how this pull request fixes the issue:

In the hxMail appModule: rather than overriding shouldCreateTreeInterceptor, expose an isInReadingPane property, and in the treeInterceptor's treeInterceptor_gainFocus event, correctly set browse mode or focus mode based on whether we are now in the reading pane, and if we were or not before.
Aso the overriding of the treeInterceptor's isAlive property has been removed, as the treeInterceptor should now stay around as long as that physical control exists.
Note that Mail uses the same physical instance of the same control for both reading and composing mail.

Testing strategy:

Opened Windows 10 mail. Opened a received email, and verified that the arrow keys could again be used to navigate / read the content of the email message and that quick navigation was available. Then pressed control+n to create a new message, tabbed to the message document, and ensured that

Known issues with pull request:

None known.

Change log entries:

None needed.

Code Review Checklist:

  • Pull Request description is up to date.
  • Unit tests.
  • System (end to end) tests.
  • Manual tests.
  • User Documentation.
  • Change log entry.
  • Context sensitive help for GUI changes.

@michaelDCurran
michaelDCurran requested a review from a team as a code owner April 21, 2021 23:57
@michaelDCurran michaelDCurran added this to the 2021.1 milestone Apr 21, 2021
@josephsl

josephsl commented Apr 22, 2021 via email

Copy link
Copy Markdown
Contributor

@LeonarddeR

Copy link
Copy Markdown
Collaborator

I'm pretty sure Outlook with UIA enabled also suffers from this. cc @bramd

@michaelDCurran

michaelDCurran commented Apr 22, 2021 via email

Copy link
Copy Markdown
Member Author

@LeonarddeR

Copy link
Copy Markdown
Collaborator

I looked at the code and honestly, I find it a little hacky. Basically what's done in #12051 is enforcing the word tree interceptor to start in focus mode and then this pr tries hard to circumvent that. I'd rather see something like moving the init logic that was added in #12051 (i.e. the part copied from ExcelBrowseModeTreeInterceptor) to BrowseModeTreeInterceptor:

	def __init__(self,rootNVDAObject):
		super().__init__(rootNVDAObject)
		if self.disableAutoPassThrough:
			self.passThrough = True
			reportPassThrough.last = True

Then, this init override can be removed for excel and word. For Excel and Word, disableAutoPassThrough should still be set to True, whereas in the outlook appModule, it should be restored back to False.

@LeonarddeR

Copy link
Copy Markdown
Collaborator

AH, I missed the fact that the interceptor is used for both reading and writing in my proposal. Also it doesn't seem to be cleaned up correctly, right?
Still, I hope it will inspire you somehow.

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There are also some unanswered comments/questions here also from @LeonarddeR who seems much more familiar with this change.

_wasInReadingPane: bool = False

def event_treeInterceptor_gainFocus(self):
isInReadingPane = self.rootNVDAObject.isInReadingPane

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This assumes that rootNVDAObject will always be a MailWordDocument. If I'm understanding this right, while unlikely, a developer may set this as the TreeInterceptor on a different object, resulting in it being constructed with a different type as the rootNVDAObject.

This took me quite some time to determine, typing would help, maybe something like this at class level:

rootNVDAObject: MailWordDocument

def __init__(self, obj:MailWordDocument):
		if not isinstance(obj, MailWordDocument):
				log.error("MailWordDocumentTreeInterceptor depends on rootNVDAObject being a MailWordDocument see event_treeInterceptor_gainFocus")
		super().__init(obj)


def _get_isAlive(self):
return super(MailWordDocumentTreeInterceptor,self).isAlive and self.rootNVDAObject.shouldCreateTreeInterceptor
_wasInReadingPane: bool = False

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this be initialized to False? Currently this will only be accurate after the gain focus event is fired the first time. Could this be initialized correctly in the __init__ method instead.

Could a situation arise where the object is created while focus is already in a readingpane? Thus no initial fire of the gainfocus to set this to True, but rendering it inaccurate?

@michaelDCurran

Copy link
Copy Markdown
Member Author

Closing this in favor of pr #12365 which reverts pr #12051.
A cleaner solution should be worked out as Outlook was also affected, not just Windows 10 Mail.

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.

nvda is not reading line by line emails when using the native windows 10 email application

4 participants