Skip to content

handle braille updates in pump all and include updates for terminal changes#15156

Closed
LeonarddeR wants to merge 3 commits into
nvaccess:masterfrom
LeonarddeR:handleUpdateInPumpAll
Closed

handle braille updates in pump all and include updates for terminal changes#15156
LeonarddeR wants to merge 3 commits into
nvaccess:masterfrom
LeonarddeR:handleUpdateInPumpAll

Conversation

@LeonarddeR

@LeonarddeR LeonarddeR commented Jul 17, 2023

Copy link
Copy Markdown
Collaborator

Link to issue number:

Related to #15134. Partial fix for #15115

Summary of the issue:

When braille is tethered to review, braille isn't properly update when the text changes. The reason behind this is quite simple, as the braille handler doesn't update in these cases.

Description of user facing changes

When the text in a terminal changes and braille is tethered to review, the text on a braille display will now properly update if the review cursor is positioned on a changed line.

Description of development approach

  1. When reporting new text, also ensure that braille.handler.handleUdpate is called.
  2. Since it is now more likely that braille.handler.hanldeUpdate is called multiple times within one core cycle, this method will now only request one braille update per core cycle. This should also improve things in case many events are processed that trigger braille updates.
  3. braille.BrailleHandler.handlePendingCaretUpdate is now deprecated with no public replacement.

Testing strategy:

T.b.d.

Known issues with pull request:

None known

Change log entries:

Changes:

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 17, 2023 16:22
@LeonarddeR LeonarddeR requested a review from seanbudd July 17, 2023 16:22
@AppVeyorBot

AppVeyorBot commented Jul 17, 2023

Copy link
Copy Markdown

See test results for failed build of commit 7b61a75265

seanbudd
seanbudd reviewed Jul 18, 2023
View reviewed changes
Comment thread source/braille.py
finally:
region.pendingCaretUpdate=False
if version_year < 2024 and NVDAState._allowDeprecatedAPI():
def handlePendingCaretUpdate(self):

@seanbudd seanbudd Jul 18, 2023

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.

is this change going to materially break any add-ons?
I know we have created no-op aliases in the past, but this seems riskier than other instances.

@LeonarddeR LeonarddeR Jul 18, 2023

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'd be surprised if there is an add-on that uses this, though we cannot rule that out. However, I don't think it can do any harm. The only way I could think of for an add-on to use this is by explicitly updating the caret (i.e. calling handleCaretMove and handlePendingCaretUpdate in one wave). That would still behave the same as before.

@LeonarddeR LeonarddeR marked this pull request as draft July 18, 2023 05:32
@burmancomp

burmancomp commented Jul 18, 2023

Copy link
Copy Markdown
Contributor

I fetched this branch but unfortunately I have not yet found changes in behavior.

Content of braille line may change at least:

  • when text is changed with caret
  • text scrolls on screen when new text is printed with caret
  • when caret location does not seem to change but somethin however changes. For example when [ ] changes to [*] (I have noticed that in Linux with Putty where change of "check box" state is shown sometimes this way)
  • there is selected text for example in Notepad, and user changes "show selection" with gesture.

These changes should be updated automatically to braille line. I would prefer that these all cases would be solved.

There is also situation where user disables braille cursor but change is not updated to braille line automatically. I suppose this is different case from those above.

@LeonarddeR

LeonarddeR commented Jul 18, 2023

Copy link
Copy Markdown
Collaborator Author

We can not improve all cases in one pr, that would have too much impact I think. Every change should be carefully tested on its own. That said, the check boxes case should work theoretically, but I must test this.

@burmancomp burmancomp mentioned this pull request Jul 18, 2023
Merged
6 tasks
@XLTechie

XLTechie commented Jul 18, 2023 •
edited
Loading

Copy link
Copy Markdown
Collaborator

@LeonarddeR I think the kind of checkbox @burmancomp is talking about, is this (which you can run by placing it in a file and executing it, in WSL or whatever). I do not have a braille display, but I have used this kind of dialog before:

#!/bin/bash
# Check if whiptail is installed
which whiptail > /dev/null || {
echo "whiptail not installed"
exit 255
}
whiptail --clear --separate-output --checklist "Choose some options" 10 30 20 \
opt1 "Option 1" OFF \
opt2 "Option 2" OFF \
opt3 "Option 3" OFF \
opt4 "Option 4" OFF \
opt5 "And the last option" OFF

Move up/down through the options, space to check/uncheck, tab then enter to choose "OK".
Escape or enter should get you out.

@burmancomp

burmancomp commented Jul 18, 2023

Copy link
Copy Markdown
Contributor

@LeonarddeR I think the kind of checkbox @burmancomp is talking about, is this (which you can run by placing it in a file and executing it, in WSL or whatever).

Yes. The other "check box" may be such one which is activated with enter but that moves cursor I suppose.

@LeonarddeR

LeonarddeR commented Jul 26, 2023

Copy link
Copy Markdown
Collaborator Author

Closing this for now as I currently have no time to work on this and it doesn't work as advertised anyway.

@LeonarddeR LeonarddeR closed this Jul 26, 2023
seanbudd pushed a commit that referenced this pull request Aug 29, 2023
@burmancomp
Update braille independently on caret (#15163)
7b31977 
Fixes #15115

Summary of the issue:
This extends pull request #15134 and includes pull request #15156. Collaborated with @LeonarddeR.

Description of user facing changes
Braille line is updated in terminal window when there is text change.

In addition braille line is updated in any application when:

braille cursor is turned on or off with gesture
if there is selected text in edit control or document, changing "show selection" state with gesture updates braille line when otherwise supported
Description of development approach
Terminal window updates are handled with:

NVDAObjects.behaviors.Terminal.event_textChange and
event_UIA_notification in winConsoleUIA._NotificationsBasedWinTerminalUIA.
Objects are passed to BrailleHandler.handleUpdate function.

When braille show selection state is changed with gesture, BrailleHandler.initialDisplay function is executed.

When braille cursor is toggled with gesture, BrailleHandler._updateDisplay function is executed.
@LeonarddeR LeonarddeR deleted the handleUpdateInPumpAll branch August 23, 2025 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@seanbudd seanbudd seanbudd left review comments

Copilot code review Copilot Awaiting requested review from Copilot Copilot will automatically review once the pull request is marked ready for review

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@LeonarddeR @AppVeyorBot @burmancomp @XLTechie @seanbudd