Skip to content

Ensure that labels explicitly set on divs and spans are reported in braille and speech#8886

Merged
michaelDCurran merged 2 commits into
masterfrom
reportGenericAriaLabel
Oct 29, 2018
Merged

Ensure that labels explicitly set on divs and spans are reported in braille and speech#8886
michaelDCurran merged 2 commits into
masterfrom
reportGenericAriaLabel

Conversation

@michaelDCurran

Copy link
Copy Markdown
Member

Link to issue number:

None.

Summary of the issue:

A web author can explicitly set the label of a node using aria-label or aria-labelledby. In BrowseMode, NvDA will in some cases use this label as the actual text content, and in some cases report it as extra meta information. However, in the case of a div or a span that is not presented in any other way, NVDA currently fails to report the label at all. It should at least be reported as meta info. As the web author rightfully expects that if they set a label, it will be exposed somewhere.
The following html fragment in Firefox demonstrates this:

data:text/html,<p>Span with label: <span aria-label="span label">span text</span></p><p>A div with a label:</p><div aria-label="div label">div text</div>

NVDA does not announce "span label" or "div label" anywhere.

Description of how this pull request fixes the issue:

In both speech and braille, NVDA now reports the label (name) if explisitly set by the author (alwaysReportName). Note that in the case of landmarks where the label is already reported specifically as the landmark label, care is taken not to duplicate the label for both speech and braille.

Testing performed:

In Firefox, read the above testcase in both speech and braille. "span label" was reported before "span text" and "div label" before "div text".

Known issues with pull request:

None.

Change log entry:

In BrowseMode, NVDA will now always report labels on divs and spans explicitly set by the web author.

…divs and spans (that don't usually get reported) are shown in browseMode in both speech and braille.
@michaelDCurran michaelDCurran merged commit fd24d81 into master Oct 29, 2018
@nvaccessAuto nvaccessAuto added this to the 2018.4 milestone Oct 29, 2018
@Neurrone

Copy link
Copy Markdown

@michaelDCurran I've been browsing with this change for a few days, and the extra verbosity is annoying. In places like GitHub, the div's label is either a substring of the actual contents, or both are equal.

I'm not sure if adding a special case to not announce the label when it is the same as its contents would be justified, since that is nonstandard, but the way it is being used in the wild is problematic.

@LeonarddeR

LeonarddeR commented Oct 30, 2018 via email

Copy link
Copy Markdown
Collaborator

@michaelDCurran

michaelDCurran commented Oct 30, 2018 via email

Copy link
Copy Markdown
Member Author

@LeonarddeR

LeonarddeR commented Oct 30, 2018

Copy link
Copy Markdown
Collaborator
<span class="timeline-comment-label tooltipped tooltipped-multiline tooltipped-s" aria-label="This user has previously committed to the nvda repository.">
      Contributor
    </span>

@jcsteh

jcsteh commented Oct 31, 2018

Copy link
Copy Markdown
Contributor

GitHub use aria-label for tool tip messages, which is totally inappropriate (and I'd argue it's a spec violation). Still, the fact remains that this is happening on major sites in the wild.

Another case where this change is problematic is where a div/span has no useful content, in which case we fall back to the name as the content. In that case, you get duplication. Consider this example:

data:text/html,<div aria-label="foo">&nbsp;</div>

With this change, you'll hear "foo foo". If we keep this, we really should detect this case and prevent the duplication.

michaelDCurran added a commit that referenced this pull request Oct 31, 2018
michaelDCurran added a commit that referenced this pull request Oct 31, 2018
…ted in braille and speech" (#8899)

* Revert "Revert "Don't announce 'selected' when the focus moves in Google sheets if the focused cell is the only cell selected (#8879)" (#8893)"

This reverts commit b4e9e83.

* Revert "Don't announce 'selected' when the focus moves in Google sheets if the focused cell is the only cell selected (#8879)"

This reverts commit 51f5b2f.

* Revert "Merge all vbufBackend dlls into nvdaHelperRemote.dll (#8866)"

This reverts commit 24f5123.

* Revert "Fix handling of table cells without a containing table in browse mode. (#8887)"

This reverts commit 5fe34c5.

* Revert "Ensure that  labels explicitly set on divs and spans are reported in braille and speech (#8886)"

This reverts commit fd24d81.
@michaelDCurran

Copy link
Copy Markdown
Member Author

I have reverted this pr:
Too many cases in the wild mis-use aria-label, and there was some duplication in NVDA itself. Plus ARIA spec may be moving towards saying that aria-label without an aria role is invalid anyway.

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.

6 participants