Skip to content

Restore new live region announcement implementation for mshtml#11875

Merged
feerrenrut merged 4 commits into
masterfrom
revert-11838-revert-liveRegion
Nov 30, 2020
Merged

Restore new live region announcement implementation for mshtml#11875
feerrenrut merged 4 commits into
masterfrom
revert-11838-revert-liveRegion

Conversation

@LeonarddeR

Copy link
Copy Markdown
Collaborator

Link to issue number:

Reverts #11838
Fixes #11834

Summary of the issue:

#11838 reverted #9079 that causes a crash in mshtml documents, reproducible when pressing NVDA+F twice to open formatting info in a browseable message. I was able to pinpoint it to a needless setting of live politeness to nullptr instead of an empty string, which is the default when politeness is not set, based on the behavior of the default constructor of wstring.

Description of how this pull request fixes the issue:

Fixed the issue. Also optimized some code while at it.

Testing performed:

Opening a browseable message by pressing NVDA+F twice in a document.

Known issues with pull request:

None known

Change log entry:

In pr
Secti

break;
}
void MshtmlVBufStorage_controlFieldNode_t::reportLiveText(wstring& text, wstring& politeness) {
if(!all_of(text.cbegin(), text.cend(), iswspace)) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

note, all_of returns true for an empty string, which is exactly what we want.

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

Thanks @LeonarddeR

this->ariaLivePoliteness = this->ariaLiveNode->ariaLivePoliteness;
} else {
this->ariaLivePoliteness = nullptr;
this->ariaLivePoliteness = L"";

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.

Annoyingly this would have been captured by the visual studio '/analyze' option.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Is there a reason why we can't use this?

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 it was because there are too many other problems. We would either need to fix all the existing issues (which are likely false positives), or create a system to filter the noise and only show new issues.

@feerrenrut feerrenrut merged commit 874230a into master Nov 30, 2020
@feerrenrut feerrenrut deleted the revert-11838-revert-liveRegion branch November 30, 2020 09:19
@nvaccessAuto nvaccessAuto added this to the 2020.4 milestone Nov 30, 2020
seanbudd added a commit that referenced this pull request Jun 18, 2021
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

3 participants