While reading text, play a sound for spelling errors instead of saying "spelling error".

[jcsteh]

Link to issue number:

Fixes #4233 .

Summary of the issue:

We already use a sound to indicate spelling errors while typing. However, while reading text, we currently report "spelling error". It is more efficient to use a sound instead, since the sound can be played in parallel and is also not conflated with the text itself.

Description of how this pull request fixes the issue:

1. Update getFormatFieldSpeech to produce a WaveFileCommand instead of "spelling error".
2. Tweak speakTextInfo so that when reporting a single character, it reports initial fields for a non-empty sequence, regardless of whether the sequence contains text strings. Without this change, when moving by character into a spelling error (with no other initial fields), the sound was not played because the sequence didn't contain any text strings.

Testing performed:

1. Opened this test case in Firefox:
   data:text/html,<textarea>This is some textt. Each sentence contanes a single spelling error. Coming up with theese errors is quite amusing.
2. Focused the textarea.
3. Read through the textarea by line, say all and word. Heard the buzz sound alongside each spelling error.

Known issues with pull request:

1. This currently completely replaces the "spelling error" text. I guess some users might not like this. However, we don't currently have a mechanism for configuring sounds instead of spoken messages. We do have the "Play sound for spelling errors while typing" option, but that lives in Keyboard Settings and it's probably odd to have that setting control this functionality.
2. I'm not quite sure why speakTextInfo was testing for strings in the initial fields sequence. I can't think of a reason for this now. This was implemented 8 years ago in 82c4bdc. Testing for a non-empty sequence should be sufficient.

Change log entry:

New features:
- While reading text, NVDA now plays a sound to indicate spelling errors instead of saying "spelling error".

[LeonarddeR]

Have you considered to make it configurable, just like with indentation reporting? I can think of cases where people still prefer speech, i.e. because thy considder te spelling sound a bit harsh. Also,may be I'm missing something, but could you describe the 'out of spelling error' behavior as well as how grammar is covered here?

[jcsteh]

Have you considered to make it configurable

I mentioned that consideration above. I worry that we don't have a good framework for configuring sounds vs messages for these things, so we'll end up with special case config options/checks scattered throughout. However:

just like with indentation reporting?

I actually forgot about this one; as you say, there's already precedent for this, including the UI. I'll follow that pattern.

Also,may be I'm missing something, but could you describe the 'out of spelling error' behavior

This hasn't changed. It doesn't really make sense to say "out of {buzz}", so we still say "out of spelling error". While admittedly inconsistent, you only get this when moving by character or word, and I don't think there's a better alternative.

as well as how grammar is covered here?

Grammar isn't changed here. Even though the sound is called textError, I noticed we don't report grammar errors while typing. I guess it makes less sense while typing, though, since grammar errors are usually broader than a word. What do you think?

[MarcoZehe]

Also,may be I'm missing something, but could you describe the 'out of spelling error' behavior

This hasn't changed. It doesn't really make sense to say "out of {buzz}", so we still say "out of spelling error". While admittedly inconsistent, you only get this when moving by character or word, and I don't think there's a better alternative.

A different sound maybe, like a lower buzz?

[LeonarddeR]

I mentioned that consideration above. I worry that we don't have a good framework for configuring sounds vs messages for these things, so we'll end up with special case config options/checks scattered throughout.

I share that concern. May be @feerrenrut has some valuable ideas here?

as well as how grammar is covered here?

Grammar isn't changed here. Even though the sound is called textError, I noticed we don't report grammar errors while typing. I guess it makes less sense while typing, though, since grammar errors are usually broader than a word. What do you think?

I think not covering grammar errors makes the uX a bit weird. We're talking about spelling errors now, but I'm pretty sure this also covers grammar errors. Now if one chooses to only play a sound for spelling errors, either grammar errors might be silenced or still reported as grammar errors with speech.

May be:

• Play the sound for both spelling and grammar
• Rename spelling errors in the gui to spelling and grammar errors

[LeonarddeR]

The following just popped into my mind.

getControlFieldSpeech and getFormatFieldSPeech currently add a lot of strings to a speech sequence. It is therefore not possible to process them later on.

Something like this:

1. Add a base class: speech.commands.ProxyCommand. Also add speech.commands.TextErrorCommand, which subclasses ProxyCommand
2. A ProxyCommand can behave as a dynamic command, holding an inner command that is either a string when we're talking about reporting spelling errors with text, and a WaveCommand when we expect SPelling errors to be reported by a wave command.
3. When speech encounters a ProxyCommand, it should stay in the sequence. The speech manager should be responsible for getting the inner command from the proxy.

In any case, it makes getControlFieldSpeech less complex.

[jcsteh]

Interesting idea. It simplifies getControlFieldSpeech/getFormatFieldSpeech, but at the cost of architectural complexity; i.e. it's harder to follow the "flow". I'm not sure I see a practical benefit. That is, why do we need to process this later on? What benefit does that bring? Taking this to its logical end, many things would eventually become speech commands (emphasis, links, etc.). So sure, we've taken the decision making out of the speech functions, but we still have the code to determine whether to insert those objects, plus now we have hundreds of little command classes.

[LeonarddeR]

why do we need to process this later on? What benefit does that bring?

I could think of NVDA Remote, where the controlling machine wants spelling errors to be reported with sounds whereas the controlled machine uses speech to report them. However, while I can see the benefit of that, it could also cause major confusion about expected behavior.

Taking this to its logical end, many things would eventually become speech commands (emphasis, links, etc.). So sure, we've taken the decision making out of the speech functions, but we still have the code to determine whether to insert those objects, plus now we have hundreds of little command classes.

What I actually consider a problem in the current situation where all these control fields are exposed as strings in a sequence, is that it is still very difficult for add-ons to modify the behavior. Imagine an add-on like audio themes, for example. What if audio themes wants to play a sound for a certain role? I assume it will still have to monkeypatch getPropertiesSpeech at some point.

If contro/format fields are somehow distinguishable as speech commands, or getPropertiesSPeech could use a command that adds some metadata to it, adding customisability to add-ons is as simple as allowing them to filter the speech sequence.

I realize now that this is kind of a different subject, may be.

For #9937, I did something like this:

class _TextChunk(str):
+	"""str subclass to distinguish normal text from field text when processing text info speech."""

[Brian1Gaff]

We did have some input before re sounds vs speech from those with hearing issues, that the buzz and certain other sounds were either not heard or tended to be painful to listen to. So in the end some kind of setting for speech and or sound would be nice to have in many cases.

[jcsteh]

What I actually consider a problem in the current situation where all these control fields are exposed as strings in a sequence, is that it is still very difficult for add-ons to modify the behavior.

That's fair.

Imagine an add-on like audio themes, for example. What if audio themes wants to play a sound for a certain role? I assume it will still have to monkeypatch getPropertiesSpeech at some point.

I thought about this in the past. getPropertiesSpeech isn't so bad because you could have a filter called at the start of the function which takes the input dict and an output sequence, removes stuff it consumes (e.g. role) and throws stuff into the output sequence (e.g. a sound for the role). Since role isn't present in the dict any more, the normal presentation won't occur. This doesn't work for getFormatFieldSpeech, though, because it can do both entry (spelling error) and exit (out of spelling error). Still, I think I'd prefer to see filters at specific points like this rather than overloading the SpeechCommand processing stage, though there are valid arguments either way.

I do think this is out of scope here, though. :)

[jcsteh]

Removing review request until I update this to be configurable. It seems I can't change this to a draft PR now. :(

[feerrenrut]

I do think this is out of scope here, though. :)

I agree. Although this may not be the best place to have it, this discussion is quite valuable. Sorry for making your PR noisey Jamie!

I have been imagining the SpeechSequences to eventually contain ONLY commands of different types. We might add a plainText speech type to start to replace str values, these could have a single "source" eg "name", "role", "description", "NVDA Speech UI". In my opinion this would offer the greatest flexibility with presentation. I actually think perhaps speech commands should be able to "wrap" other speech sequences, This can help with the "into x" and "out of x" complexity. Essentially we would be making the structure a tree rather than a flat sequence.

Some features I would like to use this to enable:

• Customisable / themeable speech ording and verbosity.
• More semantics in speech viewer. This would make accessibility testing more pleasant, at a glance being able to know speech is coming from role vs description vs somewhere else

[jcsteh]

I kinda like the idea of having metadata on commands indicating where they came from. As Leonard suggested, that would allow add-ons to do most things by just processing speech sequences. Arguably, it's weird to have a command that an add-on may then just throw away and replace (rather than altering the presentation before the command was generated in the first place), but it's probably not that costly and it probably simplifies things. It also solves the speech viewer presentation nicely. On the other hand, having sequences as a tree terrifies me. I'm not convinced it really simplifies the entering/exiting stuff; it just shifts the burden of figuring it out into a lower layer. Also, whether you report entry/exit often depends heavily on context; e.g. moving by character. For example, if a link was a nested sequence, the sequence would later have to be processed to figure out whether to report entry/exit anyway. At some point, it has to be collapsed down to a linear series of actions.

[feerrenrut]

For example, if a link was a nested sequence, the sequence would later have to
be processed to figure out whether to report entry/exit anyway. At some
point, it has to be collapsed down to a linear series of actions.

Yes, I'm thinking that collapsing nodes of the tree at a presentation layer allows for more control and flexibility for how to speak things. Tying the "speech UI", (ie whether to announce "link", speak links at all, beep, or change tone) to the contents of the link has some nice benefits.

Also, whether you report entry/exit often depends heavily on context; e.g. moving by character.

I was mostly thinking about solving the problem of ensuring symmetry, but of course there are situations where asymmetry might be a feature.

[feerrenrut]

@jcsteh Indicated that this should be changed to a draft with #10474 (comment)

Converting this into a draft until the remaining work is completed.

[codeofdusk]

Any updates on this PR?

[LeonarddeR]

@jcsteh Happy to take this one if you'd like.

[jcsteh]

You're welcome to if you'd like, but I also don't want to earn a reputation for expecting others to clean up my half-baked pull requests. :) I probably need to get better at not submitting them at all until they're more complete.

[Adriani90]

@LeonarddeR are you still planning to take this one?
#7599 has been merged a while ago, so this might have introduced a possibility to separate what is reported by speech and what is reported by tones. Also this is already possible for capital letter, they can be indicated either by tones or speech which can be set in the voice settings category. So maybe we can make use of the same concept here as well?

One further concern we should be aware of:
If capitals are indicated by tones, and we introduce a spelling error tone while reading character by character, this might make things more complicated to distinguish.
So how about introducing tones for spelling errors only when reading by all chunks of text but not character by character?
This would make sure that we don't interfere with capitals tones and the navigation character by character still reports "spelling error" instead of playing a tone.

[LeonarddeR]

I'm afraid I don't have time currently.

[jcsteh]

This PR already uses speech refactor (#7599). It uses the spelling error buzz sound already heard after typing a word with a spelling error. However, I never got around to adding configurability to this PR, and #7599 doesn't provide any generic UI for that.

[seanbudd]

@jcsteh - should we close this abandoned?