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

Disable style navigation in Outlook and non-UIA word (duplicate) #16569

Merged
merged 4 commits into from
May 20, 2024

Conversation

mltony
Copy link
Contributor

@mltony mltony commented May 17, 2024

Note: this PR is a duplicate of #16543 that appears to have merged as status due to github glitch. Recreating as requested.

Link to issue number:

Closes #16459
Closes #16408
Closes #16458
Closes #16405

Summary of the issue:

We have discovered multiple problemds with non-UIA textInfo implementation in MS Word. Some examples are #16527, #16459, #16458. Also TextInfo implenetation in Outlook has proven to be too slow for style navigation. Therefore disabling both.

Description of user facing changes

"Not supported in this document" message is spoken.

Description of development approach

Raising an error when Outlook or non-UIA Word is detected.

Testing strategy:

Manual

Known issues with pull request:

N/A

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

@mltony mltony changed the base branch from master to beta May 17, 2024 18:09
@mltony mltony marked this pull request as ready for review May 17, 2024 18:09
@mltony mltony requested a review from a team as a code owner May 17, 2024 18:09
@mltony mltony requested review from seanbudd and removed request for a team May 17, 2024 18:09
@Adriani90
Copy link
Collaborator

@mltony can you please reference the issues which will become non issues after merging this? Otherwise we will have to close them manually, so any help in this regard from your side makes it easier for triaging. These are #16459, #16458, #16408 and #16405.
Also note that the style navigation feature is very unstable in some browse mode documents, for now it seems to work properly in browsers (Firefox, Chromium, Opera, Brave), Steam, Mozilla Thunderbird and Microsoft Teams.
I think it makes sense to disable it completely for Microsoft Office (Word / Outlook) and for Kindle.
See #16559, #16546, and #16527 (which happens both with UIA enabled and disabled).

@seanbudd I think it really makes sense to develop some propper tests for textInfo related code and for browse mode documents in general.
It seems Tony was not aware of the variety of browse mode documents out there when propposing this and other navigation related features which is totally fine because it is indeed a complex environment. But we should keep this in mind for the future. It cost really a lot of time to test everything mannually and create new issues for every browse mode use case after this is merged. At least it would have helped -alot if Tony tested the main browse mode documents before merging this into the core. I think the developers who propose such textInfo related features should have something at hand to guide them in how to strategically test a new feature. Maybe there is a new feature policy needed. This will make sure people can better provide a bullet proof solution for the problems they want to solve.

cc: @gerald-hartig, @michaelDCurran thoughts are very appreciated.

source/browseMode.py Outdated Show resolved Hide resolved
seanbudd
seanbudd previously approved these changes May 20, 2024
Copy link
Member

@seanbudd seanbudd left a comment

Choose a reason for hiding this comment

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

Thanks @mltony

source/browseMode.py Outdated Show resolved Hide resolved
source/browseMode.py Outdated Show resolved Hide resolved
@seanbudd
Copy link
Member

I've removed #16527 from the list of closed issues on this PR.

@Adriani90

I think it really makes sense to develop some propper tests for textInfo related code and for browse mode documents in general.

Could you please open a ticket? Feature request template or something custom is probably ideal. We perhaps could use a template for technical changes

source/browseMode.py Outdated Show resolved Hide resolved
source/browseMode.py Outdated Show resolved Hide resolved
source/browseMode.py Outdated Show resolved Hide resolved
@seanbudd seanbudd merged commit ed6ed8c into nvaccess:beta May 20, 2024
1 check was pending
@seanbudd seanbudd added this to the 2024.2 milestone May 20, 2024
seanbudd pushed a commit that referenced this pull request May 20, 2024
)

Closes #16459
Closes #16408
Closes #16458
Closes #16405

Summary of the issue:
We have discovered multiple problemds with non-UIA textInfo implementation in MS Word. Some examples are #16527, #16459, #16458. Also TextInfo implenetation in Outlook has proven to be too slow for style navigation. Therefore disabling both.

Description of user facing changes
"Not supported in this document" message is spoken.

Description of development approach
Raising an error when Outlook or non-UIA Word is detected.
@Adriani90
Copy link
Collaborator

@seanbudd I can create a request, though I am not sure I can explain in full the technical expectations. I'll give my best, feel free to adjust as needed afterwards.

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.

4 participants