Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a appModule for taskMan, to fix some bugs in windows ten. #8147

Merged
merged 2 commits into from Apr 30, 2018

Conversation

Projects
None yet
7 participants
@derekriemer
Copy link
Collaborator

derekriemer commented Apr 8, 2018

Link to issue number:

None

Summary of the issue:

In windows ten, task manager has a bug where the columns for CPU usage, memory, and other information about a process are UIA layout, not content. This fix makes these columns UIA content, so NVDA shows them when using simple review mode.

Testing performed:

Built NVDA, installed it, and ran it on task manager on windows ten, version 1709.

Known issues with pull request:

It might or might not work on 8 and 7, I need someone to test this if possible.### Change log entry:

  • Bug fixes:
    • Fix a bug in task manager causing NVDA to not allow users to access the contents of specific details about processes.

@derekriemer derekriemer requested a review from leonardder Apr 8, 2018

@derekriemer

This comment has been minimized.

Copy link
Collaborator Author

derekriemer commented Apr 8, 2018

should I request a review from NV Access?

@Brian1Gaff

This comment has been minimized.

Copy link

Brian1Gaff commented Apr 8, 2018

from NVDAObjects import NVDAObject
from NVDAObjects.UIA import UIA

def appropriateDescendant(obj):

This comment has been minimized.

@michaelDCurran

michaelDCurran Apr 8, 2018

Contributor

Either a docstring here or a name that better explains what this function is for.

This comment has been minimized.

@leonardder

leonardder Apr 9, 2018

Collaborator

I'd say the doc string is now clear, but still personally I'd prefer a name like isChildOfRow

obj = obj.parent
return False

class BrokenUIAChild(NVDAObject):

This comment has been minimized.

@michaelDCurran

michaelDCurran Apr 8, 2018

Contributor

Should inherit from UIA as it would not work with any other type of NVDAObject.

from NVDAObjects import NVDAObject
from NVDAObjects.UIA import UIA

def appropriateDescendant(obj):

This comment has been minimized.

@leonardder

leonardder Apr 9, 2018

Collaborator

I'd say the doc string is now clear, but still personally I'd prefer a name like isChildOfRow

#This is an icon and really is layout. Don't show it.
return
if obj.presentationType == obj.presType_layout and appropriateDescendant(obj):
clsList.insert(0, BrokenUIAChild)

This comment has been minimized.

@leonardder

leonardder Apr 9, 2018

Collaborator

Please add an empty line below.

@leonardder

This comment has been minimized.

Copy link
Collaborator

leonardder commented Apr 9, 2018

It is actually interesting to compare this issue with what I described in #7949 (comment) . There seem to be several similarities, particularly the fact that both the task manager and the outlook conversations view are kind of a data grid tree view hybrid. I wonder whether these issues have the same underlying UIA bug.

@beqabeqa473

This comment has been minimized.

Copy link

beqabeqa473 commented Apr 9, 2018

@PratikP1

This comment has been minimized.

Copy link

PratikP1 commented Apr 9, 2018

@derekriemer

This comment has been minimized.

Copy link
Collaborator Author

derekriemer commented Apr 12, 2018

Is it broken with simple review? I just tested it in simple review, and it worked.

@derekriemer

This comment has been minimized.

Copy link
Collaborator Author

derekriemer commented Apr 12, 2018

Okay guys, I addressed @leonardder review actions. I'm ready.

@leonardder
Copy link
Collaborator

leonardder left a comment

Just one thing, not related to the code as such. Furthermore, if we could somehow report this at Microsoft to get fixed, that would be preferred.

return
if obj.presentationType == obj.presType_layout and isChildOfRow(obj):
clsList.insert(0, BrokenUIAChild)

This comment has been minimized.

@leonardder

leonardder Apr 13, 2018

Collaborator

Please make this an empty line

@derekriemer

This comment has been minimized.

Copy link
Collaborator Author

derekriemer commented Apr 13, 2018

done

@PratikP1

This comment has been minimized.

Copy link

PratikP1 commented Apr 13, 2018

I have an open bug regarding this with Microsoft. The last I heard about it was that they're not intending to fix because it sort of works with Narrator. If one of you can provide me ith a little more technical info, I can go back and make another argument for a fix. Thank you.

@PratikP1

This comment has been minimized.

Copy link

PratikP1 commented Apr 13, 2018

Should this pull request be linked to issue #7148?

@leonardder

This comment has been minimized.

Copy link
Collaborator

leonardder commented Apr 13, 2018

@leonardder

This comment has been minimized.

Copy link
Collaborator

leonardder commented Apr 13, 2018

@PratikP1 commented on 13 Apr 2018, 14:15 CEST:

I have an open bug regarding this with Microsoft. The last I heard about it was that they're not intending to fix because it sort of works with Narrator. If one of you can provide me ith a little more technical info, I can go back and make another argument for a fix. Thank you.

Could #7949 (comment) help you in any way? I'm still pretty sure it's related.

@PratikP1 commented on 9 Apr 2018, 16:19 CEST:

I wonder if app list in the “apps and features” setting is the same.

That is a list, no tree view I believe.

@leonardder

This comment has been minimized.

Copy link
Collaborator

leonardder commented Apr 13, 2018

@PratikP1 commented on 13 Apr 2018, 14:17 CEST:

Should this pull request be linked to issue #7148?

If you think that this pr will fix it, and I think it does, then sure.

@derekriemer derekriemer force-pushed the derekriemer:taskman-content branch from 9f7a938 to 9d4b98d Apr 13, 2018

@derekriemer

This comment has been minimized.

Copy link
Collaborator Author

derekriemer commented Apr 13, 2018

I just rebased the fixes for review on master.

michaelDCurran added a commit that referenced this pull request Apr 15, 2018

@michaelDCurran michaelDCurran merged commit 3dcd74d into nvaccess:master Apr 30, 2018

@nvaccessAuto nvaccessAuto added this to the 2018.2 milestone Apr 30, 2018

@derekriemer derekriemer deleted the derekriemer:taskman-content branch May 16, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.