UI Automation in Windows Console: Remove "Text area", replace isAtLeastWin10, and code cleanup#9761
Merged
Merged
Conversation
Contributor
Author
michaelDCurran
requested changes
Jun 17, 2019
michaelDCurran
left a comment
Member
There was a problem hiding this comment.
I would like to see this pr just focus on actual code changes and comment clarification, rather than code style.
I.e.
- Suppressing the name of the control
- Changing the windows 10 logic
- Renaming WinConsoleUIA
- Clarifying comments
This makes it a lot easier to review and test.
Plus, some of your style changes I find hard to agree with. For example, moving the final bracket up onto the line with the last argument. That is not consistant with other functions/lists split over multiple lines.
Perhaps we can revisit code style later on in this project, but if there was a major issue with styling in the first place, the previous prs would not have been approved. Getting style right in the first place is very good practice, but changing it after the fact tends to introduce bugs and makes it harder to track the history of lines in commits. Though this is just my personal opinion :)
…plicitly clear the word buffer when flushing queued chars.
de686c5 to
eca4a44
Compare
michaelDCurran
approved these changes
Jun 17, 2019
Member
|
Have you ran with this pr (with the most recent changes) for a while? I just want to ensure this has had enough testing before I merge it. |
Contributor
Author
I've tested on my machine (during development) and everything seemed fine to me... |
Contributor
Author
|
@michaelDCurran Could you please update what's new for these PRs? (remove the old entry from 2019.2 changes and add the one from this description) |
michaelDCurran
added a commit
that referenced
this pull request
Jun 18, 2019
Contributor
Author
|
@michaelDCurran Please move the change from bug fixes to changes for developers. Thanks. |
Member
|
I am confused. I placed it under Changes for Developers for 2019.3 in
changes.t2t.
Are you seeing something different?
Oh, there was a missing blank line... perhaps it was that.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to issue number:
Builds on #9614.
Summary of the issue:
Currently:
Trueif the currently-running version is at least the supplied one. For example, this makes it difficult to enable UIA consoles for Windows 10 1809 and later except 1903.Description of how this pull request fixes the issue:
isAtLeastWin10function has been renamed toisWin10. It now takes an optionalatLeastkeyword argument, which defaults toTrue. The caret movement workarounds have been explicitly enabled for 1903 (not 1903 and later) in anticipation of a possible future Microsoft fix.speech.curWordCharsis once again explicitly cleared whenscript_flushQueuedCharsis called.winConsoleUIA.winConsoleUIAhas been renamed towinConsoleUIA.WinConsoleUIAto remain consistent with other NVDA classes.Testing performed:
Tested that the console is still functional and that
isWin10returns expected results on Windows 10 1903.Known issues with pull request:
None.
Change log entry:
== Changes for Developers ==