Improve logging and typing for sysListView32#13657
Merged
Merged
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
LeonarddeR
suggested changes
May 3, 2022
Contributor
|
Do we understand why in-process fails in this case? Are you planning to follow this up for 2022.2 perhaps by adding additional logging to the in-process code to fix this properly? |
Contributor
|
According to #13659 (comment), the build of this PR fixes #13659. |
This comment was marked as resolved.
This comment was marked as resolved.
5 tasks
Contributor
|
For reference the docs to LVM_GETCOLUMNORDERARRAY and SendMessage @seanbudd When SendMessage fails, can you use GetLastError to extend the log message. |
feerrenrut
reviewed
May 4, 2022
This comment was marked as resolved.
This comment was marked as resolved.
fb2953f to
9bc6445
Compare
seanbudd
commented
May 4, 2022
This comment was marked as off-topic.
This comment was marked as off-topic.
seanbudd
added a commit
that referenced
this pull request
May 4, 2022
Summary of the issue: When developing #13657, logging NVDAObject.appModule.helperLocalBindingHandle was helpful. NVDAObject._get_devInfo is considered too complex, and should aim to be shortened. Description of how this pull request fixes the issue: Moves the appModule devInfo logging out of NVDAObject._get_devInfo into AppModule.get_devInfo. Adds appModule.helperLocalBindingHandle to the log info for an NVDAObject. Testing strategy: Test with NVDA+F1 on various NVDAObjects.
This comment was marked as off-topic.
This comment was marked as off-topic.
feerrenrut
reviewed
May 6, 2022
feerrenrut
approved these changes
May 6, 2022
Contributor
|
Hi @seanbudd |
cary-rowen
added a commit
to cary-rowen/nvda
that referenced
this pull request
May 24, 2022
This reverts commit f90caa9.
Contributor
|
I found in an application's list Control that NVDA doesn't report list items correctly, |
Member
Author
|
It looks like this PR should have been included in 2022.1 |
seanbudd
added a commit
that referenced
this pull request
May 26, 2022
Fix up of #13560, #13271 Summary of the issue: When out-of-process failed, an empty array was returned instead of None. This could have led to incorrect reporting of list items. For example, trying to get the contents of the second column of the list/table/report, the array is initialized with all values of zero, if used without being filled with valid data all columns will report the same thing. #13271 notes: >How likely are cases where injection doesn't work and the outproc variants succeed? I guess this is pretty unlikely, unless we're running under Windows Store. - If injection doesn't work there will be no `helperLocalBindingHandle`, out of process approach will be tried. - If an application like StartIsBack/StartAllBack doesn't handle the Windows Message properly then it doesn't matter if the message is sent in-process or out-of-process. Description of how this pull request fixes the issue: - When possible, use in process approach to get the column order array. It should be possible if there is a `helperLocalBindingHandle` - Don't return an array filled with zeros, return None when out-of-process fails.
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
8 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:
Fix up of #13560, #13271
Summary of the issue:
When out-of-process failed, an empty array was returned instead of None.
This could have led to incorrect reporting of list items.
For example, trying to get the contents of the second column of the list/table/report, the array is initialized with all values of zero, if used without being filled with valid data all columns will report the same thing.
#13271 notes:
helperLocalBindingHandle, out of process approach will be tried.Description of how this pull request fixes the issue:
helperLocalBindingHandleTesting strategy:
Manual testing:
Confirm affected applications are tested, outlined in #8175 (comment)
Known issues with pull request:
Navigation of menu items using StartIsBack/StartAllBack is still broken #13659.
When navigating through the start menu with the application StartAllBack/StartIsBack installed, a layer is created over file explorer. NVDA uses the file explorer app module, but fetching the column order array with SendMessage fails.
We fetch the column order array (LVM_GETCOLUMNORDERARRAY) with SendMessage. While other related ListView messages are successful, we cannot be determine the correct data without a known column order array.
When this fails, 3 different errors are reported:
We are uncertain why this fails, but expect it is to do with the StartIsBack/StartAllBack dll that is injected into file explorer.
Other messages succeed.
Change log entries:
Not needed, minor fix up without expected user changes.
Code Review Checklist: