Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Panning braille past the visible text in Scintilla doesn't update the braille display #5678

Closed
dkager opened this Issue Jan 13, 2016 · 23 comments

Comments

Projects
None yet
4 participants
Collaborator

dkager commented Jan 13, 2016

Steps to reproduce:

  • Open a text-file in Notepad++. It has to be long enough to not fit on screen in its entirety.
  • Arrow down until the text begins scrolling.
  • Perform script_braille_nextLine or repeated script_braille_scrollForward.
    • Expected: the next line of text is shown on the braille display.
    • Actual: the text is scrolled but the the braille display isn't updated.

To fix this, arrow up/down once. To work around the issue use the arrow keys to move through text, this behaves as expected. This does not happen in Notepad or WordPad. Tried with a portable copy of Notepad++ to make sure it isn't caused by a local configuration change. I haven't looked much further but will do that if this doesn't fall into the cantfix category.

Contributor

jcsteh commented Jan 14, 2016

Collaborator

dkager commented Jan 14, 2016

I suppose visually the caret doesn't move. It stays at the same place but text is scrolled up by one line. Then it's curious that using the arrow keys works as expected. I'll have a look at the Scintilla API.

Collaborator

dkager commented Jan 15, 2016

It seems that NVDA is not catching a caret update. For example, when you press the braille display's routing buttons on a line of text the braille cursor doesn't update until the next blinking cycle. Or rather, the blinking cycle isn't reset just after a routing button is pressed. This does happen if you press the arrow keys instead.

I found that sending SCI_LINEDOWN instead of SCI_GOTOPOS works, but this message is of course not applicable everywhere. Also if you repeatedly perform script_braille_nextLine it keeps sending the same offset, which is what leads me to think that NVDA is missing a state update somewhere.
Fun fact: I dealt with a similar issue roughly two years ago when SuperNova had problems tracking the caret when scrolling text. They do quite a bit of scraping, NVDA never had these problems and still doesn't when using the arrow keys. I was also a bit more naive back then. But for reference, see here.

@dkager dkager changed the title from Panning braille past the visible text in Notepad++ doesn't update the braille display to Panning braille past the visible text in Scintilla doesn't update the braille display Jan 15, 2016

Collaborator

dkager commented Jan 15, 2016

Also reproduced in SciTE, so updated the issue title to be more generic.

Collaborator

derekriemer commented Jan 15, 2016

Cintilla bug maybe?

On 1/15/2016 9:21 AM, Davy Kager wrote:

Also reproduced in SciTE, so updated the issue title to be more generic.


Reply to this email directly or view it on GitHub
#5678 (comment).


Derek Riemer
  • Department of computer science, third year undergraduate student.
  • Proud user of the NVDA screen reader.
  • Open source enthusiast.
  • Member of Bridge Cu
  • Avid skiier.

Websites:
Honors portfolio http://derekriemer.com
Awesome little hand built weather app!
http://django.derekriemer.com/weather/

email me at derek.riemer@colorado.edu mailto:derek.riemer@colorado.edu
Phone: (303) 906-2194

Collaborator

dkager commented Jan 15, 2016

That is possible. Or maybe a feature: the rationale could be that if you programmatically change the caret position you might not need a subsequent notification that the caret moved.

From: derekriemer [mailto:notifications@github.com]
Sent: Friday, January 15, 2016 17:41
To: nvaccess/nvda nvda@noreply.github.com
Cc: Davy Kager mail@davykager.nl
Subject: Re: [nvda] Panning braille past the visible text in Scintilla doesn't update the braille display (#5678)

Cintilla bug maybe?

On 1/15/2016 9:21 AM, Davy Kager wrote:

Also reproduced in SciTE, so updated the issue title to be more generic.


Reply to this email directly or view it on GitHub
#5678 (comment).


Derek Riemer

  • Department of computer science, third year undergraduate student.
  • Proud user of the NVDA screen reader.
  • Open source enthusiast.
  • Member of Bridge Cu
  • Avid skiier.

Websites:
Honors portfolio http://derekriemer.com
Awesome little hand built weather app!
http://django.derekriemer.com/weather/

email me at derek.riemer@colorado.edu mailto:derek.riemer@colorado.edu mailto:derek.riemer@colorado.edu
Phone: (303) 906-2194


Reply to this email directly or view it on GitHub #5678 (comment) . https://github.com/notifications/beacon/AC9y8YlqysKXnSN1_0EbJNQHYA_kN2pwks5paRidgaJpZM4HEEPV.gif

Contributor

jcsteh commented Jan 17, 2016

Collaborator

dkager commented Jan 18, 2016

I believe Scintilla also uses another caret, maybe even an invisible one, to support assistive technology. To this end it sometimes explicitly updates it, including after you press the down arrow (SCI_LINEDOWN). On the other hand the API docs for SCI_GOTOPOS clearly state that the view is scrolled to make the caret visible. I’ll do some more testing…

From: James Teh [mailto:notifications@github.com]
Sent: Sunday, January 17, 2016 23:40
To: nvaccess/nvda nvda@noreply.github.com
Cc: Davy Kager mail@davykager.nl
Subject: Re: [nvda] Panning braille past the visible text in Scintilla doesn't update the braille display (#5678)

If we're missing a caret update, most likely, a caret event wasn't fired. Because Scintilla uses the standard caret (at least, I think it does), this makes me wonder whether the caret is actually moving visually when you move it using braille. It'd be great if we can have someone with working eyeballs check this.


Reply to this email directly or view it on GitHub #5678 (comment) . https://github.com/notifications/beacon/AC9y8VuTG1yw8mlAPV-28icydjpLS-a_ks5pbA-9gaJpZM4HEEPV.gif

Collaborator

dkager commented Jan 22, 2016

Looking at Scintilla's code and running some tests it appears that the system caret is updated very frequently (upon every blink I suppose).
When I perform script_braille_nextLine, then Alt+tab out of the window and back in, the cursor is shown at the correct line. Furthermore, if I add some value such as 80 to the offset so that the command moves more than a single line NVDA also tracks the cursor properly, or at least it shows the newly focused line. NVDA also tracks the cursor properly if the cursor is anywhere else than on the first character of a line. That is, you can right arrow a few times, then do script_braille_nextLine and NVDA will show the newly focused line. Of course this command puts the cursor on the first character of that line so subsequent calls will fail again.
This isn't the assurance a sighted person can give, but from what I can tell Scintilla is behaving as it should. From the above I'm starting to wonder if there is some optimization that says "if the x and y coordinates of the caret haven't changed, don't update anything".

Another, possibly related issue is that sometimes when you press a cursor routing button NVDA moves the caret to the wrong position on a totally different line. By looking at NVDAObjects\window\scintilla.py the cursor routing buttons always use actual offsets into the text, not some kind of position guessing based on x-y coordinates. Could it be that both issues can be blamed on this one function _setCaretOffset?

Contributor

jcsteh commented Jan 23, 2016

Oh. Now I'm wondering whether perhaps the caret doesn't actually move at all as far as its screen location is concerned. Perhaps the new line scrolled on to the screen is at the same position as the old line, so the caret is actually in the same place. Perhaps Scintilla just redraws the text when it scrolls and doesn't have to "move" the caret in this case. That would mean we wouldn't get a locationChange event for the caret (which NVDA translates into a caret event).

That doesn't explain why pressing down arrow works, though. Maybe down arrow scrolls more than one line when it hits the bottom but SCI_GOTOPOS doesn't? I think we need some sighted help here.

Another, possibly related issue is that sometimes when you press a cursor routing button NVDA moves the caret to the wrong position on a totally different line.

That one doesn't make sense. Was the right line displayed in the first place? Perhaps it wasn't, in which case it's probably a missing caret event.

By looking at NVDAObjects\window\scintilla.py the cursor routing buttons always use actual offsets into the text, not some kind of position guessing based on x-y coordinates.

I don't understand what you mean here. Using offsets is definitely the correct way to do things. Using coordinates will cause problems if the screen scrolls visually or the like without moving the caret. It's also far less reliable; mapping characters to screen coordinates and vice versa is an ugly, sometimes asymmetric business.

Collaborator

dkager commented Jan 23, 2016

As far as SuperNova and its screen scraping can tell me pressing down arrow also scrolls single lines, although I suppose it could be scrolling partial lines.

By looking at NVDAObjects\window\scintilla.py the cursor routing buttons always use actual offsets into the text, not some kind of position guessing based on x-y coordinates.
I don't understand what you mean here. Using offsets is definitely the correct way to do things.

Exactly. So pressing cursor routing buttons should always be accurate but for some reason it isn't.

Collaborator

dkager commented Jan 23, 2016

Small update: just added some debugging code. There is only one place where the Win32 API function SetCaretPos is called. When you scroll past the visible text using the down arrow key, neither the x nor the y parameter that is passed to this function changes. Are there other functions that would trigger a change event for NVDA? Sighted help is a bit hard to come by this afternoon but I'll try for that later.

Contributor

jcsteh commented Jan 24, 2016

Collaborator

dkager commented Jan 24, 2016

[...] does SCI_GOTOPOS definitely result in SetCaretPos getting called?

Based on my current Notepad++ settings:

  • SetCaretPos is called periodically, my educated guess is on every blink.
  • It is also called whenever an arrow key is pressed, even if no movement actually happened or when the text scrolled but the caret stayed in place.
  • It is not called after you send SCI_GOTOPOS, which might explain why this command tends to feel more laggy than using the arrow keys: NVDA might not get alerted until the next blink. There is one exception...
  • It is called after you send SCI_GOTOPOS if the cursor stayed in place but the text scrolled. This is exactly the situation that NVDA is struggling with.

Not sure if this is the issue. Maybe Scintilla sends a redraw or other GUI update when using the arrow keys but not when using SCI_GOTOPOS. The two commands (SCI_LINEDOWN and SCI_GOTOPOS) have distinct code paths so it's hard to compare the two. Maybe I should throw up an issue at the Scintilla project, since I'm pretty sure NVDA really isn't receiving any events.

Two possible issues:

  1. Fix this in Scintilla.
  2. Make the braille thumb keys send SCI_LINEDOWN~ andSCI_LINEUP`. This has the added (and subjective) advantage that if you put the caret in the middle of a line and scroll down with braille, the caret won't bounce back to the start of the line. The disadvantage is that it won't work for the implicit line down you get when panning to the right and reaching the end of a line.
Contributor

jcsteh commented Jan 24, 2016

Collaborator

dkager commented Jan 25, 2016

Judging by the following traceback this event should be fired by the Scintilla object, not its TextInfo. I haven't tried this yet.

ERROR - eventHandler.executeEvent (20:48:15):
error executing event: caret on <NVDAObjects.window.scintilla.ScintillaTextInfo object at 0x0609AAB0> with extra args of {}
Traceback (most recent call last):
  File "eventHandler.py", line 137, in executeEvent
    sleepMode=obj.sleepMode
AttributeError: 'ScintillaTextInfo' object has no attribute 'sleepMode'
Collaborator

dkager commented Jan 25, 2016

Uh, so now I tried because it was super easy and I didn't know. This solution appears to work well and braille is much more responsive now. When you route the cursor with the routing buttons the move is immediately visible instead of having to wait till the next braille cursor blink. So naturally I am happy. Not going to do a PR since solution is but a single line, as per James:
eventHandler.executeEvent("caret", self.obj)

I also found a way to reproduce the 'routing to the wrong offset' issue:

  • Move the cursor a few lines down into a document and position it somewhere into the line, say at column 5 or so.
  • Press the cursor routing button above where the cursor is at that moment.
    • Expected: nothing happens
    • Actual: the cursor moves up 4 lines

Worth its own ticket you think?

Collaborator

derekriemer commented Jan 25, 2016

I think I raised this. It has to do with the fact that double tap of
braille routing buttons cause a click gesture to be performed which
happens to click in the center of the window for sintilla. This is not
the best solution of course. @jcsteh knows more.

On 1/25/2016 1:13 PM, Davy Kager wrote:

Uh, so now I tried because it was super easy and I didn't know. This
solution appears to work well and braille is much more responsive now.
When you route the cursor with the routing buttons the move is
immediately visible instead of having to wait till the next braille
cursor blink. So naturally I am happy. Not going to do a PR since
solution is but a single line, as per James:
|eventHandler.executeEvent("caret", self.obj)|

I also found a way to reproduce the 'routing to the wrong offset' issue:

  • Move the cursor a few lines down into a document and position it
    somewhere into the line, say at column 5 or so.
  • Press the cursor routing button above where the cursor is at that
    moment.
    o Expected: nothing happens
    o Actual: the cursor moves up 4 lines

Worth its own ticket you think?


Reply to this email directly or view it on GitHub
#5678 (comment).


Derek Riemer
  • Department of computer science, third year undergraduate student.
  • Proud user of the NVDA screen reader.
  • Open source enthusiast.
  • Member of Bridge Cu
  • Avid skiier.

Websites:
Honors portfolio http://derekriemer.com
Awesome little hand built weather app!
http://django.derekriemer.com/weather/

email me at derek.riemer@colorado.edu mailto:derek.riemer@colorado.edu
Phone: (303) 906-2194

Collaborator

dkager commented Jan 25, 2016

Ah, that would explain things. For the record, I do only click once, but indeed in a place where the cursor already was.

From: derekriemer [mailto:notifications@github.com]
Sent: Monday, January 25, 2016 21:31
To: nvaccess/nvda nvda@noreply.github.com
Cc: Davy Kager mail@davykager.nl
Subject: Re: [nvda] Panning braille past the visible text in Scintilla doesn't update the braille display (#5678)

I think I raised this. It has to do with the fact that double tap of
braille routing buttons cause a click gesture to be performed which
happens to click in the center of the window for sintilla. This is not
the best solution of course. @jcsteh knows more.

On 1/25/2016 1:13 PM, Davy Kager wrote:

Uh, so now I tried because it was super easy and I didn't know. This
solution appears to work well and braille is much more responsive now.
When you route the cursor with the routing buttons the move is
immediately visible instead of having to wait till the next braille
cursor blink. So naturally I am happy. Not going to do a PR since
solution is but a single line, as per James:
|eventHandler.executeEvent("caret", self.obj)|

I also found a way to reproduce the 'routing to the wrong offset' issue:

  • Move the cursor a few lines down into a document and position it
    somewhere into the line, say at column 5 or so.
  • Press the cursor routing button above where the cursor is at that
    moment.
    o Expected: nothing happens
    o Actual: the cursor moves up 4 lines

Worth its own ticket you think?


Reply to this email directly or view it on GitHub
#5678 (comment).


Derek Riemer

  • Department of computer science, third year undergraduate student.
  • Proud user of the NVDA screen reader.
  • Open source enthusiast.
  • Member of Bridge Cu
  • Avid skiier.

Websites:
Honors portfolio http://derekriemer.com
Awesome little hand built weather app!
http://django.derekriemer.com/weather/

email me at derek.riemer@colorado.edu mailto:derek.riemer@colorado.edu mailto:derek.riemer@colorado.edu
Phone: (303) 906-2194


Reply to this email directly or view it on GitHub #5678 (comment) . https://github.com/notifications/beacon/AC9y8eGmKMr7lgGcrNYV1XVD4_UxXtZCks5pdn2hgaJpZM4HEEPV.gif

Contributor

jcsteh commented Jan 27, 2016

I think I raised this. It has to do with the fact that double tap of
braille routing buttons cause a click gesture to be performed which
happens to click in the center of the window for sintilla.

Oh, yeah. That's #4703.

@jcsteh jcsteh added this to the 2016.2 milestone Jan 27, 2016

@jcsteh jcsteh self-assigned this Jan 27, 2016

Collaborator

dkager commented Jan 28, 2016

So the routing buttons moving to an unexpected place is clearly another issue.

Do you feel the fix for this panning issue should be added to the 'official' app module? It's a work-around, but probably faster than bringing this up with the Scintilla developer. Especially since if it is fixed upstream it then also has to cascade to third-party applications like Notepad++. If this kind of hack doesn't belong in NVDA I'll bake my own app module. :)

Contributor

jcsteh commented Jan 28, 2016

jcsteh added a commit that referenced this issue Mar 2, 2016

In Scintilla based editors such as Notepad++, braille is now updated …
…correctly when moving the cursor using a braille display.


Fixes #5678.

Incubated in 07b2c92.

@jcsteh jcsteh closed this in f04b53e Mar 31, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment