Fixed Edge landmark detection to work for Windows languages other than English#7333
Conversation
| if not isinstance(self,EdgeHTMLRoot) and role==controlTypes.ROLE_PANE and self.UIATextPattern: | ||
| return controlTypes.ROLE_INTERNALFRAME | ||
| ariaRole=self._getUIACacheablePropertyValue(UIAHandler.UIA_AriaRolePropertyId) | ||
| ariaRole=self._getUIACacheablePropertyValue(UIAHandler.UIA_AriaRolePropertyId).lower() |
There was a problem hiding this comment.
This change isn't related to landmark reporting, but since edge threads mixed case roles as valid, I had to convert the aria role to lower case for landmarks. Changed this here for consistency.
|
What about the case where the author has provided multiple roles (one of them possibly being a landmark)? Important if you are ever passing ariaRole directly to aria.landmarkRoles. |
|
@michaelDCurran commented on 5 jul. 2017 07:59 CEST:
So now I see where your code to split role comes from. I actually considered copying it, but I wasn't sure why. Let me investigate this later today or this week. Might be good to add a code comment that it is valid style to provide multiple aria roles |
…porting and the elements list
|
Though I was a bit reluctant to add a landmark property to EdgeNode initially, I decided to do it anyway. This is because we have at least two places where the landmark is retrieved, in the browse mode reporting case (EdgeTextInfo._getControlFieldForObject) and in the elements list. Not having a property for landmark would result in duplicate code to retrieve the aria roles, adequately split them and get the proper landmark role from it. My second rationale is in #6131 (comment). |
|
|
||
| def _get_landmark(self): | ||
| ariaRoles=self._getUIACacheablePropertyValue(UIAHandler.UIA_AriaRolePropertyId).lower() | ||
| landmarkId=self._getUIACacheablePropertyValue(UIAHandler.UIA_LandmarkTypePropertyId) |
There was a problem hiding this comment.
Do you actually use landmarkID? I'd think you'd want to fetch this right at the top, and return early if this is 0 before even needing to fetch ariaRole
| return True | ||
| return False | ||
|
|
||
| def _get_landmark(self): |
There was a problem hiding this comment.
A Docstring on this would be good as this is the base implementation.
Link to issue number:
Fixes #7328
Summary of the issue:
In Edge landmark reporting, NVDA relies on the localized landmark property. This breaks in languages other than English.
Description of how this pull request fixes the issue:
Testing performed:
See landmarkTest.html.txt