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

Core-AAM: Correct issues in aria-readonly mapping for MSAA/IA2: #619

Closed
wants to merge 5 commits into from

Conversation

joanmarie
Copy link
Contributor

  • For aria-readonly="true":
    • Add statement that IAccessibleEditableText should not be implemented.
    • Add statement that IA2_STATE_EDITABLE should be cleared.
  • For aria-readonly="false":
    • Remove statement regarding exposing IA2_STATE_EDITABLE on textbox.
    • Add statement that STATE_SYSTEM_READONLY should be cleared.

* For aria-readonly="true":
  - Add statement that IAccessibleEditableText should not be implemented.
  - Add statement that IA2_STATE_EDITABLE should be cleared
* For aria-readonly="false":
  - Remove statement regarding exposing IA2_STATE_EDITABLE on textbox.
  - Add statement that STATE_SYSTEM_READONLY should be cleared.
@joanmarie joanmarie requested a review from asurkov August 2, 2017 11:51
@joanmarie
Copy link
Contributor Author

@asurkov: Please review. Thanks!
@aleventhal: Does this address your concerns?

@joanmarie
Copy link
Contributor Author

(The conflict is just the changelog entry due to other commits. I'll fix that upon merge.)

@asurkov
Copy link
Contributor

asurkov commented Aug 2, 2017

I think COM doesn't allow to change exposed interfaces on the fly, and we have to recreate the object to conform this rule. Instead we could require the implementations to return S_FALSE on all IAccessibleEditableText methods. Otherwise the proposed change looks good.

Modify statement that IAccessibleEditableText shouldn't be implemented to
include alternative of returning S_FALSE for all IAccessibleEditableText
methods so that user agents do not have to recreate accessible objects.
* For aria-readonly="true":
  - Add statement that IAccessibleEditableText should not be implemented.
  - Add statement that IA2_STATE_EDITABLE should be cleared
* For aria-readonly="false":
  - Remove statement regarding exposing IA2_STATE_EDITABLE on textbox.
  - Add statement that STATE_SYSTEM_READONLY should be cleared.
Modify statement that IAccessibleEditableText shouldn't be implemented to
include alternative of returning S_FALSE for all IAccessibleEditableText
methods so that user agents do not have to recreate accessible objects.
@aleventhal
Copy link
Contributor

I would point out that copyText is still relevant. I think that's the real reason authors would use readonly instead of disabled. So I suggest IAccessibleEditableText is still implemented, but that all methods other than copyText should return S_FALSE. I think that both EDITABLE and READONLY states are exposed. This indicates that the editable interface is supported but it is readonly.

@aleventhal
Copy link
Contributor

My next comment is, how do we expose aria-readonly differently when it is undefined vs "false" given this statement by @mcking65:

WRT undefined, aria-readonly is like aria-pressed, aria-expanded, and aria-haspopup … browsers should not represent those properties as false when they are not present. Browsers and assistive technologies should not assume that a grid that does not have aria-readonly is a grid that contains gridcells that contain editable content. This point is an important aspect of the clarifications we made to the definition of the grid role in ARIA 1.1.

@aleventhal
Copy link
Contributor

For non-editable widgets, methods to set a value or change checked state etc. should also fail or return false.

@joanmarie
Copy link
Contributor Author

joanmarie commented Aug 4, 2017

So I suggest IAccessibleEditableText is still implemented, but that all methods other than copyText should return S_FALSE.

I always forget that's where accessibility APIs expose that functionality. It depresses me. But yeah, good point.

I think that both EDITABLE and READONLY states are exposed. This indicates that the editable interface is supported but it is readonly.

If I saw that in ATK/AT-SPI2, I'd call it a bug. Because "editable" means "editable." Would it be sufficient to communicate the not-quite-editable nature via the READONLY state + largely non-functional edtabletext interface? (In other words, only expose the EDITABLE state when something is truly editable.)

@joanmarie
Copy link
Contributor Author

My next comment is, how do we expose aria-readonly differently when it is undefined vs "false"

The ARIA 1.1 spec states that aria-readonly is a true/false type; not a true/false/undefined type. So my answer would be, "you don't" -- though clarification from @mcking65 would be helpful.

@aleventhal
Copy link
Contributor

In other words, only expose the EDITABLE state when something is truly EDITABLE.

It's probably ok. But it might break something for readonly fields in JAWS or NVDA. For example, what if they only listen to caret events when the EDITABLE state is set. Or they only allow text to be copied while in the virtual buffer if it's set.

@aleventhal
Copy link
Contributor

I checked IA2 in Firefox and Chrome -- they both expose both EDITABLE+READONLY for readonly textfields. | personally i'm not sure what's to be gained by changing that. It may feel slightly cleaner but it could cause new code paths to run in current screen readers. I'm not really sure, but I don't like the idea of changing what's been working at the last minute here.

@joanmarie
Copy link
Contributor Author

I'm not really sure, but I don't like the idea of changing what's been working at the last minute here.

Understood. Then should we put this one off until ARIA 1.2?

@asurkov
Copy link
Contributor

asurkov commented Aug 4, 2017

It feels weird to have both EDITABLE and READONLY on same accessible. Iirc we discussed this topic years ago, but I don't recall outcome unfortunately, we either decided to go with EDITABLE+READONLY or make them mutually exclusive. I know it's not helpful :)

In ATK case in Firefox, we indeed make sure we don't expose them both. Nothing like this for MSAA, what Aaron observes.

Personally I would avoid to wake up a sleeping dog if possible - a lot of legacy in a11y world, but if we have to then it'd be good to reach AT vendors to check on their assumptions.

@mcking65
Copy link
Contributor

mcking65 commented Aug 11, 2017

The most important thing that needs to be fixed for ARIA 1.1 implementations is that grids should never be announced as "editable".

I am not clear on the root cause of that behavior at the accessibility API level as I am not familiar with all the intricacies of IAccessibleEditableText and similar interfaces on other platforms. The one thing I do know is that the implementations were based on a misleading statement in ARIA 1.0 that said grids were assumed to be editable. In ARIA 1.1, we removed that statement and made it clear that aria-readonly, when applied to a grid, does not say anything about the grid itself.

Thus, in ARIA 1.1, a grid is a composite widget, not an editable component. You can't edit a grid in any manner. Similarly, a gridcell is just a container. As with the grid container, by default, screen readers should not imply that the cell itself is mutable.

ARIA 1.0 seemed to assume that the only reason to use a grid was to make a spreadsheet. In contrast, ARIA 1.1 clarifies that a grid is a composite widget that can be used to create two-dimensional, interactive structures. And, one special case use of grid is to build a spreadsheet.

ARIA 1.1 doesn't include a programatic way of distinguishing a spreadsheet-like grid from any other grid. Assistive technology users are expected to understand the nature of the grid and the affordances it provides in the same way that all other users do -- it is typically communicated by the context.

To support spreadsheet-like applications where all cells are assumed BY THE USER, not the AT, to contain editable content, it is possible to mark cells that are in a read-only state with aria-readonly. And, as a convenience, the author may propagate a value of aria-readonly to all cells by applying it to the grid.

Conclusions for browser and AT implementation:

  1. If aria-readonly is not present on a grid or gridcells, the browser/AT should not do anything that would cause the user to think that there is something editable about the grid or any of its cells.
  2. If aria-readonly=false is on a grid or gridcell, it is equivalent to aria-readonly not being present.
  3. If aria-readonly is set true on a cell, then the AT should say it is a readonly cell.
  4. If aria-readonly is true on the grid, then:
    A. It is the same as aria-readonly=true on every cell and aria-readonly not being present on the grid.
    B. Author may put aria-readonly=false on individual cells to override true on the grid.
    C. It should have no effect on how AT announces the grid container but will cause readonly announcement on all cells where it is not overridden by the author.

The changes in this PR for aria-readonly=true appear to capture the above. However, it is not clear that the language for aria-readonly=false is adequate. For grid and gridcell, to accomplish the above objectives, do we need some language that explicitly states that nothing related to editability should be exposed?

@joanmarie
Copy link
Contributor Author

Closing this as obsolete and also in the wrong repo (now).

@joanmarie joanmarie closed this Sep 13, 2018
@joanmarie joanmarie deleted the readonly-ia2 branch September 13, 2018 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants