Skip to content

Revert live region changes to mshtml virtual buffer#11838

Merged
feerrenrut merged 1 commit into
nvaccess:masterfrom
LeonarddeR:revert-liveRegion
Nov 20, 2020
Merged

Revert live region changes to mshtml virtual buffer#11838
feerrenrut merged 1 commit into
nvaccess:masterfrom
LeonarddeR:revert-liveRegion

Conversation

@LeonarddeR

Copy link
Copy Markdown
Collaborator

Link to issue number:

Fixes #11834

Summary of the issue:

Changes in #9079 caused crashes in the virtual buffer code when viewing a browsable message.

Description of how this pull request fixes the issue:

Revert the MSHTML changes from #9079. This means that we're still using the new live region code in Firefox and Chrome. While this introduces an inconsistency, I think there are several justifications.

  1. The mshtml vbuf is used less frequently than the Gecko vbuf
  2. I'm intending to investigate the issue, though I'm not at all skilled to operate a debugger with NVDA. But at least, we now have an easy method to trigger the issue.

Testing performed:

Tested that browseable messages no longer crash.

Known issues with pull request:

None

Change log entry:

In pr, as it modifies an existing entry.

@AAClause AAClause 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.

Thank for the fix. I tested the try build, issue solved for me! :)

This file is a part of the NVDA project.
URL: http://www.nvda-project.org/
Copyright 2006-2020 NV Access Limited, Google LLC, Leonard de Ruijter
Copyright 2006-2015 NVDA contributers.

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.

Suggested change
Copyright 2006-2015 NVDA contributers.
Copyright 2006-2015 NVDA contributors.

|| this->storageNode->ariaLiveIsAdditionsRelevant
)
) {
if(this->storageNode->ariaLiveNode&&this->storageNode->ariaLiveNode!=this->storageNode&&!this->storageNode->ariaLiveIsBusy&&(this->storageNode->ariaLiveIsTextRelevant||this->storageNode->ariaLiveIsAdditionsRelevant)) {

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.

I think that it's better to Split long if statements onto several lines. Do you have a reason to change this here and others places?

This file is a part of the NVDA project.
URL: http://www.nvda-project.org/
Copyright 2006-2020 NV Access Limited, Google LLC, Leonard de Ruijter
Copyright 2006-2015 NVDA contributers.

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.

Suggested change
Copyright 2006-2015 NVDA contributers.
Copyright 2006-2015 NVDA contributors.

} else {
this->ariaLiveAtomicNode=parent?parent->ariaLiveAtomicNode:NULL;
}
LOG_DEBUG(L"preProcessLiveRegion: ariaLiveNode "<<ariaLiveNode<<L", ariaLiveIsTextRelevant "<<ariaLiveIsTextRelevant<<L", ariaLiveIsAdditionsRelevant "<<ariaLiveIsAdditionsRelevant<<L", ariaLiveIsBusy "<<ariaLiveIsBusy<<L", ariaLiveAtomicNode "<<ariaLiveAtomicNode);

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.

I assume that this line should be restored and the (new) comment should be removed

@LeonarddeR

Copy link
Copy Markdown
Collaborator Author

Note that this pr simply reverts the changes done in #9079, it simply brings back the code to the state it had in NVDA 2020.3. I personally would like to leave this as is.

@feerrenrut

Copy link
Copy Markdown
Contributor

Ok, lets revert this, hopefully we can get to the bottom of the issue via #11834 in order to bring IE back into line.

@feerrenrut feerrenrut merged commit 53cecfd into nvaccess:master Nov 20, 2020
@nvaccessAuto nvaccessAuto added this to the 2020.4 milestone Nov 20, 2020
LeonarddeR added a commit that referenced this pull request Nov 27, 2020
seanbudd added a commit that referenced this pull request Jun 18, 2021
@LeonarddeR LeonarddeR deleted the revert-liveRegion branch August 23, 2025 06:28
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.

ui.browseableMessage() broken since alpha-21356,ae4ea8f9

4 participants