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

Say all does not turn the page properly in bookworm #13927

Closed
Mohamed00 opened this issue Jul 20, 2022 · 11 comments · Fixed by #14070
Closed

Say all does not turn the page properly in bookworm #13927

Mohamed00 opened this issue Jul 20, 2022 · 11 comments · Fixed by #14070
Assignees
Labels
feature/say-all p3 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority triaged Has been triaged, issue is waiting for implementation.
Milestone

Comments

@Mohamed00
Copy link

Steps to reproduce:

  1. Download Bookworm, a book reader. A build can be found here. https://ci.appveyor.com/api/buildjobs/kffqbo4qru8pok39/artifacts/scripts%2FBookworm-2022.1b1-x64-portable.zip. Go into general preferences by pressing control+shift+p, and make sure the Try to support the screen reader's continuous reading mode by automatically turning pages (may not work in some cases) option is enabled.
  2. Open a PDF file and read it using say all. Example: https://www.dropbox.com/s/rlq00oplvkfzfl2/DoubleTalk%20RC8660%20Chipset%20Manual.pdf?dl=1.

Actual behavior:

Say all doesn't turn the page after the end of it is reached. You will notice that it will stop reading around the end of page 1.

Expected behavior:

Say all should be able to continuously turn the page.

NVDA logs, crash dumps and other attachments:

System configuration

NVDA installed/portable/running from source:

Installed

NVDA version:

alpha-25928,3894e25f

Windows version:

Windows 11, Version 22H2 (OS Build 25158.1000).

Name and version of other software in use when reproducing the issue:

Bookworm 2022.1b1.

Other information about your system:

N/A

Other questions

Does the issue still occur after restarting your computer?

Yes.

Have you tried any other versions of NVDA? If so, please report their behaviors.

Yes, NVDA snapshot alpha-25824,9172c09a and below work normally.

If NVDA add-ons are disabled, is your problem still occurring?

Yes.

Does the issue still occur after you run the COM Registration Fixing Tool in NVDA's tools menu?

Yes.

@lukaszgo1
Copy link
Contributor

Possibly caused by #13670 cc @mltony

@seanbudd seanbudd added bug/regression feature/say-all release/blocking this issue blocks the milestone release labels Jul 21, 2022
@seanbudd seanbudd added this to the 2022.3 milestone Jul 21, 2022
@seanbudd
Copy link
Member

I can reproduce this.
It appears that the next page is reached successfully, however the cursor remains at the end of the page, so all speech on the page is skipped.

@seanbudd seanbudd self-assigned this Jul 21, 2022
@seanbudd seanbudd added p3 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority triaged Has been triaged, issue is waiting for implementation. labels Jul 21, 2022
@mltony
Copy link
Contributor

mltony commented Jul 21, 2022

I will take a look this weekend.

@seanbudd
Copy link
Member

Likely related: #13420

The title says "some applications" but this is just bookworm right?

@seanbudd seanbudd changed the title Say all does not turn the page properly in some applications Say all does not turn the page properly in bookworm Jul 25, 2022
@Mohamed00
Copy link
Author

It seems so, but I think it's an NVDA issue. The debug log repeatedly mentions say all:line reached callbacks when it fails to turn the page. Example: IO - speech.speech.speak (04:59:21.177) - MainThread (19208):
Speaking [CallbackCommand(name=say-all:lineReached), CallbackCommand(name=say-all:lineReached), CallbackCommand(name=say-all:lineReached), CallbackCommand(name=say-all:lineReached), CallbackCommand(name=say-all:lineReached), CallbackCommand(name=say-all:lineReached), CallbackCommand(name=say-all:lineReached), CallbackCommand(name=say-all:lineReached), CallbackCommand(name=say-all:lineReached), CallbackCommand(name=say-all:lineReached)]
IO - speech.speech.speak (04:59:21.348) - MainThread (19208):
Speaking [CallbackCommand(name=say-all:lineReached), CallbackCommand(name=say-all:lineReached), CallbackCommand(name=say-all:lineReached), CallbackCommand(name=say-all:lineReached), CallbackCommand(name=say-all:lineReached), CallbackCommand(name=say-all:lineReached), CallbackCommand(name=say-all:lineReached), CallbackCommand(name=say-all:lineReached), CallbackCommand(name=say-all:lineReached), CallbackCommand(name=say-all:lineReached)]
DEBUG - speech.sayAll._TextReader.nextLine (04:59:21.583) - MainThread (19208):
no self.reader.

@mltony
Copy link
Contributor

mltony commented Jul 25, 2022

I was overwhelmed by urgent issues at work, so didn't have time to fix this yet. Will try to find time in the next couple of weeks.

@mltony
Copy link
Contributor

mltony commented Aug 1, 2022

I spent some time looking into this issue. I haven't figured out how to fix it yet, but here is what I found in case anyone is interested.
So far it appears that BookWorm detects certain pattern in API calls of how NVDA retrieves more text. It appears that BookWorm returns empty paragraph at the end of the page and if that empty paragraph was requested a few times, then it assumes that it is time to turn page. Its page turning seems to be separate from NVDA page turning, e.g. I see some logic in sayall that triggers page turns in some other apps, but that logic is not triggered with BookWorm.
Now after my refactoring the sequence of API calls appears to be slightly different and this confuses BookWorm. I can actually see that in the refactored version NVDA finds this empty paragraph at the end of the page during sayAll, and also it requests next paragraph a few hundred times, still receiving empty paragraphs each time. Somehow page turning logic in BookWorm doesn't trigger in this case.
My preliminary conclusion is that this is peculiarity of BookWorm, as it expects some API calls in just the right sequence to trigger a page turn. I will try to identify which API call sequence did I change in my refactoring and I will try to mimick the old behavior.

@cary-rowen
Copy link
Contributor

We might ask @mush42 to explain about Bookworm's page-turning mechanism.

@mush42
Copy link

mush42 commented Aug 2, 2022

Hi @cary-rowen @mltony

Here is how we do it:

  • We use the default wx.TextCtrl which, in turn, uses the richedit control
  • We add a custom WndProc for the richedit to be able to recieve WM_NOTIFY window messages
  • We explicitly tell the underlying richedit to notify us about selection events, including empty selection, which identifies a caret move event, by adding the ENM_SELCHANGE flag to the control's event mask
  • When our WndProc recieves a message whith a notification header code of EN_SELCHANGE we examine whether the selection type is SEL_EMPTY. If so, we add the message to a queue.
  • A custom backround thread has an infinite loop that processes items from the queue, and fires a custom wx.Event if the latest caret position is different from the previous position
  • If say all is enabled by the user, we process caret movement events, and if the caret is at the last (empty) line, and about 0.75 seconds has elapsed from the last page turn, we load the next page

Best
Musharraf

@seanbudd seanbudd reopened this Aug 10, 2022
@seanbudd
Copy link
Member

Blocking #13469, #13956

@seanbudd seanbudd removed bug/regression release/blocking this issue blocks the milestone release labels Aug 10, 2022
@lukaszgo1
Copy link
Contributor

@seanbudd Please change the milestone for this issue - 2022.3 -> 2022.4

@seanbudd seanbudd modified the milestones: 2022.3, 2022.4 Aug 15, 2022
seanbudd pushed a commit that referenced this issue Aug 30, 2022
Fixes #13469.
Fixes #13927.

Summary of the issue:
Previous PR #13956 broke sayAll functionality in BookWorm (#13927) and therefore was reverted.
This PR undoes reverting, in other words it reintroduces table sayAll commands. It also contains a minor change that fixes sayAll in BookWorm.

Description of user facing changes
Reintroduces table sayAll commands.

Description of development approach
In my original PR #13956 I did a minor refactoring of SayAll classes, in order to avoid code reduplication and have a nice class inheritance. In order to achieve this I slightly rearranged calls to textInfo. This didn't affect functionality anywhere except for page turn detector in BookWorm.
Reverting to the original order of TextInfo calls at the cost of slightly less elegant code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/say-all p3 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority triaged Has been triaged, issue is waiting for implementation.
Projects
None yet
6 participants