Skip to content

Support aria-roledescription in Edge.#8449

Merged
michaelDCurran merged 7 commits into
masterfrom
i8448
Jul 17, 2018
Merged

Support aria-roledescription in Edge.#8449
michaelDCurran merged 7 commits into
masterfrom
i8448

Conversation

@michaelDCurran

@michaelDCurran michaelDCurran commented Jun 27, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

Link to issue number:

Fixes #8448

Summary of the issue:

Web authors can provide an aria-roledescription property to customize the way the type (role) of an element is announced/brailled in Assistive technologies. NVDA supports this already for Chrome/Firefox/IE but not Edge.

Description of how this pull request fixes the issue:

Implements support for aria-roledescription in Edge for both focus and browse mode.
This PR also now:

  • Ensures that braille honors roledescription in browseMode (it already did in focus mode).
  • Ensures that the name of a field is not duplicated if the field is also a landmark.
  • Region is no longer reported for a landmark region that is also another valid field (E.g. a region with a roledescription).
  • roledescription on divs in Edge now present as block fields (I.e. do not keep reporting the roledescription for each line within the field).

Testing performed:

Tested example from #8448 in Edge.

Known issues with pull request:

None.

Change log entry:

Existing what's new for aria-roledescription should be updated to mention Edge, if this is merged before 2018.3.
Section: New features, Changes, Bug fixes

@michaelDCurran michaelDCurran mentioned this pull request Jun 27, 2018
Closed
@josephsl

josephsl commented Jun 27, 2018

Copy link
Copy Markdown
Contributor

Hi,

Testing shows it is working as expected. I may need to test it on multiple Windows 10 releases to see which one added this property for Edge nodes.

Thanks.

feerrenrut
feerrenrut previously approved these changes Jun 27, 2018
View reviewed changes
@josephsl

josephsl commented Jun 27, 2018

Copy link
Copy Markdown
Contributor

CC @tgeczy

michaelDCurran added a commit that referenced this pull request Jun 27, 2018
@michaelDCurran
Incubates #8449
2df4d29
@LeonarddeR

LeonarddeR commented Jul 5, 2018 •
edited
Loading

Copy link
Copy Markdown
Collaborator

I tested some of the examples from the ARIA examples page and found the following.

<div role="region" aria-roledescription="slide" id="slide42" aria-labelledby="slide42heading">

  • Expected: Quarterly Report slide
  • Actual: Quarterly Report region slide

Note that this also is the case in firefox. In edge, the section variant of slide reports correctly, whereas in edge it still reports region redundantly.

Another thing you might be able to fix as part of this, is that roletext seems to be entirely ignored for braille.

@josephsl

josephsl commented Jul 13, 2018

Copy link
Copy Markdown
Contributor

Hi,

Marco Zehe (Mozilla) says:

Hi Joseph!

I am just now trying out the try build you posted earlier. Here are two observations regarding aria-roledescription in Edge. I am using this examples page: http://www.ets-research.org/ia11ylab/aria11spec/

  1. In Example 27, I get ""Quarterly Report region slide clickable Heading Level 1 Quarterly Report". If I then down arrow, I hear "slide clickable Slide content". So in the first bit, the "region" is not suppressed, and in the second, the fact that I am in the slide is repeated. Also, these "clickable" announcements are irritating, since this is just plain text.

  2. In the next example, example 28, I hear "Slide Quarterly Report Clickable Heading Level 1 Quarterly Report". If I then downarrow, however, the paragraph that contains "Slide content" is being skipped alltogether. It's just not there. Object nav also doesn't find it.

And a more general observation, even outside the examples, every heading gets the "clickable" spoken, on this page, but also my blog and other pages.

Message end.

JL: I believe the first issue was also raised by someone.

Thanks.

@michaelDCurran
Merge branch 'master' into i8448
573096e
@michaelDCurran
Braille: honor roleText in browseMode.
20c09ea
@michaelDCurran
BrowseMode: only explicitly include the name of a landmark if it is n…
a447dde 
…ot going to be already presented as part of the field.
@michaelDCurran
Edge: mark controlFields as block if if they have a particular role (…
e787c57 
…grouping, paragraph or section currently). This ensures that they are appropriately presented as containers when roledescription is used.
@michaelDCurran

michaelDCurran commented Jul 17, 2018

Copy link
Copy Markdown
Member Author

I believe I have now addressed the most recent issues.
New changes:

  • Ensures that braille honors roledescription in browseMode (it already did in focus mode).
  • Ensures that the name of a field is not duplicated if the field is also a landmark.
  • Region is no longer reported for a landmark region that is also another valid field (E.g. a region with a roledescription).
  • roledescription on divs in Edge now present as block fields (I.e. do not keep reporting the roledescription for each line within the field).

LeonarddeR
LeonarddeR reviewed Jul 17, 2018
View reviewed changes

@LeonarddeR LeonarddeR left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I will do some testing with braille later today.

Comment thread source/NVDAObjects/UIA/edge.py Outdated
field=super(EdgeTextInfo,self)._getControlFieldForObject(obj,isEmbedded=isEmbedded,startOfNode=startOfNode,endOfNode=endOfNode)
field['embedded']=isEmbedded
role=field.get('role')
# Fields should be treeted as block for certain roles.

@LeonarddeR LeonarddeR Jul 17, 2018

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

treeted > treated

Comment thread source/browseMode.py Outdated
return " ".join(textList)

def getControlFieldBraille(self, field, ancestors, reportStart, formatConfig):
def getControlFieldBraille(self, field, ancestors, reportStart, formatConfig,reason=None):

@LeonarddeR LeonarddeR Jul 17, 2018

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is this extra reason kwarg ever used and different than None? I'd say you can also just pass reason=None to field.getPresentationCategory

LeonarddeR
LeonarddeR approved these changes Jul 17, 2018
View reviewed changes

@LeonarddeR LeonarddeR left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This looks ok to me, also from my braille display :)

There is one major thing I found, namely that by default, roles don't seem to show up in Edge browse mode at all, unless you're using quick nav. However, that is completely unrelated to this issue. Just making note of it for future reference.

@michaelDCurran michaelDCurran merged commit df1a68c into master Jul 17, 2018
@nvaccessAuto nvaccessAuto added this to the 2018.3 milestone Jul 17, 2018
@josephsl josephsl mentioned this pull request Jul 23, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@LeonarddeR LeonarddeR LeonarddeR approved these changes

@feerrenrut feerrenrut feerrenrut left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@michaelDCurran michaelDCurran

Labels

None yet

Projects

None yet

Milestone

2018.3

Development

Successfully merging this pull request may close these issues.

UIA/Edge: no aria-roledscription and other properties are rendered in browse mode

5 participants

@michaelDCurran @josephsl @LeonarddeR @feerrenrut @nvaccessAuto