1748585 - Remove img elements with alt="" from the a11y tree

Status: RESOLVED FIXED

(Core :: Disability Access APIs, enhancement)

Description

[James Teh [:Jamie]]

img elements with alt="" are considered decorative. Currently, Gecko exposes Accessibles for these images, returning the empty string for Accessible::Name (instead of null when alt is unspecified). This was implemented long before ARIA or mapping specs. I believe the intent was to allow Windows screen readers to know about decorative images and apply their own heuristics for labelling if needed.

Since then, img alt="" has been mapped to ARIA role="presentation" in the spec. Chromium honours this mapping, stripping img alt="" from the a11y tree. I suspect WebKit does this as well.

Aside from spec compliance, exposing these is problematic on Mac, since VoiceOver sees an unlabelled image. It might be problematic for ATK/Android too.

I've been reluctant to consider changing this because it could break backwards compatibility for Windows clients. However, given the prevalence of Chromium, it seems reasonable to assume that if this were a real problem, it would have been addressed in Chromium by now.

One obscurity is that Chromium does expose img alt="" if it has a click listener, but not if it has role="presentation". That makes some sense, but isn't covered by the spec.

Implementing this will be a bit tricky because we need to watch for changes to the alt attribute and insert/remove an ImageAccessible accordingly.

Comment 1

[James Teh [:Jamie]]

Morgan, can you confirm whether alt="" gets stripped from the a11y tree by WebKit? For example, I assume this image gets reported by VO in Firefox, but not Safari?
data:text/html,before<img src="https://via.placeholder.com/10x10.png" alt="">after

Comment 2

[Morgan Reschenberg [:morgan]]

(In reply to James Teh [:Jamie] from comment #1)

Morgan, can you confirm whether alt="" gets stripped from the a11y tree by WebKit? For example, I assume this image gets reported by VO in Firefox, but not Safari?
data:text/html,before<img src="https://via.placeholder.com/10x10.png" alt="">after

Yep -- tested with VO, inspected the tree.

In firefox I get three nodes -- text, image, text (before, image, after)
In safari I get two text nodes (before, after) but VO doesn't delineate them, so it reads "beforeafter" all at once

Comment 4

[James Teh [:Jamie]]

When we do this, we can also get rid of the distinction between IsEmpty and IsVoid for the name string (getting rid of void), as well as the eNoNameOnPurpose flag.

Comment 5

[James Teh [:Jamie]]

This involves changes to the code that decides whether an Accessible should be created or not, as well as adding code to deal with insertion or removal when the alt attribute is changed. Here are some starting points:

• nsAccessibilityService::CreateAccessibleByFrameType, called from nsAccessibilityService::CreateAccessible, is responsible for creating ImageAccessibles.
  • CreateAccessibleByFrameType currently isn't allowed to return null to indicate no Accessible, so that'll probably have to be changed.
  • This made me wonder whether we should move img creation into HTMLMarkupMap. However, broken images (e.g. <img alt="foo">, no src) actually create a text Accessible, not an ImageAccessible, so that might not be possible.
• You can react to DOM attribute changes on existing Accessibles in mozilla::a11y::LocalAccessible::DOMAttributeChanged. If an Accessible doesn't exist, that won't work, so you'll want its caller, mozilla::a11y::DocAccessible::AttributeChanged, instead.
• You can force an Accessible to be re-created (or destroyed and not created again, if appropriate) using mozilla::a11y::DocAccessible::RecreateAccessible. However, we want to avoid unnecessary re-creations, so doing this for every change to the alt attribute is something we want to avoid.
• If I'm not around, Eitan is probably your best resource for help, as he has more experience with this area of the code, though I'm sure Morgan can help too.

Comment 6

[James Teh [:Jamie]]

(In reply to James Teh [:Jamie] from comment #4)

When we do this, we can also get rid of the distinction between IsEmpty and IsVoid for the name string (getting rid of void), as well as the eNoNameOnPurpose flag.

For this bit, just search for references to IsVoid, SetIsVoid and eNoNameOnPurpose in the accessible directory.

Comment 7

[Nathan LaPré]

Attachment: Bug 1748585: Remove img elements with alt="" from the a11y tree, r=Jamie

This revision changes the logic for creation and updating of accessibles
corresponding to img elements such that alt="" (without click listeners or any
other aria attribute that forces an accessible) will effectively remove that
img's accessible from the accessibility tree. Next, this revision removes the
concept of eNoNameOnPurpose from accessible name handling, since it's now
unnecessary now that alt="" means "don't create an accessible" in most cases.
This revision also adds tests to verify the functionality and updates existing tests.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e521fea384c5
Remove img elements with alt="" from the a11y tree, r=Jamie

Comment 9

[Atila Butkovits]

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

Comment 10

[Marian-Vasile Laza]

Backout link: https://hg.mozilla.org/mozilla-central/rev/063bef6f2545dc9c6d0da8cda50e76ed870e65ee

Comment 11

[Nathan LaPré]

So, there were four issues that popped up after landing this that I'll address before attempting to land a fix for this bug again:

• Bug 1781321: There were dev tools tests using alt="" on img elements and expecting accessibles to be created. I landed a patch to remove those tests. Despite the patch for this bug being backed out, this shouldn't cause an immediate problem since I just removed tests.
• Bug 1799208: Crash due to a lack of a null check. I submitted a patch for this, but it happened to fall over a weekend and so it didn't land before the original patch got backed out. I'll be sure to include that patch when submitting an improved version of the original (broken) patch.
• Bug 1799138, Bug 1799294: These merge-to-beta bugs are concerned with <embed> and <object> elements that embed images. It turns out that there is some different behavior between Nightly and other versions due to the static pref browser.opaqueResponseBlocking.syntheticBrowsingContext, which affects how the accessibility tree looks for these elements. I'm not sure how to fix this yet, but am investigating and will figure out the fix before submitting an updated patch.

Comment 12

[Nathan LaPré]

Rolled fixes for the above issues into the above patch associated with this bug and re-requested review.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/354b9b13995b
Remove img elements with alt="" from the a11y tree, r=Jamie

Comment 14

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/354b9b13995b