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

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

Closed
wants to merge 3 commits into from

Conversation

LeonarddeR
Copy link
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
Copy link

See test results for failed build of commit 7b61a75265

finally:
region.pendingCaretUpdate=False
if version_year < 2024 and NVDAState._allowDeprecatedAPI():
def handlePendingCaretUpdate(self):
Copy link
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.

Copy link
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
Copy link
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
Copy link
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.

@XLTechie
Copy link
Collaborator

XLTechie commented Jul 18, 2023

@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
Copy link
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
Copy link
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
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.
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

5 participants