Skip to content

Initialize MathPlayer when NVDA starts #13486

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

Merged
merged 5 commits into from
Mar 17, 2022
Merged

Conversation

seanbudd
Copy link
Member

@seanbudd seanbudd commented Mar 16, 2022

Link to issue number:

Fixes #13463

Summary of the issue:

mathPres.ensureInit is called whenever math content is expected to be read, including from getTextWithFields.
This is called with every read when using UIA with Word.
If mathPlayer is not installed, this pollutes the log.
Ideally, math should be initialized at NVDA startup time.

Description of how this pull request fixes the issue:

Makes mathPlayer initialize on start up.
Removes mathPlayer.ensureInit usages from within NVDA.
External documentation referencing mathPlayer may need to be updated to reflect the need to restart NVDA after installing mathPlayer.

Testing strategy:

  • Test using mathPlayer in MS Word, confirm math is read
  • Test that if mathPlayer is not installed, a warning is logged at startup but at no other time.

Known issues with pull request:

None

Change log entries:

Changes

- NVDA needs to be restarted after installing MathPlayer. (#TODO)

For Developers

- ``mathPres.ensureInit`` has been removed, MathPlayer is now initialized when NVDA starts.

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

@seanbudd seanbudd requested review from a team as code owners March 16, 2022 00:52
@seanbudd seanbudd requested review from Qchristensen, michaelDCurran and LeonarddeR and removed request for a team March 16, 2022 00:52
@seanbudd seanbudd added this to the 2022.1 milestone Mar 16, 2022
michaelDCurran
michaelDCurran previously approved these changes Mar 16, 2022
Copy link
Member

@michaelDCurran michaelDCurran left a comment

Choose a reason for hiding this comment

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

Looks good. I confirmed NVDA's MathPlayer support still functions after this change. E.g. I was able to read and interact with math in MS Word.

@seanbudd seanbudd requested a review from michaelDCurran March 16, 2022 05:51
Copy link
Collaborator

@LeonarddeR LeonarddeR left a comment

Choose a reason for hiding this comment

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

Just a thought, should mathPres also be terminateable as well as be terminated/initialized in core.resetConfiguration?

@AppVeyorBot

This comment was marked as outdated.

@seanbudd seanbudd merged commit 3b35231 into beta Mar 17, 2022
@seanbudd seanbudd deleted the initializeMathPlayerOnNVDAStart branch March 17, 2022 00:37
AAClause added a commit to AAClause/BrailleExtender that referenced this pull request May 18, 2022
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.

4 participants