VBufStorage: Fix crash by not allowing the reusing of nodes with differing parents.#8930
Conversation
jcsteh
left a comment
There was a problem hiding this comment.
However, it still allows nodes to change ordering within a given parent (if the existing node does not directly disallow this, such as in tables). If we do not keep this, the German wikipedia page becomes as slow as it used to be in NVDA 2018.3.
When you say change ordering, I assume you mean this includes adding or removing another node? If so, changing that would probably defeat the whole point of the speed-ups, no? I guess we can't tell the difference at this point between a node being added between two existing nodes (e.g. acb) and two nodes switching places (e.g. ba)? (Where a, b and c are chronologically ordered.)
| return nullptr; | ||
| } | ||
| // We only allow reusing nodes that remain at the same position in the tree. | ||
| // I.e. We never reuse a node that is from a different parent. |
There was a problem hiding this comment.
Even though you mention parent here, the term "position" is used, which might make someone think this also applies to movement within the same parent. You could either just shorten this to talk about the same parent or clarify by saying that movement within the same direct parent is still okay.
|
@MarcoZehe has confirmed this fixes the crashes he was experiencing. |
Link to issue number:
Closes #8924
Summary of the issue:
A web author can move a node from one place to another in the DOM. A web author can also move an accessible from one place to another within the accessibility tree, by dynamically changing aria-owns.
With the merging of pr #8678 this can in some cases cause NVDA to crash the web browser if NVDA's virtualBuffer for the document tries to reusethe moved node, as two separate updates may try to touch the same thing.
Examples of this are in #8924 including a real-life testcase in Gmail, and a specific code fragment.
Description of how this pull request fixes the issue:
NVDA's virtualBuffer code now refuses to reuse an existing node when re-rendering a parent if the existing node has a different parent to what is being re-rendered.
However, it still allows nodes to change ordering within a given parent (if the existing node does not directly disallow this, such as in tables). If we do not keep this, the German wikipedia page becomes as slow as it used to be in NVDA 2018.3.
Testing performed:
Tested the speed of the German wikipedia page. Tested the speed of moving up and down the inbox in Gmail.
Tested the code fragment in #8924.
However, waiting on @MarcoZehe to report if the try build in #8924 successfully fixes the crash for him.
I was never able to reproduce the crash.
Known issues with pull request:
None.
Change log entry:
None needed.