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

When moving by words with text review commands in Scintilla based editors NVDA treats spaces as separate words #8295

Closed
lukaszgo1 opened this issue May 17, 2018 · 11 comments · Fixed by #10008
Labels
component/text-info
Milestone

Comments

@lukaszgo1
Copy link
Contributor

@lukaszgo1 lukaszgo1 commented May 17, 2018

Steps to reproduce:

  1. open a Scintilla based editor (I've tested with Notepad++ and TeXnicCenter).
  2. Write some text
  3. Move navigator object to focus.
  4. Start moving with Numpad 4 and 6 between words.

Expected behavior:

NVDA should report only words without spaces between them like in all other cases.

Actual behavior:

NVDA reports every space as a separate word.

System configuration:

NVDA version:
next-15133,1f3d46d2

NVDA Installed or portable:
Installed

Windows version:
Windows 8 x64

Name and version of other software in use when reproducing the issue:
Notepad++ v7.5.4 (64-bit), TeXnicCenter 2.02 Stable (64 bit)

Other questions:

Does the issue still occur after restarting your PC?
Yes
Have you tried any other versions of NVDA?
Yes from 2017,2 onward and it didn't work.

@leonardder
Copy link
Collaborator

@leonardder leonardder commented May 18, 2018

I can confirm this.

@leonardder leonardder added the component/text-info label May 18, 2018
@lukaszgo1
Copy link
Contributor Author

@lukaszgo1 lukaszgo1 commented Apr 23, 2019

It seems that by default space isn't considered a word separator by Scintilla so when calculating word start with SCI_WORDSTARTPOSITION and word end with SCI_WORDENDPOSITION space is a separate word. There is a Scintilla message SCI_SETWHITESPACECHARS which should fix this. I don't know however how to send space using it.

@leonardder
Copy link
Collaborator

@leonardder leonardder commented Apr 24, 2019

@lukaszgo1
Copy link
Contributor Author

@lukaszgo1 lukaszgo1 commented May 24, 2019

@leonardder I'm afraid I do not understand your last comment. When navigating with ctrl+arrows the editor determines what is a word separator and what isn't. When working with object review it is an NVDA job to determine this and since in Scintilla the word separators aren't set to space by default when NVDA sends Scintilla messages asking about start and end of given word space is returned.

@lukaszgo1
Copy link
Contributor Author

@lukaszgo1 lukaszgo1 commented May 24, 2019

cc @DataTriny given your recent work on Scintilla.

@Adriani90
Copy link
Collaborator

@Adriani90 Adriani90 commented May 24, 2019

and also cc: @francipvb

@leonardder
Copy link
Collaborator

@leonardder leonardder commented May 24, 2019

@lukaszgo1 commented on 24 May 2019, 14:02 CEST:

When navigating with ctrl+arrows the editor determines what is a word separator and what isn't.

This is true.

When working with object review it is an NVDA job to determine this.

This is not entirely true. When moving to the next word, NVDA asks the TextInfo object for the particular control to move to the next word. How a TextInfo object calculates word boundaries is mostly determined by the underlying TextINfo implementation and thus by the underlying control.

since in Scintilla the word separators aren't set to space by default when NVDA sends Scintilla messages asking about start and end of given word space is returned.

This actually makes sense. Moving with ctrl+arrows does include spaces, which is inconsistent.

@leonardder
Copy link
Collaborator

@leonardder leonardder commented May 24, 2019

I looked at the code and it looks like NVDA has a certain workaround in it to ensure that word offsets are correct, with the downside of space treated as a separator.

@leonardder
Copy link
Collaborator

@leonardder leonardder commented May 25, 2019

On closer inspection, at least for Notepad++, it is Scintilla that's failing here. Scintilla is definitely treating one space or a group of spaces as a word when using SCI_WORDSTARTPOSITION and SCI_WORDENDPOSITION.

@lukaszgo1
Copy link
Contributor Author

@lukaszgo1 lukaszgo1 commented May 25, 2019

Which is exactly what I am saying. According to this quote from the Scintilla documentation when we want Scintilla not to do that we have to set spaves as word separators ourselves.

SCI_SETWHITESPACECHARS(, const char *characters)
SCI_GETWHITESPACECHARS(, char *characters) → int
Similar to SCI_SETWORDCHARS, this message allows the user to define which chars Scintilla considers as whitespace. Setting the whitespace chars allows the user to fine-tune Scintilla's behaviour doing such things as moving the cursor to the start or end of a word; for example, by defining punctuation chars as whitespace, they will be skipped over when the user presses ctrl+left or ctrl+right. This function should be called after SCI_SETWORDCHARS as it will reset the whitespace characters to the default set. SCI_GETWHITESPACECHARS behaves similarly to SCI_GETWORDCHARS.

@leonardder
Copy link
Collaborator

@leonardder leonardder commented May 25, 2019

The problem is that the docs say:

for example, by defining punctuation chars as whitespace, they will be skipped over when the user presses ctrl+left or ctrl+right.

However, the movement behaviour of ctrl+left/rightArrow doesn't match with the offsets returned by SCI_WORDSTARTPOSITION and SCI_WORDENDPOSITION

lukaszgo1 added a commit to lukaszgo1/nvda that referenced this issue Jul 31, 2019
michaelDCurran pushed a commit that referenced this issue Sep 8, 2019
…nds in Scintilla based editors (#10008)

* No longer thread spaces as a separate words when using  text review commands in Scintilla based editors. Fix for #8295

* Make space a constant

* Add spaces in the commend to make linter work

* Additional changes for linter

* Last changes for linter
@nvaccessAuto nvaccessAuto added this to the 2019.3 milestone Sep 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/text-info
Projects
None yet
4 participants