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

Update UIA aria-readonly mappings #203

Merged
merged 2 commits into from
Oct 31, 2023

Conversation

sivakusayan
Copy link
Contributor

@sivakusayan sivakusayan commented Oct 16, 2023

Closes #127

I accidentally closed #192 messing around with my fork (woops), so I'm resubmitting - sorry for the trouble 🙁.

Duplicating the original PR's content here.


Preview | Diff

@sivakusayan
Copy link
Contributor Author

sivakusayan commented Oct 16, 2023

Closes #127

The UIA aria-readonly mappings only accounted for controls that implemented the IValueProvider interface. Now, it also accounts for controls that implement the IRangeValueProvider, and otherwise puts the information in AriaProperties if a control implements neither IValueProvider nor IRangeValueProvider.

Implementation

As an aside, if you want to observe this mapping in Blink, you'll need to use Edge or run Chromium with UIAutomation enabled - the UIAutomation bridge will not map IA2 STATE_SYSTEM_READONLY to the AriaProperties property when appropriate.


Preview | Diff

index.html Outdated
@@ -7486,7 +7486,9 @@ <h4 id=ariaReadonlyTrue><code>aria-readonly</code>=<code>true</code></h4>
<tr>
<th><abbr title="User Interface Automation">UIA</abbr></th>
<td>
<span class="property">Property: <code>Value.IsReadOnly</code>: <code>true</code></span>
<span class="property">Property: <code>Value.IsReadOnly</code>: <code>true</code>, if the element implements <a href="https://learn.microsoft.com/en-us/dotnet/api/system.windows.automation.provider.ivalueprovider?view=windowsdesktop-7.0"><code>IValueProvider</code></a>.</span><br>
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Can we remove the "?view=windowsdesktop-7.0" parameters from the URLs? I counted 8 instances.

index.html Outdated
<span class="property">Property: <code>Value.IsReadOnly</code>: <code>false</code></span>
<span class="property">Property: <code>Value.IsReadOnly</code>: <code>false</code>, if the element implements <a href="https://learn.microsoft.com/en-us/dotnet/api/system.windows.automation.provider.ivalueprovider?view=windowsdesktop-7.0"><code>IValueProvider</code></a>.</span><br>
<span class="property">Property: <code>RangeValue.IsReadOnly</code>: <code>false</code>, if the element implements <a href="https://learn.microsoft.com/en-us/dotnet/api/system.windows.automation.provider.irangevalueprovider?view=windowsdesktop-7.0"><code>IRangeValueProvider</code></a>.</span><br>
<span class="property">Property: <code>AriaProperties.readonly</code>: <code>false</code>, if the element implements neither <a href="https://learn.microsoft.com/en-us/dotnet/api/system.windows.automation.provider.ivalueprovider?view=windowsdesktop-7.0"><code>IValueProvider</code></a> or <a href="https://learn.microsoft.com/en-us/dotnet/api/system.windows.automation.provider.irangevalueprovider?view=windowsdesktop-7.0"><code>IRangeValueProvider</code></a>.</span>
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we absolutely certain about this? Our approach in Chromium is to always expose the AriaProperties in this property, regardless of whether it is already exposed in other more relevant UIA properties. I believe this is also what the documentation indicates as well: https://learn.microsoft.com/en-us/windows/win32/winauto/uiauto-ariaspecification#w3c-aria-states-and-properties-mapped-to-microsoft-active-accessibility-and-ui-automation.

Copy link
Contributor Author

@sivakusayan sivakusayan Oct 23, 2023

Choose a reason for hiding this comment

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

Thanks for that link - I got the impression from 4.3 Exposing attributes that do not directly map to accessibility API properties that AriaProperties is supposed to be used purely to expose unsupported semantics. I guess I would be curious why these properties are exposed in AriaProperties when there are UIAutomation mappings that described them just as well, but in any case it's probably best to just align with the with the UIAutomation documentation.

As an aside, I notice that there are a lot of properties in that link that seem to need AriaProperties exposure, but aren't documented in the Core-AAM as such. I guess that's an issue we can address separately.

Edit: Ahh, I see - this is the problem you brought up here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Your interpretation of it is not wrong. 4.3 specifically says

When WAI-ARIA roles, states and properties do not directly map to an accessibility API, and there is a mechanism in the API to expose the WAI-ARIA role, states, and properties and their values, user agents MUST expose the WAI-ARIA data using that mechanism as follows

and later says

User agents MUST also expose the entire role string through this mechanism and MAY also expose WAI-ARIA attributes and values through this mechanism even when there is a direct mapping to an accessibility API.

In this case, I believe the UIA documentation is more restrictive than the Core-AAM one, but both can be respected if we add AriaProperties on all aria attributes.

@sivakusayan
Copy link
Contributor Author

Addressed comments - as you mentioned in the UIA aria-haspopup issue, there is a wider problem that a lot of mappings don't call out the need for UIA AriaProperties exposure, but I think it makes sense to address that in the generalized fix for #196. Let's individually call it out for aria-readonly here for now.

Copy link
Contributor

@benbeaudry benbeaudry left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you, Sayan.

@w3cbot
Copy link

w3cbot commented Oct 31, 2023

spectranaut marked as non substantive for IPR from ash-nazg.

Copy link
Contributor

@spectranaut spectranaut left a comment

Choose a reason for hiding this comment

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

This is great, thanks :)

I opened a new issue for the lack of AriaProperties entries: #209

@spectranaut spectranaut merged commit 5490c20 into w3c:main Oct 31, 2023
2 checks passed
github-actions bot added a commit that referenced this pull request Oct 31, 2023
SHA: 5490c20
Reason: push, by spectranaut

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

aria-readonly: Consider explicitly calling out UIA mapping for controls that don't follow the ValuePattern
4 participants