-
-
Notifications
You must be signed in to change notification settings - Fork 655
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
Crash in VBufStorage_buffer_t::deleteNode in Firefox Nightly with Gmail #8924
Comments
Related Mozilla bug 1505276. And on a second machine, I got a crash with a slightly different signature, but with the same steps. Always once I get past the third menu item from the top, I crash. I am using NVDA with braille, if that makes a difference. |
I can't reproduce this, neither with nor without braille. However, here are some interesting observations. This Gmail menu is pretty strange. All of the items are in the tree, but not inside the button. However, the focused item is always moved beneath the button using aria-owns and focused using aria-activedescendant. That is, there is only ever one menu item which is a child of the button. When you move the focus, the previously focused menu item returns to the full menu and the newly focused item gets owned and made the active descendant. Note that the accessibles aren't re-generated in this case. The menu items always have the same ids, but their parents change. We do fire reorder/text change events as usual when the tree mutates, though. The fact that a node with the same id is moving within the tree does make me suspicious that the buffer is somehow not handling that correctly. This distilled test case illustrates what Gmail is doing. I can't reproduce the crash with this either, but I'm curious if you can, Marco.
|
I can't reproduce this either, but:
I don't trust this code well enough to say that it would 100% cope with
moves like this.
Therefore, @jcsteh and I agree we should change the code to always
disallow reusing existing nodes from anywhere other than their current
position. I.e. now when we try to reuse a node, the node must have the
same parent as the possible new node, and the same siblings also.
Moves are rare, so we hope that this will not reduce performance much.
It certainly would not be any worse than 2018.3, and shouldn't affect
performance of the German wikipedia article.
|
I cannot reproduce the crash with @jcsteh's reduced test case on either machine where I see the crash in Gmail. Mick, let me know if you need me to test a try build with the code changes to confirm that this fixes the crash. :) |
Please test this try build:
https://ci.appveyor.com/api/buildjobs/i74fapb969yjy31p/artifacts/output%2Fnvda_snapshot_try-i8924-16239%2Cedb41aae.exe
Let me know if the crash is now gone, and that you are not noticing any
major decrease in performance.
Using @jcsteh's testcase at least, I can confirm that we now refuse to
reuse nodes if they are moving between different parents.
A quick test of the German wikipedia page, and gmail show no decrease in
performance.
|
I can confirm that the crash is fixed with this try build. I'd say "go". :) |
I can confirm that the crash is also fixed in the NVDA version alpha-16243,244ed4ec snapshot. Thank you! |
Steps to reproduce:
Actual behavior:
Reproducible crash that closed Firefox. Sometimes, Crash Reporter doesn't even come up. But once it did, and it gave me this crash report.
Expected behavior:
No crash.
System configuration:
NVDA Installed/portable/running from source:
Installed.
NVDA version:
NVDA version alpha-16220,87eb36f8
Windows version:
Windows 10 insider 18272.
Name and version of other software in use when reproducing the issue:
Firefox 65.0a1 Nightly from November 6.
Other information about your system:
Gmail is in New Design, I believe this is the default now, and you cannot eve turn that off any more. It's the standard Gmail, not G Suite.
Other questions:
Does the issue still occur after restarting your PC?
Yes. Reproducible on my system.
Have you tried any other versions of NVDA?
Yes, 2018.3.x does not crash. I suspect this is a result of either the VBuf speedup or the merging of DLLs, or both.
CC @michaelDCurran @jcsteh.
The text was updated successfully, but these errors were encountered: