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

NVDA reads the whole cell content instead of just the last line if you arrow up/down to the last line of an MS Word table cell containing more than one line of text #3421

Closed
nvaccessAuto opened this Issue Aug 7, 2013 · 13 comments

Comments

Projects
None yet
2 participants
@nvaccessAuto

nvaccessAuto commented Aug 7, 2013

Reported by Palacee_hun on 2013-08-07 20:00
MS Word 2003 Pro Hungarian
Windows XP Home SP3 32-bit Hungarian
NVDA Master of 7 August 2013 (but it is the same with 2013.1 and maybe earlier, I haven't tested)
[title is pretty self-explanatory. I have example documents, but I can't attach them, because they are connected to my work and cannot be shared. The steps to reproduce should look something like:
[[br]([br]]
The)]

  1. Position yourself on a table cell that you know to contain at least two lines of text in MS Word 2003 (later versions may produce the same too)
  2. Read the cell line by line by arrowing down.
  3. When you hit the last line, you'll be surprised to hear the entire cell read out, not just the last line.
  4. If you have another cell underneath this cell, you can slip into its first line by pressing arrow down once more.
  5. If you press Arrow up now, you end up in the last line of the multiline cell and point 3 manifests again.
    [[br]]
    Interestingly if you move around in the multiline cell with arrow left/right or ctrl+arrow left/right, the bug doesn't manifest.
@nvaccessAuto

This comment has been minimized.

nvaccessAuto commented Aug 7, 2013

Comment 1 by Palacee_hun on 2013-08-07 20:15
Upon some further testing I've found out that this bug can be reproduced even with a version as early as NVDA 2011.1!!!

@nvaccessAuto

This comment has been minimized.

nvaccessAuto commented Aug 8, 2013

Comment 2 by driemer.riemer@... on 2013-08-08 02:46
This happens on word 2010 also.. Just thought you might want to know.

@nvaccessAuto

This comment has been minimized.

nvaccessAuto commented Aug 10, 2013

Comment 3 by Palacee_hun on 2013-08-10 21:14
With most elaborate debugging I think most probably I've found out the cause of this bug, which lies not in NVDA, but MS Word :-( It seems that MS Word "deceives" NVDA when the latter asks MS Word via in-process COM calls to expand the line at the caret, and tell its starting and ending offset. I've found that in the specific case in this ticket, MS Word executes the expandtoline "perfectly": res=0 and neither offset returned is -1, so everything seems ok, but the returned offsets are in fact faulty if the caret is in the last line of a multiline cell: MS Word reports the correct end offset, but the start offset points to the beginning of the table cell, and not to the start of the last line of it!!! That causes the buggy behaviour described. This is quite sad news, as it seems quite hard to detect that NVDA got "desinformed" and to work around the bug.

@nvaccessAuto

This comment has been minimized.

nvaccessAuto commented Aug 10, 2013

Comment 4 by Palacee_hun on 2013-08-10 22:29
I have a brand new finding brought to me by chance and experimentation: you can simply experience the faulty Word behaviour described in Comment:3 if you follow the following simple steps:
[Position yourself somewhere in the middle of the last line of a multiline table cell and do a shift+end (select from cursor to end of line). Surprise: you'll hear the entire table cell selected!!!
2. Verification step: ctrl+c (copy to clipboard) and NVDA+c (read out clipboard), yes, clipboard contains the whole cell content.
3. For the next test, do step 1 again (go to somewhere in the last line).
4. Now do shift+home (select from cursor to the start of line). Another surprise: this selection will be correct, Word finds the start of the last line all right!!!
5. Do verification like in step 2.
6. Finally redo the steps above, but this time position yourself somewhere in a line which is not the last. Now there'll be no quirks, selections will be as expected.
[[br]([br]]
1.)]
I am quite sure that all this can be reproduced without any screen reader running, but I haven't done it yet, because I don't have sighted assistance at hand now.
[[br]]
This gives hope to fix the bug indeed, as NVDA utilises selection trickery through COM automation to expand the line in its inprocess C++ code for Word (in fact this gave me the idea to test the selection behaviour of Word in the specific case). As it was found, Word gives NVDA the right end offset in the specific case, only the start offset is faulty. But a selection with shift+home finds the correct start offset. Translating this to COM automation and integrating this into the inprocess C++ code may yield the fix.

@nvaccessAuto

This comment has been minimized.

nvaccessAuto commented Aug 20, 2013

Attachment winword.py added by Palacee_hun on 2013-08-20 16:12
Description:
Tiny Winword appmodule to work around the bug

@nvaccessAuto

This comment has been minimized.

nvaccessAuto commented Aug 20, 2013

Comment 5 by Palacee_hun on 2013-08-20 16:42
The attached file doesn't intend to be a proper fix, it is for those who find the bug frustrating and want a quick workaround. It alters the behaviour of caret_moveByLine script (e.g. down/up arrows) in Word so that it reads from the cursor up to the end of line. It achieves this by monkey-patching _expandToLineAtCaret method of WordDocumentTextInfo class so that the start offset (which is faulty in the specific case) will be the caret offset. This altered behaviour may also assist to avoid editing errors, because the user will know where the caret is when moving line by line. Frankly I've found the original behaviour (always reading the whole line regardless of where the caret is) quite error-prone when doing heavy editing. This altered behaviour may be better for braille as well, I don't know, I don't own a braille display.
You must put the attached file into the appmodules subfolder of %appdata%\NVDA folder for installed copies or of userconfig folder for portable copies. NVDA restart is not needed.

@nvaccessAuto

This comment has been minimized.

nvaccessAuto commented Jun 14, 2014

Comment 6 by Palacee_hun on 2014-06-14 17:06
Experimenting with Visual Basic macros in Word, I've found out exactly how the bug could be worked around in NVDA.
Winword.cpp should be modified in nvdahelper/remote. The procedure to modify is winword_expandToLine_helper. The code does its job until it requests to expand the range to line. That's where the MS Word bug lies. Instead of the simple expansion in one go, the following should be done (demonstrated by this VB macro snippet):
[let's move the selection to the start of line
' this call does nothing if we're already at the start, so that's good
' it's like pressing Home
Selection.StartOf Unit:=wdLine, Extend:=wdMove
Dim s As Long, e As Long
' fetch the start offset now, it's ok (see comment:4)
s = Selection.Start
' extend our selection to the end of line
' this call is like pressing shift+end, and does no harm if the line is empty
' start offset would be buggy after this in our case, but we've already got it
Selection.EndOf Unit:=wdLine, Extend:=wdExtend
' but end offset is good (see comment:4), so fetch it and we're done
e = Selection.End

[[br]([br]]

')]
I'd gladly submit a patch against winword.cpp that would implement this fix, but I haven't found the dispatch id constants of the needed methods on the net.
I found this code to be okay for "ordinary" lines too.

@nvaccessAuto

This comment has been minimized.

nvaccessAuto commented Jun 15, 2014

Comment 7 by Michael Curran <mick@... on 2014-06-15 01:50
In [daf2e57]:

Merge branch 't3421' into next. Incubates #3421

Changes:
Added labels: incubating

@nvaccessAuto

This comment has been minimized.

nvaccessAuto commented Jun 15, 2014

Comment 8 by Michael Curran <mick@... on 2014-06-15 02:44
In [effef5d]:

Merge branch 't3421' into next. Incubates #3421

@nvaccessAuto

This comment has been minimized.

nvaccessAuto commented Jun 15, 2014

Comment 9 by mdcurran (in reply to comment 6) on 2014-06-15 02:56
Replying to Palacee_hun:
We seemed to have been working on this one at exactly the same time. I came up with something very similar. Its certainly great to get confirmation from someone else though that this is a good way to go.
I have implemented something similar in branch t3421, and it is incubating in next. Feel free to test.
Instead of using wdExtend in the call to endOf, I used wdMove, as there is a similar bug in the first item of a table of contents where endOf(wdLine,wdExtend) selects the entire table of contents. In short, in certain situations, any kind of wdExtend can sometimes select all th way to the bound of a particular object (e.g. table cell, table of contents).
However, then I found two other bugs with startOf and endOf which I have had to work around:

  • endOf(wdLine,wdMove) when on a line that must be wrapped, sets IPAtEndOfLine (a read-only property) (which makes sense), but it does not get cleared when setting the selection to something else (e.g. restoring it to the original range). Therefore I setRange(0,0) first which seems to clear the flag.
  • startOf and endOf when called on a blank line directly after a page break ends up bounding the line containing the page break, not the blank line. Therefore if we end up with line offsets that do not at all bound the given offset, then we just use offset,offset+1 as the line, as clearly something weird happened.

I'm a little worried that there may be other bugs that are going to appear -- it always happens when we play with line stuff. But, it has fixed the table cell, and the table of contents bugs, which is very important.

@nvaccessAuto

This comment has been minimized.

nvaccessAuto commented Jun 16, 2014

Comment 10 by Michael Curran <mick@... on 2014-06-16 00:42
In [51e9747]:

Merge branch 't3421' into next. Incubates #3421

@nvaccessAuto

This comment has been minimized.

nvaccessAuto commented Jul 21, 2014

Comment 11 by Michael Curran <mick@... on 2014-07-21 23:09
In [2dbb6a1]:

Merge branch 't3421'. Fixes #3421

Changes:
Removed labels: incubating
State: closed

@nvaccessAuto

This comment has been minimized.

nvaccessAuto commented Jul 21, 2014

Comment 12 by mdcurran on 2014-07-21 23:09
Changes:
Milestone changed from None to 2014.3

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