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

MS Word browse mode: press "h" BrailleBuffer could not update heading message #4968

Closed
nvaccessAuto opened this issue Mar 9, 2015 · 13 comments

Comments

@nvaccessAuto
Copy link

Reported by surfer0627 on 2015-03-09 08:29
STR:

  1. Open a Word document and switch to browse mode.
  2. If I move away from the document while browse mode is enabled and move back again. Braille gets confused.
  3. Press "h" BrailleHandler could not update heading message.
    In this case, if there is no next / previous heading, braille will display "no next / previous heading".
  4. If I press down or up arrow, Braille updates message correctly.

log message:

IO - inputCore.InputManager.executeGesture (16:06:59):
Input: kb(desktop):h
IO - speech.speak (16:06:59):
Speaking 1', u'\u58f9\u3001\u6ce8\u610f\u4e8b\u9805\n', u'heading level 1'

IO - inputCore.InputManager.executeGesture (16:07:10):
Input: kb(desktop):upArrow
IO - braille.BrailleBuffer.update (16:07:10):
Braille regions text: Word \u6587\u4ef6 multi line edt ', u'\u516c\u5bd3\u5927\u5ec8\u898f\u7d04\u7bc4\u672c '
IO - braille.BrailleHandler.update (16:07:10):
Braille window dots: 13 12346 3 1256 5 145 345 5 15 23456 5 13 1246 3 236 3 12345 1236 5 135
IO - braille.BrailleHandler.update (16:07:10):
Braille window dots: 13 12346 3 1256 5 145 345 5 15 23456 5 13 1246 3 236 3 12345 1236 5 135
IO - speech.speak (16:07:10):
Speaking [- inputCore.InputManager.executeGesture (16:07:16):
Input: kb(desktop):shift+h
IO - speech.speak (16:07:16):
Speaking [u'no previous heading'](u'\u516c\u5bd3\u5927\u5ec8\u898f\u7d04\u7bc4\u672c']

IO)
IO - braille.BrailleBuffer.update (16:07:16):
Braille regions text: previous heading'
IO - braille.BrailleHandler.update (16:07:16):
Braille window dots: 1345 135 - 1234 1235 15 1236 24 135 136 234 - 125 15 1 145 24 1345 1245

@nvaccessAuto
Copy link
Author

Comment 1 by dkager on 2015-05-30 16:34
In master-11139,6177785 I can reproduce this even without switching away from the document. Using Word 2013 with a document in Compatibility Mode.

@nvaccessAuto
Copy link
Author

Comment 2 by dkager on 2015-06-03 10:09
So the issue appears to be that browseMode.TextInfoQuickNavItem.report() speaks the TextInfo but doesn't braille it. This also implies it's not specific to MS Word. Of course the real thrill is that there is no braille.brailleTextInfo() function.

Can one of the wizards look at this for 2015.3? As a braille-only user I feel I'm not getting the full benefit of quicknav in Word right now. ;)

@nvaccessAuto
Copy link
Author

Comment 3 by jteh on 2015-06-03 11:22
TextInfoQuickNavItem.report is definitely not the right place to do this. TextInfoQuickNavItem.moveTo should probably call review.handleCaretMove and braille.handler.handleCaretMove, but we need to make sure that's not going to have any undesirable side effects.
Changes:
Milestone changed from None to 2015.3

@nvaccessAuto
Copy link
Author

Comment 4 by dkager on 2015-06-03 14:38
Maybe TextInfoQuickNavItem.report() could/should be refactored out completely? Having this method implies that there is a scenario where you want to move to an item without reporting it, which seems counter-intuitive. The only use for this is in BrowseModeTreeInterceptor._quickNavScript(), but that can probably be done with a parameter to moveTo().

The other place where report() is used, i.e. when coming out of the element list, appears to have the same defect with braille.

VirtualBuffers overrides moveTo(), so this change doesn't break them. For Office I'll add your suggested fix locally and see how it fares.

@nvaccessAuto
Copy link
Author

Comment 5 by jteh (in reply to comment 4) on 2015-06-03 23:02
Replying to dkager:

Maybe TextInfoQuickNavItem.report() could/should be refactored out completely? Having this method implies that there is a scenario where you want to move to an item without reporting it, which seems counter-intuitive. The only use for this is in BrowseModeTreeInterceptor._quickNavScript(), but that can probably be done with a parameter to moveTo().

Conversely, requiring a parameter suggests that you sometimes don't want to do something, so the functionality should be split out. Also, subclasses rarely need to override the way something is reported, but they may need to override the way you move to something as VirtualBuffer does.

The other place where report() is used, i.e. when coming out of the element list, appears to have the same defect with braille.

Braille is somewhat different to speech. Speech requires that specific messages be delivered at certain times. If you move by line, you speak it differently to moving by character. In contrast, when dealing with navigable text in braille, braille just needs to know when the caret moved, ideally as soon as it moves. It is displayed the same way regardless of how it moved. Normally, caret events should be enough to notify braille that the caret has moved, but unfortunately, Word doesn't fire caret events, so we have to get hacky.

@nvaccessAuto
Copy link
Author

Comment 6 by dkager (in reply to comment 5) on 2015-06-04 18:28
Replying to jteh:

Normally, caret events should be enough to notify braille that the caret has moved, but unfortunately, Word doesn't fire caret events, so we have to get hacky.

Are you saying report() should never push out braille because it'll just tack along anyway? This makes sense. But if you then update the caret as per comment:3, will this not cause speech to speak twice? This is what lead me to consider "reporting" as an integral part of "moving to".

@nvaccessAuto
Copy link
Author

Comment 7 by jteh (in reply to comment 6) on 2015-06-05 00:12
Replying to dkager:

Are you saying report() should never push out braille because it'll just tack along anyway?

Yes, except you sometimes need to force it to "tag along" if events don't get fired.

But if you then update the caret as per comment:3, will this not cause speech to speak twice?

No. review.handleCaretMove and braille.handler.handleCaretMove don't speak. Speech doesn't have enough information; it has to know what it should speak (line, word, character, etc.). This is why we rely on the commands to separately speak what they wish.

@nvaccessAuto
Copy link
Author

Comment 8 by dkager on 2015-06-06 20:26
Haven't had much luck with this solution so far. I also notice that braille isn't always following along when you Control+Alt+Arrow through a table in MS Word, which I think is related.

@nvaccessAuto
Copy link
Author

Comment 9 by dkager on 2015-06-07 16:49
Got it working. braille.handler.handleCaretMove() has to be called from BrowseModeTreeInterceptor, because of the logic in handleCaretMove(). Specifically, this call should probably be in _quickNavScript().

The obvious problem with this approach is that it will cascade down to all places that support browse mode unless they override this behavior. The solution is to extend WordDocumentTreeInterceptor with its own _quickNavScript() that calls super and then pushes the braille handler. If we do that the only "wasted" call to handleCaretMove() is when there is no previous/next QuickNavItem, i.e. when the superclass function simply returns without doing any caret updates. IMO this is trivial.

The remaining question is why navigation in tables with Control+Alt+Arrows is sometimes not brailling correctly outside of browse mode.

@nvaccessAuto
Copy link
Author

Comment 10 by James Teh <jamie@... on 2015-06-29 05:50
In [b12f665]:

Merge branch 't4968' into next

Incubates #4968.

Changes:
Added labels: incubating

@nvaccessAuto
Copy link
Author

Comment 11 by jteh on 2015-06-29 05:51
I can't reproduce the problem with table navigation. Braille seems to be updated as expected for me when I use control+alt+arrows, both in and out of browse mode. Can you provide exact steps and/or a test case?

@nvaccessAuto
Copy link
Author

Comment 12 by dkager on 2015-06-29 18:40
I haven't been able to reliably reproduce the tables issue, but will give it another shot. The headings issue is fixed though, awesome!

@nvaccessAuto
Copy link
Author

Comment 13 by James Teh <jamie@... on 2015-07-14 06:01
In [15c8bd2]:

In browse mode in Microsoft Word, single letter navigation now updates the braille display and the review cursor as expected.

Fixes #4968.

Changes:
Removed labels: incubating
State: closed

@nvaccessAuto nvaccessAuto added this to the 2015.3 milestone Nov 10, 2015
jcsteh added a commit that referenced this issue Nov 23, 2015
…s the braille display and the review cursor as expected.

Fixes #4968.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant