Skip to content

show result of report current focus, navigator object, and selection in braille#15858

Merged
seanbudd merged 23 commits into
nvaccess:masterfrom
Emil-18:brailleReporting
Dec 8, 2023
Merged

show result of report current focus, navigator object, and selection in braille#15858
seanbudd merged 23 commits into
nvaccess:masterfrom
Emil-18:brailleReporting

Conversation

@Emil-18

@Emil-18 Emil-18 commented Nov 29, 2023

Copy link
Copy Markdown
Contributor

Link to issue number:

fixes #15844

Summary of the issue:

The report current focus and navigator object commands reports more information then NVDA normaly does by navigating to an object. Because of this, A user that only uses braille is unable to get this information. They are also unable to see acuratly what is selected if the selection is grater then one line, because when tethered to review, the selection information for text is lost, and when tethered to focus, the text is un selected when moving up or down a line.

Description of user facing changes

A user that only uses braille will be able to get the information described above

Description of development approach

I changed both the report focus and navigator object scripts, so that instead of calling speakObject, they call getObjectSpeech and saves its return value.
It then uses speech.speech.speak to speak this information
I then go over the list and remove every item that isn't a string, and then joins it with ' '.join
I then show this string in braille
In the reportCurrentSelection script, I get the information spoken using the _getSelectionMessageSpeech function, then show it in braille

Testing strategy:

I tested the report current focus and navigator scripts. Both worked as expected. I tested the report selection script in a situation where it was no selection, in a situation where the selection was small enough to be reported by NVDA, and in a situation where it was large enough that NVDA reported the number of characters selected. It worked in all those cases

Known issues with pull request:

None so far

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.

@burmancomp

Copy link
Copy Markdown
Contributor

As to showing selection in braille when braille is tethered to review, it should be possible to see selection directly when navigating right/left/up/down, when "show selection" is enabled in braille category.

@Emil-18

Emil-18 commented Nov 30, 2023

Copy link
Copy Markdown
Contributor Author

@burmancomp I agree, but for some reason when braille is tethered to review, it says that brlRegions[-1].selectionStart and selectionEnd is None even though some text are selected in the control. I have no idea why it happens

@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit 75493e59ab

@burmancomp

Copy link
Copy Markdown
Contributor

I do not have solution.

If you comment ReviewTextInfoRegion._getSelection function you should see selection, but then you cannot navigate up/down, and left/right navigation may repeat current text (at least if text is longer than braille line). Then if you look at TextInfoRegion.update function you should find line
unit = self._getReadingUnit()

and then you notice that unit is "line" or "paragraph".

@Emil-18 Emil-18 marked this pull request as ready for review December 5, 2023 21:15
@Emil-18 Emil-18 requested a review from a team as a code owner December 5, 2023 21:15
@Emil-18 Emil-18 requested a review from seanbudd December 5, 2023 21:15
@Emil-18

Emil-18 commented Dec 5, 2023

Copy link
Copy Markdown
Contributor Author

@burmancomp As I litterarly have no idea how to proceed, and since at least with this change, you can check the selection when braille is tethered to review by executing the reportSelection script, i am marking this as ready for review. Isn't it better if you create an issue that describes the exact problem and your prefered solution for it?

@seanbudd seanbudd left a 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.

Thanks @Emil-18, the approach generally looks good

Comment thread source/globalCommands.py
Comment thread source/globalCommands.py
Comment thread user_docs/en/changes.t2t Outdated
Comment thread source/globalCommands.py Outdated
Comment thread source/globalCommands.py Outdated
@seanbudd seanbudd marked this pull request as draft December 6, 2023 01:21
Emil-18 and others added 3 commits December 6, 2023 02:28
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
@burmancomp

Copy link
Copy Markdown
Contributor

Isn't it better if you create an issue that describes the exact problem and your prefered solution for it?

Possibility to check selection is always better than not to check it at all.

I suppose we both would prefer that one could navigate up/down/right/left, and if there is selection braille would indicate it without extra steps, and similarly when selecting text braille should automatically show selection.

I have no other solution at this moment (maybe never), and I do not know how much work it would require.

I am quite sure that @LeonarddeR have some thoughts about what is possible with moderate workload with current braille module.

@burmancomp

Copy link
Copy Markdown
Contributor

@Emil-18 it may likely take time to find and implement alternative solution, and because it is not available, it cannot be evaluated.

I would suggest that you would add browseable message alternative for example like in scripts script_reportLinkDestination / script_reportLinkDestinationInWindow.

Comment thread source/globalCommands.py Outdated
Comment thread source/globalCommands.py Outdated
Emil-18 and others added 3 commits December 7, 2023 20:08
Co-authored-by: Leonard de Ruijter <3049216+LeonarddeR@users.noreply.github.com>
@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit ac2dd66ff6

@Emil-18 Emil-18 marked this pull request as ready for review December 7, 2023 22:13

@seanbudd seanbudd left a 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.

Thanks @Emil-18

Comment thread source/globalCommands.py Outdated
Comment thread source/globalCommands.py Outdated
Comment thread user_docs/en/changes.t2t Outdated
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.

show result of report focus, report navigator object, and report selection in braille

6 participants