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

Fix braille extension points not properly unregistered for braille viewer #15214

Merged
merged 3 commits into from Jul 31, 2023

Conversation

LeonarddeR
Copy link
Collaborator

@LeonarddeR LeonarddeR commented Jul 28, 2023

Link to issue number:

Fixes regression from #14503

Summary of the issue:

As part of #14503, braille extension points have been added that are used by the braille viewer. However when destroying the braille viewer by pressing alt+f4, the display size filter handler was never unregistered, resulting in the braille handler still assuming 40 braille cells even though no display was connected.

Description of user facing changes

Braille handler connectly restores to 0 cells again after disabling the braille viewer with alt+f4 when no braille display is connected.

Description of development approach

  1. Changed if check on the dialog to is not None explicitly. wx gives a destroyed window a bool value of False, so the boolean check would work.
  2. Ensure braille.filter_displaySize is unregistered when destroyBrailleViewer is called when the window is already destroyed. Note that destroying the window will unregister braille.pre_writeCells properly, so that unregistration can be limited to an undestroyed window only.

Testing strategy:

Tested closing the braille viewer either by toggling from the menu or by closing it with alt+f4.

Known issues with pull request:

None known

Change log entries:

Bug fixes

  • When no braille display is connected and the braille viewer is closed by pressing alt+f4 or clicking the close button, the display size of the braille subsystem will again be reset to no cells.

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.

@LeonarddeR LeonarddeR requested a review from a team as a code owner July 28, 2023 18:04
@LeonarddeR LeonarddeR requested review from seanbudd and removed request for a team July 28, 2023 18:04
seanbudd
seanbudd previously approved these changes Jul 30, 2023
@seanbudd
Copy link
Member

Does this need a changelog entry if it is unreleased regression (i.e. not in a final release)?

@seanbudd seanbudd added this to the 2023.2 milestone Jul 30, 2023
@LeonarddeR
Copy link
Collaborator Author

LeonarddeR commented Jul 31, 2023 via email

@seanbudd seanbudd merged commit 31436b6 into nvaccess:beta Jul 31, 2023
1 check was pending
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.

None yet

2 participants