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

Braille doesn't follow when extending a selection to the left or right #5770

Closed
dkager opened this Issue Feb 22, 2016 · 28 comments

Comments

Projects
None yet
5 participants
@dkager
Collaborator

dkager commented Feb 22, 2016

One of these days I'm going to report an issue not related to braille. Until then...

STR:

  • Move to the start of a line of text that is longer than the braille display width.
  • Repeatedly press Ctrl+Shift+RightArrow.
    • Expected: braille pans to follow the right edge of the selection once it goes beyond the display width.
    • Actual: it doesn't.

The same in reverse applies when you extend the selection to the left. Extending up or down works as expected in most cases (but not in browse mode: #4682). I'm opening a separate issue for this because it concerns selections on a single line.

This is kind of the continuation of #5410, but I only noticed it just recently because now I use NVDA much more than before. My guess is that this is non-trivial to fix, in which case I'd say it's a low-priority issue.

@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh Feb 22, 2016

Contributor

I think the underlying issue is the same as #4682, just a slightly different manifestation. In short, NVDA has no idea which end of the selection is the anchor and which end is the active end, since most APIs don't give us that info. We did have some thoughts about how to fix this, but never got around to making them more solid/documenting them.

Contributor

jcsteh commented Feb 22, 2016

I think the underlying issue is the same as #4682, just a slightly different manifestation. In short, NVDA has no idea which end of the selection is the anchor and which end is the active end, since most APIs don't give us that info. We did have some thoughts about how to fix this, but never got around to making them more solid/documenting them.

@dkager

This comment has been minimized.

Show comment
Hide comment
@dkager

dkager Feb 23, 2016

Collaborator

It should be possible to have browse mode expose this info. As for Ctrl+Shift+arrows, this info could be obtained by looking at which keys the user is pressing, but that is ugly.

Collaborator

dkager commented Feb 23, 2016

It should be possible to have browse mode expose this info. As for Ctrl+Shift+arrows, this info could be obtained by looking at which keys the user is pressing, but that is ugly.

@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh Feb 23, 2016

Contributor

Sure, browse mode could certainly provide it, but because no other APIs
do, we never thought about how to fit it into the core abstraction. That
isn't to say it shouldn't; that's just why we haven't done it yet. :)

One way we could do it without changing the TextInfo API at all is to
treat the caret and selection positions separately even when there's a
selection. The caret position would indicate the correct end of the
selection. That's potentially a bit expensive, though, since APIs which
do expose this probably expose it as part of the selection.

A simpler, less expensive way of doing this is to have an
isAnchorAtStartOfSelection (or simila) attribute on
NVDAObject/TreeInterceptor. For everything other than browse mode, this
could just be calculated in EditableText.detectPossibleSelectionChange
based on which way the selection is moving. (We already get the old and
new selections in that method.) It should then be fairly trivial to have
braille use this info to do the right thing. This is also needed to fix
#250.

One open question, though: while this nicely fixes #4682, I'm worried
about the impact on #5410. If you are searching forward for text, this
will cause braille to pan to the end of the found text, not the start,
since the selection moved to the right. We can detect this case (the end
of the old selection won't be the start of the new), but we need to
somehow communicate this to braille. It's not true to say that the end
is the anchor, but we kind of want it to show that way initially.

Contributor

jcsteh commented Feb 23, 2016

Sure, browse mode could certainly provide it, but because no other APIs
do, we never thought about how to fit it into the core abstraction. That
isn't to say it shouldn't; that's just why we haven't done it yet. :)

One way we could do it without changing the TextInfo API at all is to
treat the caret and selection positions separately even when there's a
selection. The caret position would indicate the correct end of the
selection. That's potentially a bit expensive, though, since APIs which
do expose this probably expose it as part of the selection.

A simpler, less expensive way of doing this is to have an
isAnchorAtStartOfSelection (or simila) attribute on
NVDAObject/TreeInterceptor. For everything other than browse mode, this
could just be calculated in EditableText.detectPossibleSelectionChange
based on which way the selection is moving. (We already get the old and
new selections in that method.) It should then be fairly trivial to have
braille use this info to do the right thing. This is also needed to fix
#250.

One open question, though: while this nicely fixes #4682, I'm worried
about the impact on #5410. If you are searching forward for text, this
will cause braille to pan to the end of the found text, not the start,
since the selection moved to the right. We can detect this case (the end
of the old selection won't be the start of the new), but we need to
somehow communicate this to braille. It's not true to say that the end
is the anchor, but we kind of want it to show that way initially.

@dkager

This comment has been minimized.

Show comment
Hide comment
@dkager

dkager Feb 24, 2016

Collaborator

In Notepad++ when you Shift+DownArrow, braille shows the new line below the selection. I think this is how SuperNova works too. So I’m wondering if when extending the selection up/down, NVDA should show the last selected line or the first unselected line, if that makes sense.

I also just noticed that if you select downward in Windows Notepad the horizontal line of dots 7-8 is one cell longer than the text content. My first guess would be that this is because of the newline character at the end of the line. Not really an issue, but it may be a bit confusing.

Long story short, this definitely needs some more thinking. :)

Collaborator

dkager commented Feb 24, 2016

In Notepad++ when you Shift+DownArrow, braille shows the new line below the selection. I think this is how SuperNova works too. So I’m wondering if when extending the selection up/down, NVDA should show the last selected line or the first unselected line, if that makes sense.

I also just noticed that if you select downward in Windows Notepad the horizontal line of dots 7-8 is one cell longer than the text content. My first guess would be that this is because of the newline character at the end of the line. Not really an issue, but it may be a bit confusing.

Long story short, this definitely needs some more thinking. :)

@dkager

This comment has been minimized.

Show comment
Hide comment
@dkager

dkager May 5, 2016

Collaborator

If you are searching forward for text, this will cause braille to pan to the end of the found text, not the start, since the selection moved to the right. We can detect this case (the end of the old selection won't be the start of the new), but we need to somehow communicate this to braille.

Could we catch this by detecting when both the start and the end of the selection changed? In this case it isn't obvious which end is more important, so we could show the start initially. Even when searching backward you would probably want to see the start of the newly found text first.

One more detail: does it make sense to default to showing the start of the selection for RTL languages?

Collaborator

dkager commented May 5, 2016

If you are searching forward for text, this will cause braille to pan to the end of the found text, not the start, since the selection moved to the right. We can detect this case (the end of the old selection won't be the start of the new), but we need to somehow communicate this to braille.

Could we catch this by detecting when both the start and the end of the selection changed? In this case it isn't obvious which end is more important, so we could show the start initially. Even when searching backward you would probably want to see the start of the newly found text first.

One more detail: does it make sense to default to showing the start of the selection for RTL languages?

@dkager

This comment has been minimized.

Show comment
Hide comment
@dkager

dkager May 7, 2016

Collaborator

A simpler, less expensive way of doing this is to have an isAnchorAtStartOfSelection (or simila) attribute on NVDAObject/TreeInterceptor.

What I would like to propose is a tristate value:

SELECTION_ANCHOR_UNKNOWN = 0
SELECTION_ANCHOR_START = 1
SELECTION_ANCHOR_END = 2

But there doesn't seem to be a good single point of definition for this. My understanding is that at least EditableText and CursorManager need to be able to expose this value. But the nearest common ancestor is ScriptableObject, which is a bit too high up in the hierarchy.

Alternatively the property could be implemented as int, with negative value for start, positive for end, and zero for unknown. Or even a boolean that may also be None. Key point is that the unknown state needs to be conveyed somehow.

That said, calculating the anchor for the selection in EditableText turns out to be quite reliable in standard controls. This strategy looks like a good choice.

Collaborator

dkager commented May 7, 2016

A simpler, less expensive way of doing this is to have an isAnchorAtStartOfSelection (or simila) attribute on NVDAObject/TreeInterceptor.

What I would like to propose is a tristate value:

SELECTION_ANCHOR_UNKNOWN = 0
SELECTION_ANCHOR_START = 1
SELECTION_ANCHOR_END = 2

But there doesn't seem to be a good single point of definition for this. My understanding is that at least EditableText and CursorManager need to be able to expose this value. But the nearest common ancestor is ScriptableObject, which is a bit too high up in the hierarchy.

Alternatively the property could be implemented as int, with negative value for start, positive for end, and zero for unknown. Or even a boolean that may also be None. Key point is that the unknown state needs to be conveyed somehow.

That said, calculating the anchor for the selection in EditableText turns out to be quite reliable in standard controls. This strategy looks like a good choice.

@dkager

This comment has been minimized.

Show comment
Hide comment
@dkager

dkager May 7, 2016

Collaborator

Hum, just read up on the mix-in classes like CursorManager. I think I get that now, but am not entirely sure about the relation between NVDAObject and TreeInterceptor. I see that a braille.Region can contain either one as its obj reference. Does that cover all possibilities?

The developer guide mentions that events not processed by a TreeInterceptor are passed to the NVDAObject, but clearly this does not mean I can ignore TreeInterceptors. :)

Collaborator

dkager commented May 7, 2016

Hum, just read up on the mix-in classes like CursorManager. I think I get that now, but am not entirely sure about the relation between NVDAObject and TreeInterceptor. I see that a braille.Region can contain either one as its obj reference. Does that cover all possibilities?

The developer guide mentions that events not processed by a TreeInterceptor are passed to the NVDAObject, but clearly this does not mean I can ignore TreeInterceptors. :)

@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh May 8, 2016

Contributor
Contributor

jcsteh commented May 8, 2016

@dkager

This comment has been minimized.

Show comment
Hide comment
@dkager

dkager May 9, 2016

Collaborator

In the past, we discussed having some subclass of ScriptableObject for things which have a TextInfo; i.e. NVDAObject and DocumentTreeInterceptor. There didn't seem to be much point back then, but if we're adding more common stuff, maybe it makes sense now.

I think it makes sense if we use a tri-state variable with constants, but if it is going to be a boolean that alone doesn't justify this subclass IMO.

How do you intend to handle the unknown case for braille?

Currently I use a boolean that can be True/False/None. If True the selection is anchored at the start, else it is anchored at the end or unknown. In both of the latter cases I have braille scroll the start into view. My reasoning is that it makes the most sense for LTR languages, and if it's anchored at the end you obviously want to show the start since that can potentially move.

Here None can mean:

  • No (prior) selection.
  • Both ends of the selection moved. This does not take into account if the old and new selections overlap.

It is possible to add a value indicating the selection hasn't changed too, but I can't think of a use case. In fact, if instead we set a sane default directly in EditableText the None value is avoided. This seems good enough for now.

Collaborator

dkager commented May 9, 2016

In the past, we discussed having some subclass of ScriptableObject for things which have a TextInfo; i.e. NVDAObject and DocumentTreeInterceptor. There didn't seem to be much point back then, but if we're adding more common stuff, maybe it makes sense now.

I think it makes sense if we use a tri-state variable with constants, but if it is going to be a boolean that alone doesn't justify this subclass IMO.

How do you intend to handle the unknown case for braille?

Currently I use a boolean that can be True/False/None. If True the selection is anchored at the start, else it is anchored at the end or unknown. In both of the latter cases I have braille scroll the start into view. My reasoning is that it makes the most sense for LTR languages, and if it's anchored at the end you obviously want to show the start since that can potentially move.

Here None can mean:

  • No (prior) selection.
  • Both ends of the selection moved. This does not take into account if the old and new selections overlap.

It is possible to add a value indicating the selection hasn't changed too, but I can't think of a use case. In fact, if instead we set a sane default directly in EditableText the None value is avoided. This seems good enough for now.

@dkager

This comment has been minimized.

Show comment
Hide comment
@dkager

dkager May 10, 2016

Collaborator

Ugh, I'm running into an issue with scrolling to the right end of the selection. In WordPad you can apparently navigate onto the carriage return at the end of the line, and even select it. In NVDA 2016.1 this leads to the selection indicator (dots 7 and 8) extending one cell beyond the text. But if I deliberately scroll the selection's end into view I get a LookupError from regionPosToBufferPos. My first guess is that this is because TextInfoRegion trims line endings.

In Word 2016 I can also select line endings, but there braille doesn't show that such is happening. Should something be tweaked to avoid this oddness? What do sighted people see?

Note that you have to select forward for the carriage return to be included.

Collaborator

dkager commented May 10, 2016

Ugh, I'm running into an issue with scrolling to the right end of the selection. In WordPad you can apparently navigate onto the carriage return at the end of the line, and even select it. In NVDA 2016.1 this leads to the selection indicator (dots 7 and 8) extending one cell beyond the text. But if I deliberately scroll the selection's end into view I get a LookupError from regionPosToBufferPos. My first guess is that this is because TextInfoRegion trims line endings.

In Word 2016 I can also select line endings, but there braille doesn't show that such is happening. Should something be tweaked to avoid this oddness? What do sighted people see?

Note that you have to select forward for the carriage return to be included.

@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh May 11, 2016

Contributor
Contributor

jcsteh commented May 11, 2016

@dkager

This comment has been minimized.

Show comment
Hide comment
@dkager

dkager May 11, 2016

Collaborator

Regarding Wordpad, when you say you're scrolling the end into view, are you accounting for the fact that the end of a range is exclusive?

No. I'm scrolling to region.brailleSelectionEnd. The idea was that you would always see the next character to be included in the selection when extending. I was relying on the fact that there is always an extra space at the end of the region so the caret/insertion point can move beyond the last character. Clearly in WordPad this goes wrong.

This behavior was observed in SuperNova, so I took it as my starting point. Do you think it makes more sense to ensure the moving end of the selection is always visible instead?

Regarding Word 2016, not including line endings when expanding to line actually makes for a less confusing experience. I'd say WordPad is the weird one here. :) But if we use the inclusive selction as described above it's not a problem.

Collaborator

dkager commented May 11, 2016

Regarding Wordpad, when you say you're scrolling the end into view, are you accounting for the fact that the end of a range is exclusive?

No. I'm scrolling to region.brailleSelectionEnd. The idea was that you would always see the next character to be included in the selection when extending. I was relying on the fact that there is always an extra space at the end of the region so the caret/insertion point can move beyond the last character. Clearly in WordPad this goes wrong.

This behavior was observed in SuperNova, so I took it as my starting point. Do you think it makes more sense to ensure the moving end of the selection is always visible instead?

Regarding Word 2016, not including line endings when expanding to line actually makes for a less confusing experience. I'd say WordPad is the weird one here. :) But if we use the inclusive selction as described above it's not a problem.

@dkager

This comment has been minimized.

Show comment
Hide comment
@dkager

dkager May 11, 2016

Collaborator

Do you think it makes more sense to ensure the moving end of the selection is always visible instead?

On second thought, there is a serious problem with this. If you are selecting with shift+downArrow it is far more intuitive to see the next line to be selected rather than the end of the selection.

Collaborator

dkager commented May 11, 2016

Do you think it makes more sense to ensure the moving end of the selection is always visible instead?

On second thought, there is a serious problem with this. If you are selecting with shift+downArrow it is far more intuitive to see the next line to be selected rather than the end of the selection.

@dkager

This comment has been minimized.

Show comment
Hide comment
@dkager

dkager May 11, 2016

Collaborator

One more problem that I hope is a bug. Reproducible in Notepad.
In NVDAObjects.window.edit.Edit.event_caret():

...
        super(Edit,self).event_caret()
        self.detectPossibleSelectionChange()

The invocation of super also calls detectPossibleSelectionChange(). Could this duplicate call be avoided? It causes my anchor detection to fail because on the second call there is obviously no change. Stack traces attached.

i5770_stacks.txt

Collaborator

dkager commented May 11, 2016

One more problem that I hope is a bug. Reproducible in Notepad.
In NVDAObjects.window.edit.Edit.event_caret():

...
        super(Edit,self).event_caret()
        self.detectPossibleSelectionChange()

The invocation of super also calls detectPossibleSelectionChange(). Could this duplicate call be avoided? It causes my anchor detection to fail because on the second call there is obviously no change. Stack traces attached.

i5770_stacks.txt

@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh May 11, 2016

Contributor
Contributor

jcsteh commented May 11, 2016

@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh May 12, 2016

Contributor

No. I'm scrolling to region.brailleSelectionEnd. The idea was that you would always see the next character to be included in the selection when extending. I was relying on the fact that there is always an extra space at the end of the region so the caret/insertion point can move beyond the last character.

The thing is that in this case, there is no possible character beyond the last that the insertion point can move to. If we go exclusive end, I think we just need to handle that case and try for end - 1; I don't think there's a bug here as such. It's just an edge case.

Regarding Word 2016, not including line endings when expanding to line actually makes for a less confusing experience. I'd say WordPad is the weird one here. :)

Firefox, which includes line endings for <br> tags, etc., also includes the line ending character in the line. Having units whose edges don't meet is actually really annoying to code for and I'd argue it's broken behaviour, at least according to NVDA's abstraction (and most APIs). Including the line ending character makes lines consistent with words, where you should include the space at the end.

Contributor

jcsteh commented May 12, 2016

No. I'm scrolling to region.brailleSelectionEnd. The idea was that you would always see the next character to be included in the selection when extending. I was relying on the fact that there is always an extra space at the end of the region so the caret/insertion point can move beyond the last character.

The thing is that in this case, there is no possible character beyond the last that the insertion point can move to. If we go exclusive end, I think we just need to handle that case and try for end - 1; I don't think there's a bug here as such. It's just an edge case.

Regarding Word 2016, not including line endings when expanding to line actually makes for a less confusing experience. I'd say WordPad is the weird one here. :)

Firefox, which includes line endings for <br> tags, etc., also includes the line ending character in the line. Having units whose edges don't meet is actually really annoying to code for and I'd argue it's broken behaviour, at least according to NVDA's abstraction (and most APIs). Including the line ending character makes lines consistent with words, where you should include the space at the end.

@dkager

This comment has been minimized.

Show comment
Hide comment
@dkager

dkager May 12, 2016

Collaborator

Slight clarification here: the line ending is confusing because in braille it comes out as a space in some braille tables, and also the braille cursor doesn't move if the insertion point is located right after the last character and you press rightArrow to move it to the line ending. This because line endings are trimmed in braille.

My feeling is that using the exclusive selection end makes for a better experience, because it is better suited for selecting entire lines with shift+downArrow. It would be good to know what other screen readers do (besides SuperNova). Not that we should copy them just because, but this seems one of those areas that people have already given some thought to. With speech you do hear what was last selected, but in braille this gets confusing if a line is longer than the display length and you end up seeing e.g. only the last 5 characters of the line. Hence my preference for the exclusive end, which would show you the next thing to be selected.

Collaborator

dkager commented May 12, 2016

Slight clarification here: the line ending is confusing because in braille it comes out as a space in some braille tables, and also the braille cursor doesn't move if the insertion point is located right after the last character and you press rightArrow to move it to the line ending. This because line endings are trimmed in braille.

My feeling is that using the exclusive selection end makes for a better experience, because it is better suited for selecting entire lines with shift+downArrow. It would be good to know what other screen readers do (besides SuperNova). Not that we should copy them just because, but this seems one of those areas that people have already given some thought to. With speech you do hear what was last selected, but in braille this gets confusing if a line is longer than the display length and you end up seeing e.g. only the last 5 characters of the line. Hence my preference for the exclusive end, which would show you the next thing to be selected.

@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh May 12, 2016

Contributor

Slight clarification here: the line ending is confusing because in braille it comes out as a space in some braille tables

I think we strip line endings and always convert them to one space anyway, so this bit isn't applicable.

and also the braille cursor doesn't move if the insertion point is located right after the last character and you press rightArrow to move it to the line ending.

I don't follow this. If the insertion point is located right after the last character, it's already on the line ending, no?

My feeling is that using the exclusive selection end makes for a better experience, because it is better suited for selecting entire lines with shift+downArrow. ... With speech you do hear what was last selected, but in braille this gets confusing if a line is longer than the display length and you end up seeing e.g. only the last 5 characters of the line. Hence my preference for the exclusive end, which would show you the next thing to be selected.

I still don't quite follow this. If you're selecting an entire line, the next thing to be selected is the next line, but we definitely don't want to (and can't) show that.

Contributor

jcsteh commented May 12, 2016

Slight clarification here: the line ending is confusing because in braille it comes out as a space in some braille tables

I think we strip line endings and always convert them to one space anyway, so this bit isn't applicable.

and also the braille cursor doesn't move if the insertion point is located right after the last character and you press rightArrow to move it to the line ending.

I don't follow this. If the insertion point is located right after the last character, it's already on the line ending, no?

My feeling is that using the exclusive selection end makes for a better experience, because it is better suited for selecting entire lines with shift+downArrow. ... With speech you do hear what was last selected, but in braille this gets confusing if a line is longer than the display length and you end up seeing e.g. only the last 5 characters of the line. Hence my preference for the exclusive end, which would show you the next thing to be selected.

I still don't quite follow this. If you're selecting an entire line, the next thing to be selected is the next line, but we definitely don't want to (and can't) show that.

@dkager

This comment has been minimized.

Show comment
Hide comment
@dkager

dkager May 12, 2016

Collaborator

and also the braille cursor doesn't move if the insertion point is located right after the last character and you press rightArrow to move it to the line ending.
I don't follow this. If the insertion point is located right after the last character, it's already on the line ending, no?
Yes. I was trying to say that in braille you don't see whether there is a line ending as you move the cursor. There is always exactly one space after the text. But if you select text that contains a line ending that space is marked as being selected. That is:

  • foo\r is marked by 4 times dots 7 and 8.
  • foo (without carriage return) is marked by 3 times dots 7 and 8.

I still don't quite follow this. If you're selecting an entire line, the next thing to be selected is the next line, but we definitely don't want to (and can't) show that.
This is in fact what SuperNova does. NVDA also does this for some editors, but that is obviously not by design. But if it technically isn't possible in some editors we shouldn't do it. I haven't looked ahead that far just yet. :)

Collaborator

dkager commented May 12, 2016

and also the braille cursor doesn't move if the insertion point is located right after the last character and you press rightArrow to move it to the line ending.
I don't follow this. If the insertion point is located right after the last character, it's already on the line ending, no?
Yes. I was trying to say that in braille you don't see whether there is a line ending as you move the cursor. There is always exactly one space after the text. But if you select text that contains a line ending that space is marked as being selected. That is:

  • foo\r is marked by 4 times dots 7 and 8.
  • foo (without carriage return) is marked by 3 times dots 7 and 8.

I still don't quite follow this. If you're selecting an entire line, the next thing to be selected is the next line, but we definitely don't want to (and can't) show that.
This is in fact what SuperNova does. NVDA also does this for some editors, but that is obviously not by design. But if it technically isn't possible in some editors we shouldn't do it. I haven't looked ahead that far just yet. :)

@dkager

This comment has been minimized.

Show comment
Hide comment
@dkager

dkager May 13, 2016

Collaborator

I still don't quite follow this. If you're selecting an entire line, the next thing to be selected is the next line, but we definitely don't want to (and can't) show that.

Can this not be done by finding the last line of the selection and calling TextInfo.move(textInfos.UNIT_LINE, 1)?

An additional point of consideration is that some editors (Notepad++, Eclipse) notify braille when you press shift+downArrow, so the braille display follows while you're selecting lines. This uses the behavior of showing the next line to be selected, which I understand isn't what we want for NVDA. So the question is if we should prefer NVDA's selection detection over the caret events the editor sends during a selection. In short: which has higher priority, caret or selection?

Hopefully I've now found all the tricky corner cases!

Collaborator

dkager commented May 13, 2016

I still don't quite follow this. If you're selecting an entire line, the next thing to be selected is the next line, but we definitely don't want to (and can't) show that.

Can this not be done by finding the last line of the selection and calling TextInfo.move(textInfos.UNIT_LINE, 1)?

An additional point of consideration is that some editors (Notepad++, Eclipse) notify braille when you press shift+downArrow, so the braille display follows while you're selecting lines. This uses the behavior of showing the next line to be selected, which I understand isn't what we want for NVDA. So the question is if we should prefer NVDA's selection detection over the caret events the editor sends during a selection. In short: which has higher priority, caret or selection?

Hopefully I've now found all the tricky corner cases!

dkager added a commit to dkager/nvda that referenced this issue May 16, 2016

Store whether a selection is anchored at the start in NVDAObject and …
…TreeInterceptor. The property defaults to False.

Also use this to have braille scroll the selection's end into view if the anchor is at the start.
re #5770
@dkager

This comment has been minimized.

Show comment
Hide comment
@dkager

dkager May 16, 2016

Collaborator

Pushed some initial code I wrote last week. In particular, there is an attribute CursorManager._lastSelectionMovedStart which is similar but not the same as _isSelectionAnchoredAtStart. Maybe it's best to discuss this approach before extending it to multi-line selections. So far I have tested in Notepad, WordPad, Word and browse mode.

Collaborator

dkager commented May 16, 2016

Pushed some initial code I wrote last week. In particular, there is an attribute CursorManager._lastSelectionMovedStart which is similar but not the same as _isSelectionAnchoredAtStart. Maybe it's best to discuss this approach before extending it to multi-line selections. So far I have tested in Notepad, WordPad, Word and browse mode.

@dkager

This comment has been minimized.

Show comment
Hide comment
@dkager

dkager Jun 17, 2016

Collaborator

According to @leonardder JAWS also follows the end of the selection when you select multiple lines. So that should probably be the way to go, instead of showing the next line to be selected as SuperNova does. Especially since that is technically difficult or impossible.

Collaborator

dkager commented Jun 17, 2016

According to @leonardder JAWS also follows the end of the selection when you select multiple lines. So that should probably be the way to go, instead of showing the next line to be selected as SuperNova does. Especially since that is technically difficult or impossible.

@dkager

This comment has been minimized.

Show comment
Hide comment
@dkager

dkager Jun 22, 2016

Collaborator

Picking this patch up again, should finish it this week. There is one remaining task: get braille to show the moving end of a multi-line selection. So here's a heads up on my ideas for that.

The good stuff happens in TextInfoRegion.update(). This first grabs the cursor, expands it to a line or paragraph and then grabs the selection and restricts it to that reading unit. This goes wrong if the cursor is reported at one end of the selection while the other end is actually moving. I plan to work around this by replacing the cursor with the line at the moving end of the selection. This should result in the behavior as described above for JAWS.

It sounds feasible, but real-world tests will have to tell...

Collaborator

dkager commented Jun 22, 2016

Picking this patch up again, should finish it this week. There is one remaining task: get braille to show the moving end of a multi-line selection. So here's a heads up on my ideas for that.

The good stuff happens in TextInfoRegion.update(). This first grabs the cursor, expands it to a line or paragraph and then grabs the selection and restricts it to that reading unit. This goes wrong if the cursor is reported at one end of the selection while the other end is actually moving. I plan to work around this by replacing the cursor with the line at the moving end of the selection. This should result in the behavior as described above for JAWS.

It sounds feasible, but real-world tests will have to tell...

@dkager

This comment has been minimized.

Show comment
Hide comment
@dkager

dkager Jun 24, 2016

Collaborator

A few changes to braille.TextInfoRegion.update() seem to have done the trick, except for a few corner cases. However, my solution shows you the line that is about to be selected rather than the end of the last line that is selected. Presumably because of the way TextInfo.expand() is implemented.

That is exactly the behavior I was looking for initially. I spoke to @leonardder and he agrees. But @jcsteh said this is technically impossible and also not what should go into NVDA. How can we proceed? I can push my code so we can verify if it is technically sound, but how do we reach consensus on the desired behavior? Should we address the issue of single-line selection first and then the multi-line issue separately?

Collaborator

dkager commented Jun 24, 2016

A few changes to braille.TextInfoRegion.update() seem to have done the trick, except for a few corner cases. However, my solution shows you the line that is about to be selected rather than the end of the last line that is selected. Presumably because of the way TextInfo.expand() is implemented.

That is exactly the behavior I was looking for initially. I spoke to @leonardder and he agrees. But @jcsteh said this is technically impossible and also not what should go into NVDA. How can we proceed? I can push my code so we can verify if it is technically sound, but how do we reach consensus on the desired behavior? Should we address the issue of single-line selection first and then the multi-line issue separately?

@leonardder

This comment has been minimized.

Show comment
Hide comment
@leonardder

leonardder Jun 24, 2016

Collaborator

The reason why I agree is that, as soon as you select something on one line with shift+down arrow and your cursor was on the first character of that line initially, the cursor is now at the first character of the second line which is about to be selected when you press shift+down arrow again. Thus, I assume the cursor is on the second line visually as well, so that's the line the braille display should show.
I wonder, how about coding it in a way that, when panning braille backwards in such a case, the selected text is underlined with dot 7+8 to make the selection visible in braille? That will of course get in the way with 8 dot braille tables, but same applies to the cursor already.

Collaborator

leonardder commented Jun 24, 2016

The reason why I agree is that, as soon as you select something on one line with shift+down arrow and your cursor was on the first character of that line initially, the cursor is now at the first character of the second line which is about to be selected when you press shift+down arrow again. Thus, I assume the cursor is on the second line visually as well, so that's the line the braille display should show.
I wonder, how about coding it in a way that, when panning braille backwards in such a case, the selected text is underlined with dot 7+8 to make the selection visible in braille? That will of course get in the way with 8 dot braille tables, but same applies to the cursor already.

@dkager

This comment has been minimized.

Show comment
Hide comment
@dkager

dkager Jun 24, 2016

Collaborator

I wonder, how about coding it in a way that, when panning braille backwards in such a case, the selected text is underlined with dot 7+8 to make the selection visible in braille?

That doesn't currently work because panning past a line causes the TextInfo to be asked for the previous/next line, which typically moves the cursor.

The first paragraph Leonard wrote probably explains why this behavior is so intuitive to me. Similar logic can be applied to Shift+rightArrow: braille should pan if all but the last character visible on the display are selected, since the cursor is ahead of the selection. But I remember hearing that this is not how it works visually. Still, we should pick the most intuitive solution IMO.

One problem with this approach is that some TextInfos cause errors when they encompass the last character, you collapse it to the end and then expand to line. At first glance this collapses onto \n and then tries to expand. In Microsoft Word that causes a RuntimeError.

Collaborator

dkager commented Jun 24, 2016

I wonder, how about coding it in a way that, when panning braille backwards in such a case, the selected text is underlined with dot 7+8 to make the selection visible in braille?

That doesn't currently work because panning past a line causes the TextInfo to be asked for the previous/next line, which typically moves the cursor.

The first paragraph Leonard wrote probably explains why this behavior is so intuitive to me. Similar logic can be applied to Shift+rightArrow: braille should pan if all but the last character visible on the display are selected, since the cursor is ahead of the selection. But I remember hearing that this is not how it works visually. Still, we should pick the most intuitive solution IMO.

One problem with this approach is that some TextInfos cause errors when they encompass the last character, you collapse it to the end and then expand to line. At first glance this collapses onto \n and then tries to expand. In Microsoft Word that causes a RuntimeError.

dkager added a commit to dkager/nvda that referenced this issue Dec 4, 2016

Store whether a selection is anchored at the start in NVDAObject and …
…TreeInterceptor. The property defaults to False.

Also use this to have braille scroll the selection's end into view if the anchor is at the start.
re #5770
@dkager

This comment has been minimized.

Show comment
Hide comment
@dkager

dkager Dec 4, 2016

Collaborator

@jcsteh Could we pick this up again in 2017? I plan to read up on this issue in the week after Christmas. As far as I recall there are two issues:

  1. When a selection scrolls past the braille display length it isn't always tracked. This looked like it was relatively easy to fix.
  2. Braille stays on the top line when making a multi-line selection. At least two separate areas are affected, Microsoft Word and browse mode.

The rebased code lives in i5770. This is as far as I got and it doesn't work properly, but it showcases the idea we had about detecting the "selection anchor". I got quite good results with this approach, but in some TextInfos it caused exceptions.

Anyway, the festive season comes first! :)

Collaborator

dkager commented Dec 4, 2016

@jcsteh Could we pick this up again in 2017? I plan to read up on this issue in the week after Christmas. As far as I recall there are two issues:

  1. When a selection scrolls past the braille display length it isn't always tracked. This looked like it was relatively easy to fix.
  2. Braille stays on the top line when making a multi-line selection. At least two separate areas are affected, Microsoft Word and browse mode.

The rebased code lives in i5770. This is as far as I got and it doesn't work properly, but it showcases the idea we had about detecting the "selection anchor". I got quite good results with this approach, but in some TextInfos it caused exceptions.

Anyway, the festive season comes first! :)

@feerrenrut feerrenrut added the Braille label Dec 15, 2016

@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh Dec 22, 2016

Contributor

P2 because this prevents braille users from accessing selection properly.

Contributor

jcsteh commented Dec 22, 2016

P2 because this prevents braille users from accessing selection properly.

@jcsteh jcsteh added the p2 label Dec 22, 2016

jcsteh added a commit that referenced this issue Jul 4, 2017

@jcsteh jcsteh closed this in #6775 Aug 1, 2017

@nvaccessAuto nvaccessAuto added this to the 2017.3 milestone Aug 1, 2017

jcsteh added a commit that referenced this issue Aug 1, 2017

Braille now correctly follows the selection when selecting text beyon…
…d the width of the display. For example, if you select multiple lines with shift+downArrow, braille now shows the last line you selected. (issue #5770, PR #6775)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment