Skip to content

Conversation

nvdaes
Copy link
Collaborator

@nvdaes nvdaes commented Aug 7, 2025

Link to issue number:

Fixes #7608

Summary of the issue:

Currently, spelling errors aren't presented in braille. This may negatively impact deaf-blind people and other braille users.

Description of user facing changes:

If NVDA is configured to report Font attributes in braille (or speech and braille), and to announce spelling errors, these mistakes would be presented in braille with the "m" letter (m stands for mistake).

Description of developer facing changes:

None.

Description of development approach:

Added a new item in the fontAttributeFormattingMarkers dictionary of braille.py for spelling errors (invalid-spelling).

Testing strategy:

Tested locally in Microsoft Word and Notepad, typing text detected as spelling errors.

Known issues with pull request:

None.

Code Review Checklist:

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

@coderabbitai summary

@josephsl
Copy link
Contributor

josephsl commented Aug 7, 2025 via email

@nvdaes nvdaes marked this pull request as ready for review August 7, 2025 18:29
@nvdaes nvdaes requested review from a team as code owners August 7, 2025 18:29
@CyrilleB79
Copy link
Contributor

@nvdaes, I have not tested this PR myself.

I acknowledge the need to have spelling errors reported in brailles, especially, but not only, for deaf blind users.
Though I am concerned by the UX provided in this PR. Having "Font attribute" setting controlling reporting spelling errors in braille makes Document formatting reporting confusing, more than today, I'd say.

Until now:

These 2 items were not linked. Now, the UX proposed in this PR links them, making the definition of a font attribute much less clear, and actually inexact.

And what if I want to have spelling errors reported in braille but not by speech?

IMO, to be able to customise how additional text information is reported in braille (font attributes, spelling mistakes, we need a specific dedicated panel for Braille formatting reporting.

@SaschaCowley
Copy link
Member

@nvdaes I agree with @CyrilleB79 that this should not be controlled by the "font attributes" option. I suggest that we change the "spelling errors" option to allow the user to select speech and/or braille as can be done with font attributes. This will require changing the way NVDA calculates the indicators in braille. I think the approach I would take would be to add a callable to FormattingMarker that returns True if the marker should be used if appropriate, and False if not. We can then refactor the code to use that callable rather than the overarching check as done currently. What do you think?

@SaschaCowley SaschaCowley marked this pull request as draft August 8, 2025 01:18
@nvdaes
Copy link
Collaborator Author

nvdaes commented Aug 8, 2025

@SaschaCowley and @CyrilleB79 , I see what Cyrille means and I agree with you, Sascha. I like your idea and I'll try to implement it.
@josephsl , m is delimited by unicode patterns like other attributes.

@CyrilleB79
Copy link
Contributor

Why did you use "m" letter while we have "Spelling errors" in the UI, and not "Mistakes"? Have you considered using "e" instead?
It's worth noting that "s" cannot be used since it is already used by "strikethrough"

@nvdaes
Copy link
Collaborator Author

nvdaes commented Aug 8, 2025

Why did you use "m" letter while we have "Spelling errors" in the UI, and not "Mistakes"? Have you considered using "e" instead? It's worth noting that "s" cannot be used since it is already used by "strikethrough"

Yes, in fact I have considered using "e". I'll change this and will continue my work when your PR to add ability to use audio is merged.
I think we can use a single line in the changelog mentioning our names and the corresponding issues, something like:

@CyrilleB79
Copy link
Contributor

@SaschaCowley you wrote:

@nvdaes I agree with @CyrilleB79 that this should not be controlled by the "font attributes" option. I suggest that we change the "spelling errors" option to allow the user to select speech and/or braille as can be done with font attributes.

What do you suggest regarding the GUI and taking into account the work in #17997 (see more specifically #17997 (comment))?

@nvdaes you wrote:

I think we can use a single line in the changelog mentioning our names and the corresponding issues, something like:

* Added ability to report spelling errors with audio and braille (@CyrilleB79, @nvdaes)

Feel free to do so when the final design is clearer and if you still find it appropriate.

@SaschaCowley
Copy link
Member

@CyrilleB79 thanks for bringing that to my attention. I'm not sure of a good solution at present. I'll talk to the team about it.

@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Aug 12, 2025
@SaschaCowley
Copy link
Member

@CyrilleB79 @nvdaes We think that a check list with "speech", "sound" and "braille" options is most appropriate in this case. These could be internally stored as a bit flag.

@nvdaes
Copy link
Collaborator Author

nvdaes commented Aug 12, 2025

Sascha wrote:

We think that a check list with "speech", "sound" and "braille" options is most appropriate in this case. These could be internally stored as a bit flag.

I agree with this proposal. I'll continue my work, as mentioned, when Cyrille's work about sound and speech is merged.

@CyrilleB79
Copy link
Contributor

How would you modify the toggle script?

@nvdaes
Copy link
Collaborator Author

nvdaes commented Aug 13, 2025

Cyrille wrote:

How would you modify the toggle script?

Probably I would remove it, but I would study other similar cases to decide the best way to proceed.

@SaschaCowley
Copy link
Member

@CyrilleB79 we could remove it, make the script cycle between all options, or add new scripts to toggle braille and speech/sound

@seanbudd

This comment was marked as off-topic.

@seanbudd seanbudd closed this Sep 2, 2025
@seanbudd seanbudd reopened this Sep 2, 2025
@seanbudd

This comment was marked as off-topic.

@CyrilleB79
Copy link
Contributor

@nvdaes #17997 has just been merged. You can now continue with this PR if you wish.

Regarding the toggle script, I suggest:

  • keep the existing script and adapt it to cycle through all possible audio feedback.
  • if you wish/find it useful, add a toggle script, still unassigned, to have spelling mistake reported in braille or not.

@nvdaes
Copy link
Collaborator Author

nvdaes commented Sep 5, 2025

Cyrille wrote:

Regarding the toggle script...

OK, I'll ask you if I have any questions when I continue working for this

BTW, in changes.md, I will mention just my username for this, since Jamie and you were mentioned in the PR about reporting errors with sounds, and this work is just an addition based on your previous work.

@nvdaes
Copy link
Collaborator Author

nvdaes commented Sep 7, 2025

I think we should make the following modes available to cycle with a script to report spelling errors:

  • Off.
  • Sounds.
  • Speech and sounds.
    And, additionally, a different script can be used to turn on and off reporting spelling errors in braille.

Probably the toggle integer value is not appropriate here.

@nvdaes
Copy link
Collaborator Author

nvdaes commented Sep 7, 2025

@CyrilleB79 , think that I've addressed your review.
I see that the toggleIntegerValue function is not documented in changes.md, but the toggle boolean function was documented, I haven't added a note about the toggleBinarieValue function, but perhaps it would be useful.
Also, to turn on and off single values of int flags like Reporting errors in braille ond or off, I used the tilde and it's not easy to find, at least not in all keyboards. Perhaps a function for this may be added.
If you don't have other comments, I'll request a review from NV Access soon, as this works for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Have spelling errors indicated in Braille when supported
5 participants