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

LocalizedControlTypeReasonable fails with German locale #1572

Closed
ollypsilon opened this issue Mar 14, 2023 · 10 comments
Closed

LocalizedControlTypeReasonable fails with German locale #1572

ollypsilon opened this issue Mar 14, 2023 · 10 comments
Assignees
Labels
bug Something isn't working status: resolved This issue has been merged into main.

Comments

@ollypsilon
Copy link

The following accessibility issue needs investigation.

App: MSACCESS

Element path: Bild 'Mit Access verknüpfte Tabelle'

Issue Details: The LocalizedControlType should be reasonable based on the element's ControlTypeId. Section 508 502.3.1

How To Fix: Set a meaningful LocalizedControlType UI Automation property for this element.

This accessibility issue was found using Accessibility Insights for Windows, a tool that helps debug and find accessibility issues earlier. Get more information and download this tool at https://aka.ms/AccessibilityInsights.

Beside automatic issue text above here's the element's details:
image

@microsoft-github-policy-service microsoft-github-policy-service bot added the status: new This issue is new and requires triage by DRI. label Mar 14, 2023
@codeofdusk
Copy link
Contributor

“Bild” is image in German, so this appears to be a false positive .

@codeofdusk codeofdusk added the status: ready for triage This issue is ready to be triaged by the Accessibility Insights team. label Mar 14, 2023
@microsoft-github-policy-service
Copy link
Contributor

This issue has been marked as ready for team triage; we will triage it in our weekly review and update the issue. Thank you for contributing to Accessibility Insights!

@microsoft-github-policy-service microsoft-github-policy-service bot removed the status: new This issue is new and requires triage by DRI. label Mar 14, 2023
@codeofdusk codeofdusk changed the title Section 508 502.3.1: (MSACCESS/Bild 'Tabelle') The LocalizedControlType should be reasonable based on the element's ControlTypeId. LocalizedControlTypeReasonable fails with German locale Mar 14, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the status: new This issue is new and requires triage by DRI. label Mar 14, 2023
@codeofdusk codeofdusk removed the status: new This issue is new and requires triage by DRI. label Mar 14, 2023
@DaveTryon
Copy link
Contributor

Some more info here: Neither Axe.Windows nor AIWin currently support running with non-English localized resources, so in this case the rule is comparing the German-localized name to the English-localized name and not finding a match.

We could potentially alter the rule to exclude scenarios where the locale language doesn't match the process language, but we'd need to work out the details of how this works, how to document it, etc.

@JGibson2019 JGibson2019 added status: ready for work This issue is ready to be worked on. status: needs investigation This issue requires investigation by the Accessibility Insights team. and removed status: ready for triage This issue is ready to be triaged by the Accessibility Insights team. status: ready for work This issue is ready to be worked on. labels Apr 25, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot removed the status: needs investigation This issue requires investigation by the Accessibility Insights team. label Apr 25, 2023
@microsoft-github-policy-service
Copy link
Contributor

This issue requires additional investigation by the Accessibility Insights team. When the issue is ready to be triaged again, we will update the issue with the investigation result and add "status: ready for triage". Thank you for contributing to Accessibility Insights!

@DaveTryon
Copy link
Contributor

We've identified 2 options here: Either:

  1. Disable this rule if the language is not English OR
  2. Provide a localized dictionary of appropriate names and check the localized name. If we don't have a localized version available, then skip the rule

@DaveTryon DaveTryon added the status: needs investigation This issue requires investigation by the Accessibility Insights team. label Apr 26, 2023
@microsoft-github-policy-service
Copy link
Contributor

This issue requires additional investigation by the Accessibility Insights team. When the issue is ready to be triaged again, we will update the issue with the investigation result and add "status: ready for triage". Thank you for contributing to Accessibility Insights!

@DaveTryon DaveTryon added status: ready for work This issue is ready to be worked on. and removed status: needs investigation This issue requires investigation by the Accessibility Insights team. labels May 2, 2023
@DaveTryon
Copy link
Contributor

Plan is to disable this rule if running on a non-English language setting

@DaveTryon DaveTryon added the bug Something isn't working label May 2, 2023
@DaveTryon DaveTryon added status: active This issue is currently being worked on by someone. and removed status: ready for work This issue is ready to be worked on. labels May 10, 2023
@DaveTryon DaveTryon self-assigned this May 10, 2023
DaveTryon added a commit to microsoft/axe-windows that referenced this issue May 10, 2023
#### Details

microsoft/accessibility-insights-windows#1572
called out the fact that the `LocalizedControlTypeReasonable` rule uses
English language strings to validate localized names. The chosen
solution was to change the `Condition` so that this rule only applies on
English language systems. This PR adds a new `Condition` called
`IsEnglish` that takes advantage of .NET's built-in support of 3 letter
language codes as defined in the ISO 639-2 spec. For our purposes, we
disable the rule for anything other than "eng". The check is
case-insensitive, which probably isn't strictly necessary, but seemed
like the safest bet.

Tested by setting my Windows language to French and verifying that
`SystemPropertiesTests.OverriddenCultureName_ResetsToDefault` reports as
inconclusive. It also revealed the need for adjustments to `MonsterTest`
when run on non-English systems.

##### Motivation

Address
microsoft/accessibility-insights-windows#1572,
avoid noise for Visual Customers running on non-English systems.

##### Context

<!-- Are there any parts that you've intentionally left out-of-scope for
a later PR to handle? -->

<!-- Were there any alternative approaches you considered? What
tradeoffs did you consider? -->
My first iteration used the `CultureInfo.Name` property, which follows a
format like **en-us**. This worked reasonably well, but using the
three-letter language standard made the code a little it simpler.
 
#### Pull request checklist
<!-- If a checklist item is not applicable to this change, write "n/a"
in the checkbox -->
- [x] Addresses an existing issue:
microsoft/accessibility-insights-windows#1572
@DaveTryon DaveTryon added status: resolved This issue has been merged into main. and removed status: active This issue is currently being worked on by someone. labels May 11, 2023
@DaveTryon DaveTryon added the status: active This issue is currently being worked on by someone. label May 24, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot removed the status: resolved This issue has been merged into main. label May 24, 2023
@DaveTryon
Copy link
Contributor

DaveTryon commented May 24, 2023

This has been addressed in Axe.Windows 2.1.2, which was picked up in #1620. It will be in the next Canary release

@DaveTryon DaveTryon added status: resolved This issue has been merged into main. and removed status: active This issue is currently being worked on by someone. labels May 24, 2023
@DaveTryon
Copy link
Contributor

@ollypsilon, this is fixed in our Canary release available at at https://github.com/microsoft/accessibility-insights-windows/releases/tag/v1.1.2334.01. It will soon ship to Production.

@DaveTryon
Copy link
Contributor

Per our policy, closing the issue as it's available in Canary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working status: resolved This issue has been merged into main.
Projects
None yet
Development

No branches or pull requests

5 participants