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

Java Access Bridge announcement fixes: toggle buttons, window titles, position information #13744

Merged
merged 11 commits into from
Jun 2, 2022

Conversation

mwhapples
Copy link
Collaborator

@mwhapples mwhapples commented May 27, 2022

Link to issue number:

Fixes #9184 and fixes #9728

Summary of the issue:

A number of features in Java Access Bridge were not working optimally.

  1. Various commands like read window and read window title were not working when Java applications have multiple windows. (nvda does not read the window name correctly in a java application #9184)
  2. Toggle buttons were not read correctly. (Java SwingSet2 application with NVDA #9728)
  3. Unnecessary spaces when HTML tags removed from Java controls. (Java SwingSet2 application with NVDA #9728)
  4. Provide position information for Java tab controls. (Java SwingSet2 application with NVDA #9728)

Description of how this pull request fixes the issue:

  1. According to Java documentation when getAccessibleIndexInParent returns -1 there is no parent. So in NVDA if indexInParent is None we should treat it as if parent is not set.
  2. Java toggle buttons use the checked state for pressed, correctly map this. The value should be ignored for a toggle button.
  3. Updated the regular expression processing for HTML tags to check if the tags are at the start or end or have surrounding whitespace, in which case the tag is stripped. If there is only non-whitespace characters around the tag then replace the tag with a space to prevent words being joined.
  4. For Java controls with Role.TAB use indexInParent and parent.childCount to find the position information.

Testing strategy:

Manual following steps in issue #9728 and unit tests for HTML processing.

Known issues with pull request:

None known.

Change log entries:

New features

Changes

  • NVDA now announces position information for tab controls in Java applications.

Bug fixes

  • Correctly report state of Java toggle buttons.
  • Correctly identify the window in a Java application with multiple windows.

For Developers

Code Review Checklist:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests Not sure how to do such tests for java applications.
    • Manual testing
  • API is compatible with existing add-ons.
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English

@AppVeyorBot
Copy link

See test results for failed build of commit f452fb1228

@cary-rowen
Copy link
Contributor

@mwhapples
Thanks for your efforts, Could you take a look at #13039
, is this also relevant for Java applications.
Thanks

@mwhapples
Copy link
Collaborator Author

@mwhapples Thanks for your efforts, Could you take a look at #13039 , is this also relevant for Java applications. Thanks

IntelliJ, Android Studio, PyCharm and other JetBrains IDEs use Java Access Bridge. I also use those tools daily so am interested in fixing issues with those. I will take a look at that and other related issues. However for the purposes of this pull request, I think #13039 is out of scope and should be dealt with in another pull request. I prefer to keep pull requests small and targeted so that functionality/fixes do not get blocked by other fixes.

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.

generally LGTM, some minor changes needed

source/NVDAObjects/JAB/__init__.py Outdated Show resolved Hide resolved
source/NVDAObjects/JAB/__init__.py Outdated Show resolved Hide resolved
source/NVDAObjects/JAB/__init__.py Outdated Show resolved Hide resolved
source/NVDAObjects/JAB/__init__.py Outdated Show resolved Hide resolved
@seanbudd
Copy link
Member

seanbudd commented Jun 1, 2022

This PR description is quite hard to follow.
Would it be possible to split this PR to address fixing each issue individually?

@mwhapples
Copy link
Collaborator Author

This PR description is quite hard to follow. Would it be possible to split this PR to address fixing each issue individually?

I could improve the description if that would help. I tend to rely upon the referenced issue providing the detail, but I could supply more detail in this case to help clarify what were the bugs.
I created this PR to mirror the issue #9728 and I noticed it overlapped with #9184. I prefer to keep 1 pull request for 1 issue for ease of tracking when the issue can be closed. The exception being where the work will take some time and in stages, but even in that case it then raises the question whether the original issue ticket should be split into multiple issues as well.
To split this into multiple pull requests now I feel would not be an effective use of time due to the amount of time it may take to reorganise the commits.

mwhapples and others added 4 commits June 1, 2022 13:59
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
@AppVeyorBot
Copy link

See test results for failed build of commit d7c3f943d4

@seanbudd seanbudd changed the title Fix a number of (java Access Bridge issues Java Access Bridge announcement fixes: toggle buttons, window titles, position information Jun 2, 2022
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 @mwhapples - I can tidy up the PR description

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Java SwingSet2 application with NVDA nvda does not read the window name correctly in a java application
5 participants