nvaccess / nvda

NVDA, the free and open source Screen Reader for Microsoft Windows
Other
2.08k stars 626 forks source link

Feature request: Include Sentence Nav as part of NVDA #8518

Open andrew-l-d opened 6 years ago

andrew-l-d commented 6 years ago

Tony Malykh's Sentence Nav add-on is a most valuable resource. Not only does it resolve #5805, but it allows reading by sentence in a wide variety of applications. I humbly suggest that the facility to read by sentence should be an integral component of a screen reader. I have checked with Tony and he is happy for me to make this suggestion. Andrew

josephsl commented 2 years ago

Ping @MichaelDCurran, @feerrenrut, and other NV Access folks.

cary-rowen commented 2 years ago

From the user's point of view, I think this issue has been discussed enough, I really hope it will appear in the NVDA core, I use this add-on every day, it works fine in Chinese language.

cary-rowen commented 2 years ago

@seanbudd

mltony commented 2 years ago

@michaelDCurran, @feerrenrut, @seanbudd Michael, Reef, Sean, Could you guys tell us about your attitude on incorporating sentence navigation into NVDA? As if would you be ok if I prepare a PR with sentence navigation? As you can see in this thread so many users want to have this feature in NVDA core, yet nobody from NVAccess showed up in this thread so far. This leaves NVDA users guessing, whether you haven't seen this thread, or have some concerns about sentence navigation or just decided not to respond for some reason? In my opinion it would be totally fine to say "no, we don't want Sentence Navigation in NVDA because of X,Y and Z", especially since you asked contributors to discuss with you first before sending PRs. But so far we don't even have a reason as to why you might dislike sentence navigation and whether they could be mitigated if they are technical in nature. A quick recap if you don't want to read this whole thread: SentenceNav is a mature add-on, that adds sentence navigation in editables and browse mode. It didn't require almost any maintenance in the last couple of years besides compatibility updates, so I expect maintenance cost to be minimal. If there is a green light from NVAccess I will prepare a PR (not sure about timeline though). If we don't hear from you this time, we would have to assume that NVAccess refused to consider including sentence navigation and declines to provide any comments or justifications.

seanbudd commented 2 years ago

@mltony - Thanks for providing a summary of this thread. To be able to provide a helpful response, NV Access needs to schedule time to read this thread and discuss outcomes. I've added this to a list of issues for us to look at.

cary-rowen commented 2 years ago

Hi, @seanbudd , Any news about this issue?

andrew-l-d commented 2 years ago

I find the lack of movement on this issue very disappointing. As stated previously, reading by sentence is a crucial facility and is available in other screen readers. I understand that Tony will not be updating SentenceNav in the immediate future, so I will likely not upgrade when NVDA 2022.1 is released.

josephsl commented 2 years ago

Hi, note that a compatible version of SentenceNav was released for 2022.1. Thanks.

andrew-l-d commented 2 years ago

Thanks Joseph. SentenceNav has indeed been updated.

bhavyashah commented 1 year ago

A gem of a quote from https://github.com/nvaccess/nvda/issues/8518#issuecomment-538504462: "... primarily since only 10% of NVDA users know what is an add-on and how to install one, and that's probably an overly optimistic statement." Not sure of the accuracy of the statistic - my intuition is that it is - but I resonate with the sentiment. I was teaching NVDA to a beginner recently, and I realized how many add-ons I had to have him install in order to perform natural screen reading tasks he asked about. Anyhow, pinging @seanbudd in case there have been any updates since https://github.com/nvaccess/nvda/issues/8518#issuecomment-1030941610. No worries if this fell through the cracks (happens to the best of us!), but please consider prioritizing sharing NV Access's position at the earliest given the age of this ticket and the level of interest.

andrew-l-d commented 1 year ago

Due to other commitments, it took Tony some time to update SentenceNav to be compliant with NVDA 2023.1 and how frustrating it was for me. I intended to nag ... no, remind about this glaring omission in NVDA but did not get around to it. The above comment has coaxed me into action.

Andrew

cary-rowen commented 1 year ago

We merged #13797. We've also added a Document Navigation category to the Settings panel. Sentence navigation is similar. What is NV Access's current stance on this feature request? Any updates?

CyrilleB79 commented 7 months ago

@seanbudd, @gerald-hartig, given #16031 implements an algorithm to detect sentence separators, it would really be the opportunity to state on this issue and to triage it. Also, the more because @mltony (author of sentence nav add-on) is available these days (on contrary to usual) and is quite productive; it would be a pity to miss this opportunity.

NV Access has planned to look at this issue two years ago (see https://github.com/nvaccess/nvda/issues/8518#issuecomment-1030941610), but it seems that nothing has happened since then.

michaelDCurran commented 7 months ago

We will take this feature. We already have moving by sentence in MS Word, using alt+up/downArrow. I have not looked at the add-on itself, but all I would ask is that it was abstract enough so that a particular TextInfo implementation can provide its own sentence unit if the underlying accessibility / platform API has a concept of sentences. Then in the cases where there is no native support, we will use the generalised algorithm. There is still the concern of alt+up/downArrow in browse mode being used to manipulate open combo boxes. How would this conflict be addressed?

CyrilleB79 commented 7 months ago

We will take this feature. We already have moving by sentence in MS Word, using alt+up/downArrow.

Thanks @michaelDCurran for the "triaged" label.

I have not looked at the add-on itself, but all I would ask is that it was abstract enough so that a particular TextInfo implementation can provide its own sentence unit if the underlying accessibility / platform API has a concept of sentences. Then in the cases where there is no native support, we will use the generalised algorithm.

@mltony are you still interested in implementing this in core with the design suggested by @michaelDCurran, i.e. a general algorithm in TextInfo to define sentence unit, that could be redefined by a specific subclass of Textinfo?

There is still the concern of alt+up/downArrow in browse mode being used to manipulate open combo boxes. How would this conflict be addressed?

Jaws also uses alt+up/downArrow in browse mode. It may be worth looking at how it behaves on webpages containing comboboxes.

Anyway, I am not concerned about this. If we cannot fin a solution to handle this conflict, we still can add Sentence Nav in core with another non-conflicting gesture or with unassigned gestures, what would already be a plus with respect to what we have today (i.e. nothing except in Word).

DrSooom commented 7 months ago

@michaelDCurran: I changed these hotkeys from ALT+ArrowUP/ArrowDown to NVDA+ALT+ArrowUP/ArrowDown.

mltony commented 7 months ago

Thanks @michaelDCurran , I will start working on that. Re Alt+Down conflict: currently we handle it by checking the role of current element and in case it's combo box we toggle it instead of doing sentence navigation. AFAIK that'also how Jaws does it. I see there's textInfos.UNIT_SENTENCE constant, I will try to rewrite SentenceNav so that it makes use of that framework. Also one thing to mention is that SentenceNav tries to reconstruct sentences spanning across multiple paragraphs. I found it useful in two cases: when someone sends an email using GMail Basic HTML interface - in this case every line turns into its own paragraph, so sentences do span across multiple paragraphs most of the time; and the other use case is reading scientific papers, where PDF format has the same problem. SO the algorithm that reconstructs sentences across paragraphs is somewhat complex. I will add plenty of comments describing what is being done there, but I thought just wanted to call that out now.

CyrilleB79 commented 7 months ago

Also one thing to mention is that SentenceNav tries to reconstruct sentences spanning across multiple paragraphs. I found it useful in two cases: when someone sends an email using GMail Basic HTML interface - in this case every line turns into its own paragraph, so sentences do span across multiple paragraphs most of the time; and the other use case is reading scientific papers, where PDF format has the same problem. SO the algorithm that reconstructs sentences across paragraphs is somewhat complex. I will add plenty of comments describing what is being done there, but I thought just wanted to call that out now.

I am unsure that the paragraph reconstruction of Sentence Nav add-on should be ported as is in NVDA core. Indeed, I think that NVDA's TextInfo has its own notion of paragraph. If there are situations where paragraphs are not detected correctly, it should be fixed in NVDA globally, i.e. should also impact paragraph navigation commands and paragraph selection commands.

cary-rowen commented 7 months ago

@DrSooom I prefer Alt+Up/Down which is consistent with MS Word. Also, I've been using sentence navigation for several years and haven't been bothered by the conflict between Alt+down to expand combo boxes and sentence navigation.

cary-rowen commented 7 months ago

Also one thing to mention is that SentenceNav tries to reconstruct sentences spanning across multiple paragraphs. I found it useful in two cases: when someone sends an email using GMail Basic HTML interface - in this case every line turns into its own paragraph, so sentences do span across multiple paragraphs most of the time; and the other use case is reading scientific papers, where PDF format has the same problem. SO the algorithm that reconstructs sentences across paragraphs is somewhat complex. I will add plenty of comments describing what is being done there, but I thought just wanted to call that out now.

For me, this works really well, and my use of sentence-nav relies heavily on this feature. I'm sure you'll want to review your code to make it more robust before bringing sentence-nav into NVDA. There are currently several Open Issues for sentence-nav that you might want to take a look at.

mltony commented 7 months ago

@CyrilleB79 , I created a separaet issue #16089 to discuss whether it is feasible to fix paragraphing. TLDR, I don't see a good solution and I explain there why existing algorithm of SentenceNav won't work well for fixing paragraphing. But LMK if you have any other good ideas.

mltony commented 7 months ago

@CyrilleB79, I didn't hear back from you regarding fixing paragraph definition #16089 - I assume you don't have any good ideas there, so since I don't see a good way to fix paragraph definition myself either, I'll plan to preserve SentenceNav logic and only reconstruct sentences across paragraphs.

CyrilleB79 commented 7 months ago

I have no better idea to detect paragraph accurately, and there will probably a need to do some tests.

Anyway, I think that the algorithm to reconstruct paragraphs for sentence nav feature should be available in the "Paragraph style" parameter of the "Document navigation" settings category. We may then decide if sentence nav feature should follow this parameter to decide of paragraph reconstruction or if the type of paragraph should specifically be forced to the better algorithm for this feature.

mltony commented 6 months ago

@seanbudd, @michaelDCurran I am currently working on sentence navigation PR and I am running into following problem. I already have an algorithm that detects sentences in array of strings - where each string represents a paragraph. Let's call that array strings. Suppose a sentence starts at strings[0][5]. The problem I am struggling with is how to transform that offset 5 into TextInfo representing start of sentence. I have textInfo representing paragraph. What I figured out is that textInfo.move(UNIT_CHARACTER, 5) will not work because of different encodings in Python vs the app. For browser textInfos I can use offsets and I can use textUtils.WideStringOffsetConverter to figure out the right offset. For Notepad++ I can I can write a similar utf-8 converter class and also compute offsets. But what should I do for non-offset text infos? For example, notepad appears to be using UIA textInfo which is not offset based. I've been playing with UIA textInfos and I am slowly figuring out there are many different edge cases. For example in Microsoft Word (I am not targeting word in in my PR, but just want to illustrate quirks of UIA textInfos) - if we have a bulleted list and the cursor is at the beginning of the first item, then textInfos.move(UNIT_CHARACTER, 1) would actually move over 3 characters in textual representation: the bullet character, whitespace and actual first character of list item. So far my best idea is to have a number of trial-and-error attempts. Something like:

def moveCharacters(paragraphInfo: TextInfo, offset: int):
    paragraphStartInfo = paragraphInfo.copy()
    paragraphStartInfo.collapse()
    info = paragraphStartInfo.copy()
    # first attempt!
    info.move(UNIT_CHARACTER, offset)
    # now compute textInfo span spanning from paragraphStart to info.
    s = span.text
    # now check if len(s) is greater or less than offset
    # and try to move info a few characters left or right, depending on whetehr we overshot or undershot.

I know this appears to be ugly and hacky, but so far I couldn't come up with any better solution. Does anyone have a better idea?

mltony commented 6 months ago

Also cc: @LeonarddeR - since you authored wide string converter - do you have any ideas regarding my question in previous comment?

CyrilleB79 commented 6 months ago

For example in Microsoft Word (I am not targeting word in in my PR, but just want to illustrate quirks of UIA textInfos) - if we have a bulleted list and the cursor is at the beginning of the first item, then textInfos.move(UNIT_CHARACTER, 1) would actually move over 3 characters in textual representation: the bullet character, whitespace and actual first character of list item.

In Word, the caret cannot be placed before the bullet. In a line containing a bullet, the first position is after the bullet and the indentation, before the first text character. Thus the cursor actually moves over one character as expected.

mltony commented 6 months ago

I know. But the problem is the first character in bullet list item stands for 3 characters. E.g., Put your cursor in the beginning of said list item and do:

>>> t=focus.makeTextInfo('caret')
>>>t.move('character', 1, endPoint='end')
1
>>> t.text
'• F'

So moving by 1 in terms of textInfo characters corresponds to moving by 3 in term of Pythonic string indices. My question is this: is there a way to somehow obtain this mapping for UIA text infos? My guess is not, that's why I am planning to do some version of binary search, but would be happy to learn if there is a better way.