1425541 - [a11y] Make reader state easily to retrieve programmatically

Status: RESOLVED FIXED

(Toolkit :: Reader Mode, defect)

Description

[James Teh [:Jamie]]

The Reader button on the toolbar is already accessible (bug 1131458). However, this is not easy for accessibility clients to retrieve programmatically for a given document. For example, it might be useful for screen readers to report that the reader is available immediately after loading the document, since this isn't obvious otherwise without deliberate interaction with the button. Right now, doing this would require that the screen reader walk the tree to find the toolbar and then the correct button, which is slow and error prone. I propose that this be exposed as an attribute on the browser accessible so that it can be easily retrieved in a programmatic way.

Comment 1

[James Teh [:Jamie]]

Attachment: Bug 1425541: Expose reader mode state on the browser accessible using a "reader" object attribute.

This makes it easy for accessibility clients to retrieve the reader mode state programmatically.

There are three possibilities:
1. Reader mode is available for the current page (reader:available).
2. Reader mode is being used now (reader:active).
3. Reader is not available (the reader attribute is not present).

We do this by setting/removing the aria-reader attribute on the node.
This is not a real ARIA attribute, but it causes Gecko to expose it as an object attribute.

Review commit: https://reviewboard.mozilla.org/r/207854/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/207854/

Comment 2

[James Teh [:Jamie]]

Eitan, a couple of notes:
1. This is exposed on the browser accessible, not the document accessible. Exposing it on the document accessible would have meant adding attributes to the body, which is problematic because we're messing with the document itself. That might affect (or be affected by) scripts in the document. There might be a way to do this without setting attributes, but that would require modifying core a11y code, which I'm not comfortable with since this is browser (not web) specific. This shouldn't be a problem for clients; they just have to walk to the parent to get this info, which is not a big deal.
2. This is not standard in any way; it is Gecko specific. We might be able to convince other browsers to adopt it. However, I think it would be hard to document in any standard, since it's rather domain specific to browsers; it's not something you can apply across applications in general.
3. Because it's not standard, it could be argued this should be exposed with some sort of prefix; e.g. moz-reader. Personally, I don't think it's worth it; it's not like we're exposing this in the actual web content. However, I'm happy to make this change if you feel it's necessary.

Comment 3

[:Gijs (he/him)]

Comment on attachment 8937143 [details]
Bug 1425541: Expose reader mode state on the browser accessible using a "reader" object attribute.

https://reviewboard.mozilla.org/r/207854/#review213864

rs=me in that this looks fine, though I don't really understand what the attribute is *for*. That is, if this isn't a real aria attribute, how is it exposed to screenreaders? I don't see a patch to `accessible/` or anywhere else that does this. Do screenreaders just need to look for the actual attribute? Unless `aria-` prefixed attributes are especially exposed, this doesn't really add any information for screenreaders.

As it is, they can already get all this information by looking at the button (whether it's hidden, and whether it has the 'readeractive' attribute). I suppose it doesn't hurt too much to add a more explicit exposure of this, I'm just a bit confused how this is effective where the existing code isn't.

Comment 4

[James Teh [:Jamie]]

Sorry. I attempted to explain this in my comments, but apparently didn't give enough context. :(

1. It's true that you can get this info from the button. However, that would require locating the button in the tree, which means walking the tree or obscure hard-coding of relationships which is not future proof. There's no way to get directly to the button to query it, so it's difficult to, for example, have a screen reader report the reader state when the page finishes loading.
2. Accessibility clients can't retrieve DOM attributes directly. However, Gecko has a rule whereby aria-xx attributes (even non-standard ones) get exposed as accessibility "object attributes" which clients *can* retrieve. So, aria-reader gets exposed as a "reader" attribute on the browser accessible. In contrast, a "reader" attribute (no prefix) wouldn't get exposed at all to accessibility clients.

Hope this clarifies things.

Comment 5

[James Teh [:Jamie]]

Clarification on point 1: The reason that exposing this on the browser element solves the first issue is that the browser accessible is the direct parent of the document, so an acccssibility client need only fetch the parent of the document to get this information.

Comment 6

[Eitan Isaacson [:eeejay]]

Comment on attachment 8937143 [details]
Bug 1425541: Expose reader mode state on the browser accessible using a "reader" object attribute.

https://reviewboard.mozilla.org/r/207854/#review214236

I'm not in love with the name aria-reader, it suggests a relationship with the standard. A more appropriate thing would be to use the node's dataset for non-standard attributes. I don't think our a11y module will propagate a data-* attribute, so I guess the current name, aria-reader, will have to do.

Comment 7

[James Teh [:Jamie]]

(In reply to Eitan Isaacson [:eeejay] from comment #6)
> I'm not in love with the name aria-reader, it suggests a relationship with
> the standard.

Yeah, I know. It's kind of ugly.

> A more appropriate thing would be to use the node's dataset
> for non-standard attributes. I don't think our a11y module will propagate a
> data-* attribute,

It won't.

> so I guess the current name, aria-reader, will have to do.

There are two alternatives:
1. Change our a11y code to expose data attributes with a certain prefix; e.g. data-moz-a11y-reader.
2. The class attribute does get exposed, so we could include a class only used for a11y identification; e.g. class="... document-reader-active" or class="... document-reader-available". Clients would then inspect the class attribute to figure this out.
Personally, I think the aria-reader hack is less ugly than the alternatives, but I'm happy to try implementing either one of these if you prefer.

Comment 8

[James Teh [:Jamie]]

Eitan, I know you r+ed this, but before I land it, I wanted to check whether you'd prefer some of the alternatives I proposed in comment 7. Thanks.

Comment 9

[Eitan Isaacson [:eeejay]]

(In reply to James Teh [:Jamie] from comment #8)
> Eitan, I know you r+ed this, but before I land it, I wanted to check whether
> you'd prefer some of the alternatives I proposed in comment 7. Thanks.

No, I don't really prefer them. They each have an issue. I say go ahead!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/196006c746bd
Expose reader mode state on the browser accessible using a "reader" object attribute. r=eeejay,Gijs

Comment 11

[Dorel Luca [:dluca]]

Backed out changeset 196006c746bd (bug 1425541) for failing Mochitest on intl/l10n/test/dom/test_domloc_overlay.html on a CLOSED TREE

https://treeherder.mozilla.org/logviewer.html#?job_id=156455645&repo=autoland&lineNumber=2629

https://hg.mozilla.org/integration/autoland/rev/10400d85b71114dbee7c73b7fd02f9b3dae0495e

https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=196006c746bde8b894252ec1310a4b5124e04c14

Comment 12

[Zibi Braniecki [:zbraniecki][:gandalf]]

This is not the right bug. The regression in test_domloc_overlay has been introduced by me in bug 1389384. I'll have a fix for it in a couple minutes.

Comment 13

[James Teh [:Jamie]]

Misidentified as the failure cause; see comment 11. I'll re-land now.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a6004a75d2d3
Expose reader mode state on the browser accessible using a "reader" object attribute. r=eeejay,Gijs

Comment 15

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/a6004a75d2d3

Comment 16

[James Teh [:Jamie]]

I intend to remove and supersede this in bug 1585907.