Navigation Menu

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

derekriemer
Copy link
Collaborator

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
Copy link
Collaborator Author

should I request a review from NV Access?

@Brian1Gaff
Copy link

Brian1Gaff commented Apr 8, 2018 via email

from NVDAObjects import NVDAObject
from NVDAObjects.UIA import UIA

def appropriateDescendant(obj):
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

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):
Copy link
Member

Choose a reason for hiding this comment

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

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):
Copy link
Collaborator

Choose a reason for hiding this comment

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

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)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add an empty line below.

@LeonarddeR
Copy link
Collaborator

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
Copy link
Contributor

beqabeqa473 commented Apr 9, 2018 via email

@PratikP1
Copy link

PratikP1 commented Apr 9, 2018 via email

@derekriemer
Copy link
Collaborator Author

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

@derekriemer
Copy link
Collaborator Author

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

Copy link
Collaborator

@LeonarddeR LeonarddeR left a comment

Choose a reason for hiding this comment

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

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)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please make this an empty line

@derekriemer
Copy link
Collaborator Author

done

@PratikP1
Copy link

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
Copy link

Should this pull request be linked to issue #7148?

@LeonarddeR
Copy link
Collaborator

LeonarddeR commented Apr 13, 2018 via email

@LeonarddeR
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
Copy link
Collaborator

@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
Copy link
Collaborator Author

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

None yet

7 participants