Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an article role and allow quick navigation to articles #10424

Merged
merged 15 commits into from
Oct 29, 2019

Conversation

LeonarddeR
Copy link
Collaborator

@LeonarddeR LeonarddeR commented Oct 25, 2019

Link to issue number:

Fixes #9227

Summary of the issue:

It has been a long standing request to allow quick navigation from/to articles. Also, articles should be able to mentioned as such.

Description of how this pull request fixes the issue:

  1. Added a new article role, which is set to articles in virtual buffers and on NVDAObjects that are articles.
  2. Added the possibility for quick navigation scripts to be unbound by default.
  3. Added a quicknav script for articles, working in MSHTML and IA2 based browsers.

Testing performed:

Tested the following in Firefox, Chrome and IE:

  1. Articles are mentioned at such when using the tag or the article aria role when reporting of articles is enabled
  2. Quicknav works after assigning keys to the scripts.

Known issues with pull request:

The implementation for EdgeHTML is limited in that it works for nodes that have been marked up with the aria role of article. Articles marked up as such with the <article> tag aren't recognized, because they have the grouping role and the only way to detect whether it's an article is by means of the LocalizedControlType. Bah!

Change log entry:

  • New features
    • In Internet Explorer, Google Chrome and Mozilla Firefox, You can now navigate by article using quick navigation scripts. These scripts are unbound by default and can be assigned in the Input Gestures dialog when the dialog is opened from a browse mode document. (Support quick navigation by article in browse mode #9227)
  • Changes for developers
    • The aria.htmlNodeNameToAriaLandmarkRoles dictionary has been renamed to aria.htmlNodeNameToAriaRoles. It now also contains roles that aren't landmarks.

@AppVeyorBot

This comment has been minimized.

@AppVeyorBot

This comment has been minimized.

@LeonarddeR LeonarddeR marked this pull request as ready for review October 28, 2019 11:17
@LeonarddeR
Copy link
Collaborator Author

Just did a quick test with the UIA implementation of Edge Chromium. There, it looks like the aria role of article will be properly exposed, even when using the <aticle> tag. Therefore, I think that we should ignore the limitation in Edge for now.

"""Adds a script for the given quick nav item.
@param itemType: The type of item, I.E. "heading" "Link" ...
Copy link
Member

Choose a reason for hiding this comment

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

Why was this line removed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ugh, that must have been an accident. Restoring now.

@LeonarddeR
Copy link
Collaborator Author

In the current implementation, the "out of article" sentence is also spoken when leaving articles. For landmarks, this isn't the case. While landmarks and articles aren't the same, I feel they should behave equivalent in this case. Thus, either they should announce exit, or they shouldn't. See also #10420

@michaelDCurran michaelDCurran merged commit 2f9c7d4 into nvaccess:master Oct 29, 2019
@nvaccessAuto nvaccessAuto added this to the 2019.3 milestone Oct 29, 2019
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.

Support quick navigation by article in browse mode
4 participants