Skip to content

NVDAObjects.IAccessible: output alerts in braille#14565

Merged
seanbudd merged 7 commits into
nvaccess:masterfrom
josephsl:i14562IAccessibleAlertBrailleOutput
Jan 18, 2023
Merged

NVDAObjects.IAccessible: output alerts in braille#14565
seanbudd merged 7 commits into
nvaccess:masterfrom
josephsl:i14562IAccessibleAlertBrailleOutput

Conversation

@josephsl

@josephsl josephsl commented Jan 18, 2023

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #14562

Summary of the issue:

Alert event text is not shown on braille displays when the event occurs such as when downloading files via Google Chrome.

Description of user facing changes

NVDA will present alert messages in braille.

Description of development approach

Add braille output (braille.handler.message) to NVDAObjects.IAccessible.IAccessible.event_alert, borrowing heavily from notification behavior.

Testing strategy:

Manual testing:

  1. Visit websites such as community add-ons website (addons.nvda-project.org) where files can be downloaded.
  2. Download a file while using a braille display.

Before the PR: speech output only when alert event is fired.
After the PR: speech and braille outpu ocurs.

Known issues with pull request:

While Chrom's file download alert provides a good test case for testing braille output for alert event,s it might be possible that apps and web elements may not fire alert events, or if they do, presented in a way that NVDA doesn't know about (alert events are handled if the control role is alert).

Change log entries:

Bug fixes:

In web browsers such as Chrome and Firefox, alerts such as file downloads via Chrome are shown in braille in addition to being spoken. (#14562)

Code Review Checklist:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • API is compatible with existing add-ons.
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • Security precautions taken.

When processing MSAA alert event, only speech output was invoked whereas other implementations such as notification behavior class also brailled alerts and notifications. Therefore add braille output to alert event handler, both for the root alert and its children, borrowing heavily from notification behavior.
@josephsl josephsl requested a review from a team as a code owner January 18, 2023 01:59
@josephsl josephsl requested a review from seanbudd January 18, 2023 01:59
@josephsl

josephsl commented Jan 18, 2023

Copy link
Copy Markdown
Contributor Author

Hi,

I think this might be a good opportunity to introduce a generic alert event to base NVDA object or the notification behavior (perhaps in 2023.x or 2024.1) unless alert event is specific to IAccessible and/or UIA. Among other things, the default implementation would allow vision enhancement providers to offer a way to look at where the alert is coming from.

@Qchristensen, could you mind directing the user who reported the original issue to the issue you created and this PR? Or, if you want, do CC me in the converation you are having with the user.

Thanks.

if self in api.getFocusAncestors():
return
speech.speakObject(self, reason=controlTypes.OutputReason.FOCUS, priority=speech.Spri.NOW)
# Ideally, we wouldn't use getPropertiesBraille directly.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what would be better? why not do that? it would be good to include this in the comment.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it appears that using this directly is common practice, I think this comment can be droppped

@josephsl

josephsl commented Jan 18, 2023 via email

Copy link
Copy Markdown
Contributor Author

@seanbudd

Copy link
Copy Markdown
Member

I think this might be a good opportunity to introduce a generic alert event to base NVDA object or the notification behavior (perhaps in 2023.x or 2024.1) unless alert event is specific to IAccessible and/or UIA. Among other things, the default implementation would allow vision enhancement providers to offer a way to look at where the alert is coming from.

Would you be interested in doing this in this PR?

@josephsl

josephsl commented Jan 18, 2023 via email

Copy link
Copy Markdown
Contributor Author

Suggestion from Sean Budd (NV Access): since braille.handler.message comment is found in other places and the actual method is used elsewhere, remove the message comment.
@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit 4e57d3f244

@seanbudd seanbudd merged commit bcdf6ea into nvaccess:master Jan 18, 2023
@nvaccessAuto nvaccessAuto added this to the 2023.1 milestone Jan 18, 2023
@josephsl josephsl deleted the i14562IAccessibleAlertBrailleOutput branch January 19, 2023 22:40
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.

Report status in Braille when downloading files in Chrome

4 participants