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

NVDA very sluggish when navigating 1password for windows desktop #10508

Closed
jfayre opened this issue Nov 19, 2019 · 10 comments · Fixed by #11011
Closed

NVDA very sluggish when navigating 1password for windows desktop #10508

jfayre opened this issue Nov 19, 2019 · 10 comments · Fixed by #11011
Milestone

Comments

@jfayre
Copy link

jfayre commented Nov 19, 2019

Steps to reproduce:

  1. Download the latest windows desktop version of 1password from http://www.1password.com
  2. If you don't have a 1password account, sign up for a free trial.
  3. Add a few passwords to your vault.
  4. Tab to the list of passwords and use the arrow keys to navigate. NVDA will pause for around 1.5-2 seconds as you press the arrow keys to navigate.
    JAWS and Narrator don't present this issue.

Actual behavior:

NVDA very sluggish when navigating between items in password list. No errors show up in the log.

Expected behavior:

NVDA should be as responsive as in other list views.

System configuration

NVDA installed/portable/running from source:

NVDA installed

NVDA version:

. 2019.2.1.

Windows version:

insider build 19023.1

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

1password desktop version 7.3.712

Other information about your system:

Other questions

Does the issue still occur after restarting your PC?

yes

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

same behavior on alpha-19195,8306ec15
nvda.log

@jfayre
Copy link
Author

jfayre commented Nov 20, 2019

Just wanted to add a clarification. The sluggishness happens in the main 1password desktop app, not 1password Mini, which is the interface used to fill in passwords on websites, etc.

@stickbear2015
Copy link

This looks like something specific to 1password, as running 7.2.576 on both machines here, doesn't present this issue with latest stable NVDA.
I've been offered the upgrade multiple times, but now I'm glad I didn't upgrade.
@jfayre contact me privately, I still have the older installer.

@MarcoZehe
Copy link
Contributor

I am also seeing this. Not always, but often. It also happens that sometimes, NVDA takes about 10 to 20 seconds after entering the master password to speak the search field. Searching then also reacts very slowly.

@jage9
Copy link
Contributor

jage9 commented Mar 29, 2020

Can confirm. Using latest 1Password desktop 7.4.759 and Alpha 19834, this issue still occurs. Using narrator, there is no major lag between each list item. It does not occur for every press of down or up arrow, but it is quite frequent.

@jcsteh
Copy link
Contributor

jcsteh commented Apr 15, 2020

I've just started using 1Password and this is really, really horrible. I regularly see lag of more than a second when moving between items in the list.

I debugged this a bit. 1Password fires 50+ UIA property change (name and item status) events every time you move between items in the list. These all seem to be on the fields for viewing/editing the item. This is very poor behaviour on their part and they should really fix it. If I make NVDA stop listening to property change events, it's as responsive as Narrator.

Interestingly, with the property change events enabled, the focus event callback happens after the lag. So, this isn't due to stuff we're doing when we handle the event in the main thread. This is all happening in the MTA (where UIA events get handled), before the event is queued to NVDA's core.

Obviously, not listening to property change events altogether isn't an option. However, Narrator and JAWS don't seem to be impacted like NVDA is. Some thoughts on that:

  1. This is another reason to again consider selectively registering for property change events (e.g. on focus + ancestors and progress bar value changes), rather than unconditionally listening to all of them.
  2. If I Register for property change events but return at the top of IUIAutomationPropertyChangedEventHandler_HandlePropertyChangedEvent (so we receive the event but discard it), it's pretty responsive. That suggests we might be able to optimise some of the stuff we do in the event handler; e.g. constructing the object, choosing overlay classes, etc.

@jcsteh
Copy link
Contributor

jcsteh commented Apr 15, 2020

I have a really hacky patch which works around this, but there's no way to do this in an app module, which means we end up with 1Password specific code in UIAHandler. :(

@galich
Copy link

galich commented Apr 16, 2020

@jcsteh hi there, I'm 1Password for Windows dev lead. Sorry we didn't notice we are flooding with these events that much.

While we are going to find out if we can suppress most of them, I'd love to know what would be the reasonable limit?

@jcsteh
Copy link
Contributor

jcsteh commented Apr 16, 2020

Thanks so much for looking into this, @galich.

It's hard to talk in terms of limits for events. Where events serve an important purpose, some client might need them and it's reasonable to fire them.

The issue here is that the events don't really seem to serve any useful purpose. When you move to a new item, from the user's perspective, the fields for the new item are effectively completely separate fields. Therefore, it's not useful to notify clients that the name, item status, etc. of each field changed.

Technically, I can understand that if you recycle the controls for each item and just change their labels, this is difficult to fix; the framework just fires the events. What's puzzling, though, is that it seems like the controls aren't entirely recycled. If I grab a reference to a UIA element for a field and then select a different item, that UIA element remains associated with the previous item; i.e. it doesn't shift to cover the new item. Furthermore, while I haven't dug into the exact events in great detail, it seems there are multiple events fired for each field, where I'd only expect one per field if they were indeed being recycled.

If it helps track this down, #10508 (comment) suggests that this might have been a regression some time after 1Password version 7.2.576. I can't confirm that myself, though.

@nvaccessAuto nvaccessAuto added this to the 2020.1 milestone Apr 17, 2020
@feerrenrut feerrenrut modified the milestones: 2020.1, 2020.2 Apr 17, 2020
@Mike-AgileBits
Copy link

Hi,

I'm Mike, the lead QA engineer for 1Password for Windows. We have made some adjustments to 1Password to try to speed this up but we're not sure if it is better or worse, we tried testing it ourselves and NVDA seem to be more responsive but not entirely lag-free. We would like to ask if anyone here can help us test it with a private build. If yes, please email us directly at support+windows@1Password.com and mention "Attention: Mike and NVDA" in the subject or body.

Unfortunately, we were not able to prevent WPF from "flooding" entirely as we've detected some oddities with the item list. Plus we've tried to filter out duplicate events and it made it even worse and slower in general.

Thanks in advance.

@Mike-AgileBits
Copy link

Mike-AgileBits commented Apr 21, 2020

Hi guys,

Just an update, we've shipped a beta update to 1Password (7.4.767) that should improve on this a bit. Someone from here has written to us and they were able to confirm that it helped a bit. We also fixed a few other accessibility issues in the same beta update.

We're looking into another lag that doesn't seem to happen in Narrator, there's a slight delay when pressing Control + N to select a category (arrow down, to select Secure Note). We're not exactly sure why this is limited to NVDA, Narrator/JAWS seems fine. Let me know if I should file a separate issue or ask the customer to file that issue.

Here's a direct link to the beta update: https://c.1password.com/dist/1P/win6/1PasswordSetup-7.4.767-BETA.exe or if you want, enable the beta update in 1Password settings via 1Password menu > Settings > Updates, enable Include beta updates.

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

Successfully merging a pull request may close this issue.

9 participants