Skip to content

handle aria-invalid="spelling,grammar"#11787

Merged
michaelDCurran merged 10 commits into
masterfrom
invalidSpellingAndGrammar
Nov 26, 2020
Merged

handle aria-invalid="spelling,grammar"#11787
michaelDCurran merged 10 commits into
masterfrom
invalidSpellingAndGrammar

Conversation

@michaelDCurran

Copy link
Copy Markdown
Member

Link to issue number:

None.

Summary of the issue:

A web page author can indicate to ATs that text is a spelling error, grammar error, or both, via the aria-invalid attribute.
NVDA currently supports "spelling" and "grammar" but does not handle both at the same time.
Web applications such as Google Docs would like to specify that text is both a spelling and grammar error, but right now cannot, as if they expose "spelling, grammar" NVDA will not announce anything at all.

Description of how this pull request fixes the issue:

When NVDA collects the "invalid" IAccessible2 text attribute, it splits the value on "," and removes whitespace, and then looks for one or more of "spelling" or "grammar" separately.

Testing performed:

Added a system test which tests three paragraphs in Chrome, containing a spelling error, a grammar error, and both spelling and grammar error.

Known issues with pull request:

Although this has been done generically for all Gecko IA2 compatible browsers in NvDA, this does not work in Firefox as Firefox does not currently expose multiple values for aria-invalid via IAccessible2, rather it just exposes "true" in this case. This should be fixed in Firefox.

Change log entry:

Bug fixes:

  • Text marked both as being a spelling and grammar error at the same time in Google Chrome will be appropriately announced as both a spelling and grammar error by NVDA.

@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit 2f4fdc30a8

@lijianwei2019

lijianwei2019 commented Oct 27, 2020

Copy link
Copy Markdown

@jcsteh

@jcsteh

jcsteh commented Oct 27, 2020

Copy link
Copy Markdown
Contributor

Note that this violates the ARIA spec as it currently stands. The spec only allows a single token for aria-invalid, not a token list. If we were going to support this in Firefox, I'd want to see this accepted in at least the editor's draft of ARIA.

Furthermore, ARIA properties accepting multiple values should use a token list, which is defined as "Space-separated tokens". Thus, comma + space is a further spec violation.

@feerrenrut

Copy link
Copy Markdown
Contributor

Thanks for the heads up Jamie, we'll discuss this further.

@michaelDCurran

Copy link
Copy Markdown
Member Author

what is your view @aleventhal , as you were driving this request I believe? Are there plans in ARIA to allow for a set of values? I'm not too keen on supporting this in NVDA if there are no documented plans to get this in to ARIA, especially also if there is no real-world web app doing this yet...
Though having said all of this, if we see this pr just as supporting the idea of a set of values for the 'invalid' object attribute in IAccessible2, which Chrome does in deed expose, then perhaps things are okay.

@jcsteh

jcsteh commented Oct 27, 2020 via email

Copy link
Copy Markdown
Contributor

@feerrenrut

Copy link
Copy Markdown
Contributor

Although we care about the ARIA spec for this and advocate for any issues to be resolved, I believe we are consuming the IA2 spec correctly and think we should push forward on this. If the spec is updated, it will be a minor change to our sample. Perhaps a note should be added to the sample to highlight that it is still under discussion.

@michaelDCurran

Copy link
Copy Markdown
Member Author

@feerrenrut I have fixed conflicts and added a note about it not yet being standard ARIA and how it may need to change.

@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit 8a72bde3e0

@michaelDCurran michaelDCurran force-pushed the invalidSpellingAndGrammar branch from f5758b3 to 3e6a1dd Compare November 25, 2020 23:07
feerrenrut
feerrenrut previously approved these changes Nov 26, 2020

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@michaelDCurran michaelDCurran merged commit 4704df6 into master Nov 26, 2020
@michaelDCurran michaelDCurran deleted the invalidSpellingAndGrammar branch November 26, 2020 06:24
@nvaccessAuto nvaccessAuto added this to the 2020.4 milestone Nov 26, 2020
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