Skip to content

speech.speakTextInfo: only announce exiting old controlFields if the reason is not reason_focus.#7434

Merged
michaelDCurran merged 2 commits into
masterfrom
i2591
Aug 29, 2017
Merged

speech.speakTextInfo: only announce exiting old controlFields if the reason is not reason_focus.#7434
michaelDCurran merged 2 commits into
masterfrom
i2591

Conversation

@michaelDCurran

Copy link
Copy Markdown
Member

Link to issue number:

Fixes #2591

Description of how this pull request fixes the issue:

This ensures that quicknav / tabbing etc does not announce many exit messages when jumping to an arbitrary part of a web page.

Testing performed:

Wikipedia article:

  • Arrow through table of contents: list and out of list is appropriately still announced
  • When in deep list item of table of contents, quicknav to next heading: no out of list is announced.

Known issues with pull request:

  1. Although exiting old fields is now suppressed for quicknav / focus, entering is not. So for example, we sould still announce a new list, or new table etc, yet we would not announce exiting it next time. I think this is what people want, but it is definitely not symmetrical. If people did want entering suppressed also, we'd need a way of knowing the controlField level the focus / quicknav actually hit, and only speak from there down. But this is impossible at the moment I think.
  2. At the moment this pr checks for lack of reason_focus when deciding if exiting controlFields should be spoken. The original issue only talked about quicknav. Either we need to move the quicknav reason int controlTypes to make it acceptable to use in speech, or perhaps there is an advantage to doing this for all focus (I.e. tabbing etc as well as this is similar to quicknav). It is really reason_caret we don't want to do this for.

Change log entry:

Section: Changes

…reason is not reason_focus. This ensures that quicknav / tabbing etc does not announce many exit messages when jumping to an arbitrary part of the page.
@michaelDCurran michaelDCurran requested a review from jcsteh July 24, 2017 03:28

@jcsteh jcsteh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. I think checking for != REASON_FOCUS is acceptable, as this makes tabbing more consistent with focus mode.

Comment thread source/speech.py Outdated
endingBlock=bool(int(controlFieldStackCache[count].get('isBlock',0)))
if endingBlock:
speechSequence.append(SpeakWithoutPausesBreakCommand())
# #2591: If the reason is not focus, Speak the exit of any controlFields not in the new stack.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be good to have a comment here explaining why we don't do this for focus in practical terms; e.g. We don't do this for focus because hearing "out of list", etc. isn't useful when tabbing or using quick navigation and makes navigation less efficient.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Don't announce end-of-container when quick navigation keys are used in browse mode

3 participants