Add support for IA2_ROLE_LANDMARK

[LeonarddeR]

Link to issue number:

Fixes #10101

Summary of the issue:

IA2 version 3 introduced IA2_ROLE_LANDMARK which is set on landmarks in recent Firefox and Chrome versions. This role was not supported.

Description of how this pull request fixes the issue:

1. Add a new role: controlTypes.ROLE_LANDMARK and map this appropriately
2. When speaking a landmark object in focus mode, also speak the landmark itself, e.g. banner landmark. This works in Internet Explorer, Firefox, Chrome, Edge legacy and Edge beta. This is accomplished by using the roleText attribute on objects.
3. IA2 virtual buffrs now also use the landmark role for landmark quick navigation.

Testing performed:

Tested the following snippet:

data:text/html,<header><button>I am a banner with a button in it</button></header>

When moving with tab to the button, ensure that banner landmark is spoken before the button name. Also make sure that the landmark is silenced when reporting of landmarks is off.

Known issues with pull request:

1. Strange enough, the landmark isn't spoken when navigating by landmark itself.
2. In internet explorer, HTML node name to aria landmark roles mapping still doesn't work.

Change log entry:

• New features
  • In Internet Explorer, Microsoft Edge and recent versions of Firefox and Chrome, landmarks are now reported in focus mode. (Support the IA2_ROLE_LANDMARK #10101)

[AppVeyorBot]

PR introduces Flake8 errors 😲

See test results for Failed build of commit 81d7d8ae95

[AppVeyorBot]

PR introduces Flake8 errors 😲

See test results for Failed build of commit 43028ba5ea

[MarcoZehe]

@MarcoZehe: Do you know in what exact Chrome version this support was introduced?

Unfortunately, no, I just found out through testing that the current version of Chrome does.

[LeonarddeR]

@feerrenrut The remaining lint error is about using IA2_ROLE_LANDMARK which is imported using from from comInterfaces.IAccessible2Lib import *. The IAccessibleHandler relies on this method of importing, if we don't, things get really painful.
I think we should either ignore these errors for now or come up with a documented alternative to ensure consistency across the code base.

[michaelDCurran]

My first thought would be that we should move towards importing it as

import comInterfaces.Ia2Lib as ia2

Then doing things like ia2.IA2_ROLE_LANDMARK

[LeonarddeR]

I like that. Probably will go for IA2 (upper case) for consistency reasons.

[michaelDCurran]

Are the types of landmarks presented in braille?
I wonder if it might be less complicated to just expose the landmark type via roleText (which is already supported in both speech and braille). E.g. the roleText for a landmark of type "main" could be "main landmark". This would remove the need for the specific code for landmarks in speakObjectProperties etc.

[LeonarddeR]

I like the idea of using roleText!

[LeonarddeR]

Ugh, problem with using roleText is actually that we couldn't use landmark abbreviations in braille.

[michaelDCurran]

Take a look at the nvaccess/ariaRoleDescriptionBraille branch. That implements roleTextBraille on NVDAObjects and handles it in braille.py. We won't want to take all of that code yet as aria-roleDescription-braille is not at all in the aria standard yet, but it was prototyped by Sina, Reef and myself last year at a math workshop. But you can certainly take the code in NVDAObjects/__init__.py and source/braille.py

[AppVeyorBot]

PR introduces Flake8 errors 😲

See test results for Failed build of commit 5687503491

[AppVeyorBot]

PR introduces Flake8 errors 😲

See test results for Failed build of commit 3d98393ae0

[LeonarddeR]

@michaelDCurran I finished the implementation based on roleText. Everything uses roleText for landmarks now, i.e. the special code for landmarks has been removed from browseMOde and control field speech now uses roleText if there is a landmark in the field.

Please note that the first known issue isn't caused by this pr.

Code snippet: <div role="search" aria-label="This is a search landmark">test</div>

Control field for the landmark: FieldCommand controlStart with {'runtimeID': (42, 329956, 4, 10), '_startOfNode': False, '_endOfNode': False, 'role': 86, 'states': set(), 'nameIsContent': False, 'name': 'This is a search landmark', 'description': '', 'level': None, 'embedded': False, 'isBlock': True, 'roleText': 'search landmark', 'landmark': 'search', 'current': None}

Note the _startOfNode value, which is False and really should be True here, as I were positioned at the start of the landmark. We only add the name if _startOfNode is True. This code is there for historical reasons, but I can probably remove it because the contro field presentation category now takes landmarks into account.

[LeonarddeR]

@michaelDCurran I finished the implementation based on roleText. Everything uses roleText for landmarks now, i.e. the special code for landmarks has been removed from browseMode and control field speech now uses roleText if there is a landmark in the field.

Please note that the first known issue isn't caused by this pr.

[AppVeyorBot]

PR introduces Flake8 errors 😲

See test results for Failed build of commit b9f5213e7f

[michaelDCurran]

@LeonarddeR just a reminder there are failing flake8 checks on this pr.

[LeonarddeR]

@michaelDCurran Sorry, should have been fixed now.

[michaelDCurran]

There is a possibility that a landmark will be ignored if the landmark is not the first role in the ARIA roles. E.g. role="list navigation".
For instance, when fetching xml-roles, after splitting, you should take the first role that also appears in aria.landmarkRoles, rather than just the first thing in the list.
In other words, the check for aria.landmarkRoles should be done in each implementation of the landmark property, rather than in the base roleText property, as this is too late.

[LeonarddeR]

Thanks for pointing this out. I actually wasn't aware that you are officially allowed to have multiple roles on a HTML node.

[LeonarddeR]

@michaelDCurran how do you actually expect role="list navigation" to be reported by NVDA?

In the current implementation (master), landmarks are treated separately from role, so this case will be reported as navigation landmark list. However, in the implementation in this pr, it is either navigation landmark or list. Current code even reports navigation landmark with 1 item because the underlying role is LIST, but roletext is overriden because it's a landmark, which is of course a bit problematic

[michaelDCurran]

Hmm that is a tricky one. I seem to think landmark should override role, but I can't really back up why I think that.

[LeonarddeR]

In firefox with IA2ROLE_LANDMARK support:

• <div role="list navigation"> is a list
• <div role="navigation list"> is a landmark

May be @MarcoZehe and @jcsteh would like to share their ideas?

[MarcoZehe]

Perhaps this section of the WAI-ARIA 1.1 spec or the listed Core-AM 1.1 section 5.4.1 help clarify: The list of tokens acts as a fallback mechanism, so if a user agent doesn't know the first role, it falls back to the second, etc. Since list is known, the landmark role, whichever that may be, will never be evaluated, and List wins. The first non-abstract role is what gets exposed. So if you have "list banner", "banner" would be considered a fall-back and would not get exposed since "list" is already there. Except for through the attribute XML-Roles, since that always gets the full token list.

[LeonarddeR]

Ugh, turns out that I entirely forgot about the braille implementation for this. I will file a fixup pr later this week. cc @bramd