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

An error in a regular expression dictionary entry can result in entry deletion rather than giving the user a correction opportunity #11407

Closed
XLTechie opened this issue Jul 22, 2020 · 2 comments · Fixed by #11409
Milestone

Comments

@XLTechie
Copy link
Contributor

Steps to reproduce:

  1. Open the temporary speech dictionary: NVDA+n, p, d, t.
  2. Press Alt+A to add a new entry.
  3. Set the pattern as "\(test\)", and the replacement as "\1".
  4. Press Alt+E to set as regular expression.
  5. Press enter.
  6. Press enter.

Note: this is a test string to demonstrate the case. When I encountered this issue, I had entered a very long regexp (about 200 characters), and made some kind of grouping error with one of the groups.

Actual behavior:

NVDA waits until after the dictionary dialog is closed (not the add dialog, the enclosing dictionary dialog). It then:

  • Plays an error sound.
  • Irrecoverably deletes the added dictionary entry.
  • Records something like the following in the log:
ERROR - speechDictHandler.SpeechDict.sub (20:42:36.514) - MainThread (5848):
Invalid dictionary entry 1 in temporary dictionary: "\(test\)", invalid group reference 1 at position 1

Possibly even worse than the addition case I demonstrated here, if you edit an existing entry, and cause a grouping error, the entry is also deleted, rather than, at least, reverting back to its pre-edited state.

Expected behavior:

The dictionary entry is validated when the entry add/edit dialog attempts to close, and if it fails, the user remains in the dialog, and is given a chance to continue work on the entry, without having to re-enter the whole thing as a new entry.

The dictionary already does this for non-grouping errors. For example, try a pattern like: "(\", and a replacement of "test". In that case, when hitting OK to accept the entry, a dialog is shown as follows:

Regular Expression error: "bad escape (end of pattern) at position 1".

Pressing OK in that dialog, returns you to the add/edit dialog, with the existing pattern and replacement still available for further editing.
That is what I would expect for group errors as well.

Furthermore, I'm testing this on alpha, with debug logging. I don't know if a user with disabled logging on a stable branch, would even hear the error beep. If not, then it means the user has no idea this happened.

System configuration

Installed NVDA alpha-20566,dfbda72e, no other versions tested.
Windows 10 Version 2004 (OS Build 19041.388)

Add-ons, reboot, and reg fix tool: N/A

@Adriani90
Copy link
Collaborator

cc: @JulienCochuyt

@JulienCochuyt
Copy link
Collaborator

Indeed, some errors arise only when using the regex, compiling is not enough.
Adding a substitution attempt after compiling solves at least the simple reported example.
Upcoming PR.

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 a pull request may close this issue.

4 participants