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

Allow users to disable browse mode by default #8846

Merged
merged 9 commits into from Mar 29, 2019

Conversation

@leonardder
Copy link
Collaborator

@leonardder leonardder commented Oct 16, 2018

Link to issue number:

Fixes #8716

Summary of the issue:

On every page load, browse mode is enabled by default, even when you explicitly disable it with NVDA+space. This is undesirable behavior for some users, particular magnifier users, who are able to navigate the web primarily using magnification.

Description of how this pull request fixes the issue:

  1. Added an "Enable browse mode on page load" option to browse mode settings. If checked (default), the behavior is as usual. If unchecked, browse mode is only activated on explicit request (by pressing NVDA+space). The state of browse mode or focus mode is not remembered across page loads. As suggested by @michaelDCurran, this might be addressed by another pr.

  2. An appModule author can also set a new property "disableBrowseModeByDefault" on the app module, in which case browse mode will also be off by default, regardless of the config setting. This is especially helpful for Electron apps such as Spotify and Skype, which are intended to be used as desktop apps. The property is implemented on a new Skype appmodule that replaces the legacy module which is no longer of use due to deprecation of SKype 7.

  3. Added an event_treeInterceptor_gainFocus to basic browse mode tree interceptors that will report pass through. This fixes a bug in Excel where pass through wasn't properly reported when moving from non-browse mode firefox to browse mode enabled Excel.

Testing performed:

Tested enabling and disabling the browse mode settings options, behavior as expected. When enabled, browse mode enabled itself on every page load, when disabled, browse mode was off on every page load.

Known issues with pull request:

Though I've tested this and it did not happen, in excel, there might be redundant focus mode pass through reports when moving from a browse mode enabled browser to an excel sheet. The previous behavior was way worse, though.

Change log entry:

  • New features

    • You can now disable browse mode by default by disabling the new "Enable browse mode on page load" option in NVDA's browse mode settings. (#8716)
      • Note that when this option is disabled, you can still enable browse mode manually by pressing NVDA+space.
  • Bug fixes

    • When browse mode is enabled in Microsoft Excel and you switch to a browser in focus mode or vise versa, browse mode state is now reported appropriately.
  • Changes for developers

    • You can now set the "disableBrowseModeByDefault" property on app modules to leave browse mode off by default.
@leonardder leonardder requested a review from michaelDCurran Oct 17, 2018
Copy link
Member

@michaelDCurran michaelDCurran left a comment

The user Guide needs to be updated with the new option.

for obj in itertools.chain((api.getFocusObject(),), reversed(api.getFocusAncestors())):
if obj.shouldCreateTreeInterceptor:
continue
Copy link
Member

@michaelDCurran michaelDCurran Mar 28, 2019

Choose a reason for hiding this comment

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

Can you explain to me why it is no longer necessary to skip objects which a treeInterceptor should always be created?

@leonardder
Copy link
Collaborator Author

@leonardder leonardder commented Mar 28, 2019

If browse mode on page load is disabled, tree interceptor situations such as in Firefox or other browsers do not create a tree interceptor by default. If people want to manually enable browse mode when browse mode on page load is disabled, they will have to press NVDA+space (i.e. this script) to have the interceptor created on demand. However in that case, when we skip objects that have shouldCreateTreeInterceptor set to True (which is the default for objects), this will result in no tree interceptor being created at all, and thus the script does not work. IN short, this script assumed that, if invoked, either there was always a tree interceptor on the focus object, or shouldCreateTreeInterceptor on the object ought to be False (such as in Word). The first assumption is no longer met when browse mode on page load is off.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants