nvda icon indicating copy to clipboard operation
nvda copied to clipboard

Fix Java Access Bridge compatibility on 32 bit systems

Open FelipeZanabria opened this issue 1 year ago • 45 comments

Link to issue number:

fixes #10842, fixes #16330

Summary of the issue:

When NVDA focuses a Java application on a 32-bit system, it gets stuck on the icon, without the ability to interact with it.

Description of user facing changes

When the user on a 32-bit system focuses on a Java application, the controls can be navigated normally, as was the case in version 2019.2.1 and earlier.

Description of development approach

  • Check the architecture of the running system
  • Based on this define the jobj64 class.
    • If the system is 64 bits use the type c_int64, otherwise use c_int.
    • Finally define which dll should be loaded. If the system is 64-bit, load WindowsAccessBridge-32.dll included in NVDA, otherwise load WindowsAccessBridge.dll, available in c:\Windows\System32.

Testing strategy:

Assuming the user has Java installed on a 32-bit system, simply search for Java in the start menu or control panel. When you find the configure Java element, press enter and move through it

Known issues with pull request:

None.

Code Review Checklist:

  • [x] 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
  • [x] UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • [x] API is compatible with existing add-ons.
  • [x] Security precautions taken.

FelipeZanabria avatar May 16 '24 17:05 FelipeZanabria

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • FAIL: Lint check. See test results for more information.
  • PASS: System tests (tags: installer NVDA).
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/4gyqqk370bur9hv6/artifacts/output/nvda_snapshot_pr16557-31953,3d1837b1.exe
  • CI timing (mins): INIT 0.0, INSTALL_START 1.2, INSTALL_END 1.0, BUILD_START 0.0, BUILD_END 31.7, TESTSETUP_START 0.0, TESTSETUP_END 0.4, TEST_START 0.0, TEST_END 2.4, FINISH_END 0.2

See test results for failed build of commit 3d1837b138

AppVeyorBot avatar May 16 '24 18:05 AppVeyorBot

  • FAIL: Lint check. See test results for more information.
  • CI timing (mins): INIT 0.0, INSTALL_START 1.0, INSTALL_END 0.9, BUILD_START 0.0, BUILD_END 29.6, TESTSETUP_START 0.0, TESTSETUP_END 0.4, TEST_START 0.0, TEST_END 2.3, FINISH_END 0.2
  • PASS: System tests (tags: installer NVDA).
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/17vhgbbd9osfsybb/artifacts/output/nvda_snapshot_pr16557-31955,833372aa.exe
  • PASS: Translation comments check.
  • PASS: Unit tests.

See test results for failed build of commit 833372aadc

AppVeyorBot avatar May 16 '24 23:05 AppVeyorBot

What environment precisely is this other DLL to be used in? To clarify specifically what I am thinking, is it for a 32-bit OS environment where both NVDA and the Java runtime will be 32-bit, or is it for a 32-bit (java runtime regardless of the OS environment (IE. it is possible to run a 32-bit JRE on a 64-bit OS). Does this correctly detect the actual thing of concern or does it miss a case IE. if the DLL is to be used whenever a 32-bit JRE application is running, then detecting the OS architecture misses the case of 32-bit JRE on 64-bit windows). Can NVDA detect whether the application is a 32-bit or 64-bit process?

mwhapples avatar May 17 '24 06:05 mwhapples

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • FAIL: Lint check. See test results for more information.
  • PASS: System tests (tags: installer NVDA).
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/spkk5koadevgnhwq/artifacts/output/nvda_snapshot_pr16557-31973,95b6ced4.exe
  • CI timing (mins): INIT 0.0, INSTALL_START 1.2, INSTALL_END 1.0, BUILD_START 0.0, BUILD_END 31.7, TESTSETUP_START 0.0, TESTSETUP_END 0.4, TEST_START 0.0, TEST_END 2.4, FINISH_END 0.2

See test results for failed build of commit 95b6ced447

AppVeyorBot avatar May 17 '24 07:05 AppVeyorBot

What environment precisely is this other DLL to be used in? To clarify specifically what I am thinking, is it for a 32-bit OS environment where both NVDA and the Java runtime will be 32-bit?

Yes.

FelipeZanabria avatar May 17 '24 17:05 FelipeZanabria

See https://github.com/nvaccess/javaAccessBridge32-bin/pull/2

FelipeZanabria avatar May 17 '24 21:05 FelipeZanabria

Hi, if you want to know system architecture, do NOT use “PROCESSOR_ARCHITECTURE” environment variable as it will , if yoㅕsystem architecture, 애 NOT use PROCESSOR_ARCHITECTURE environment variable as it returns whatever the architecture is set for the current process only. In addition to making this an API constant, a much better method is winVersion.getWinVer().processorArchitecture.endswith(“64”), and this call will be useful for a while as we move toward 64-bit NVDA (see #16304). Thanks.

josephsl avatar May 21 '24 14:05 josephsl

Thanks @FelipeZanabria, can you please pull in the latest changes from https://github.com/nvaccess/javaAccessBridge32-bin/pull/2 and also update https://github.com/nvaccess/nvda/blob/e7e8233fc8fe6a8a1f02002cbe9a142ca2f95ad2/projectDocs/dev/createDevEnvironment.md#L86

seanbudd avatar May 22 '24 01:05 seanbudd

Please also add a message to changes.md like this one https://github.com/nvaccess/nvda/blob/57ce236a8b28de5ce788d5582040238396c075d3/user_docs/en/changes.md#L205

seanbudd avatar May 22 '24 01:05 seanbudd

Regarding PR #2 from the other repository, I don't know what to do. The call to the last dll is already implemented in JABHandler.py. Regarding the constant in winApi.constants, Can I write the code that Josefsl provided as a variable outside of the 2 classes? Can I update everything in this PR?

FelipeZanabria avatar May 22 '24 17:05 FelipeZanabria

Regarding PR 2 from the other repository, I don't know what to do. The call to the last dll is already implemented in JABHandler.py.

You need to include it in this PR, you can do this by the following

cd include/javaAccessBridge32
git pull
cd ../..
git add include/javaAccessBridge32
git commit -m "update java access bridge dependency"

Regarding the constant in winApi.constants, Can I write the code that Josefsl provided as a variable outside of the 2 classes? Can I update everything in this PR?

Yep, just add the line to that file

seanbudd avatar May 23 '24 03:05 seanbudd

I don't have git.

FelipeZanabria avatar May 23 '24 05:05 FelipeZanabria

I'm not sure how to help here then. Please consider following the contributor process before opening pull requests.

seanbudd avatar May 23 '24 05:05 seanbudd

Hi @FelipeZanabria. The good news is that git is free, easy to install and there is lots of information available on how to use it. It's an essential part of developing many, many different software, including NVDA. Our documentation includes instructions on how to use git with NVDA. https://github.com/nvaccess/nvda/blob/master/projectDocs/dev/createDevEnvironment.md

gerald-hartig avatar May 23 '24 06:05 gerald-hartig

I didn't download Git because I also run w7, and because my hdd is failing, so it's very slow. I thought that the web interface would be enough for me. In fact I can do everything except update the required submodule. I can even continue with the constant in winApi and hopefully we close this. I don't know if it's possible for someone with Git to update the submodule i have modified in https://github.com/nvaccess/javaAccessBridge32-bin cC. I've realized that it can be a bit disorganized when creating 2 PRs, and at the same time uncomfortable for reviewers/developers.

FelipeZanabria avatar May 24 '24 01:05 FelipeZanabria

@FelipeZanabria - I can cover it this time, but can I ask how you tested this without a dev environment?

seanbudd avatar May 24 '24 01:05 seanbudd

When the PR build completes you can find the installer artifact from here: https://ci.appveyor.com/project/NVAccess/nvda/builds/49878888

You should be able to test with this build to confirm it works.

seanbudd avatar May 24 '24 01:05 seanbudd

https://ci.appveyor.com/api/buildjobs/xw9r4f83r8dacj89/artifacts/output/nvda_snapshot_pr16557-32127,a894d102.exe

seanbudd avatar May 24 '24 03:05 seanbudd

I downloaded the latest JABHandler.py from the source code, and checked that it was the same as version 2023.3.4. Since it was the same, I compared it with one from before 2019.3, and I realized that it was missing a reference to the last incorporated dll. So I edited the source file with what was missing and tried like this: I removed JABHandler.pyc from librari.zip in the NVDA root, put the source file in the root, because the compiled file is also in the root of the zip. Restarting NVDA loaded the source file with the changes. To delete the source file and keep the compiled one, I went into the __pycache folder that was created in the root of NVDA, removed the suffix added by python when generating the bytecode, and put it back into librari.zip. For the winAPI package, I downloaded it completely, created the folder referring to the package in the root of NVDA, removed it from the zip, modified constants.py, restarted NVDA and with the python console I imported the entire package to create the bytecode, to re-include in the .zip. If NVDA failed due to a coding error, I started a portable that I had on hand, looked at the old log and continued coding, even though the changes were minimal. For the lintTest, I have noticed the errors in the compilation of Appveyor.

FelipeZanabria avatar May 24 '24 04:05 FelipeZanabria

Hi,

I see. While I'm not from NV Access, I would like to strongly recommend doing the following before contributing future pull requests:

  1. UPGRADE TO (or BUY) Windows 10, preferably Version 22H2 (2022 Update/build 19045); or even better, if you are able, get a system with Windows 11 Version 23H2 (2023 Update) installed so you can be on the same page as most developers (NV Access folks included).
  2. INSTALL Git LOCALLY! As you have learned, there are tasks requiring local Git on your computer (one important task is updating submodules).
  3. If able, try setting up the local development environment as close to what NV Access folks and contributors are using (Python 3.11 32-bit, Visual Studio (or VS build tools) 2022, to name a few). You can use Git for Windows, or if you would like to do so, try using Windows Subsystem for Linux (WSL) with a distro such as Ubuntu with Git and other tools installed. You can also try using Visual studio Code for writing NVDA contributions.

The reason for recommending these (not merely suggesting them) is because you are using a setup that is not ideal (or even unusable/unacceptable) for software development - Windows 7 is no longer supported by Microsoft in any shape or form, and a failing (dying) hard disk means you need to be careful of what you are doing before the data (including contributions you are making) is gone; I would go further and get a new system as soon as possible or back up your data to somewhere safe if you intend to work on future NVDA contributions. Trust me when I advise you: the new system doesn't have to be a shiny computer with topmost specs - the system should meet your needs when doing various tasks, including coding, hopefully lasting several years. I thank you for taking our pull request review seriously and incorporating our feedback; at the same time, take our advice to heart when we recommend getting a system that can help you do many things for years to come.

Thanks.

josephsl avatar May 24 '24 04:05 josephsl

@FelipeZanabria - can you test this build out: https://ci.appveyor.com/api/buildjobs/lxn6iv0md5en4cd8/artifacts/output%2Fnvda_snapshot_pr16557-32144%2C803cdcf3.exe

seanbudd avatar May 27 '24 00:05 seanbudd

Some wider community testing would be also be welcome, for using Java Access Bridge Applications on various combinations of:

  • 32/64bit system architecture
  • 32/64bit software
  • Windows 8.1, 10, 11

seanbudd avatar May 27 '24 00:05 seanbudd

@FelipeZanabria - can you test this build out: https://ci.appveyor.com/api/buildjobs/lxn6iv0md5en4cd8/artifacts/output%2Fnvda_snapshot_pr16557-32144%2C803cdcf3.exe

With my W7 it is impossible to test this snapshot, but 3 days ago when you updated the submodule, I checked if the two Java dlls were in the installer. They weren't there, so I looked in sconscript, and after seeing that the first dll was there, I added the last one, waited for the compilation and it was included there. Also before publishing each commit I tested the changes in my setup.

FelipeZanabria avatar May 27 '24 02:05 FelipeZanabria

Hello Felipe,

Do you have access to a computer running Windows 8.1 or later, preferably Windows 10 22H2 (build 19045) or later? That way you can do a live test (running the modified NVDA executable) of the modifications from this pull request instead of doing compilation and library inclusion tests.

As I stated earlier, testing NVDA pull requests on Windows 7 is really questionable because NVDA 2024.1 and later (including the master branch code) does NOT support Windows 7 in any form or shape. This is an important thing to remember because the best way to see if your PR is working is doing live tests - running the modified NVDA with Java apps (with JAB applied) yourself. I think testing the inclusion of the library is a useful strategy on Windows 7, but by not considering live tests, the effectiveness and impact of the pull request is diminished (I'm sorry to say). So as I also stated, before writing another pull request, PLEASE get a system running updated Windows releases such as 10 and 11 so you can do live tests, improving the effectiveness and impact of pull requests.

Thanks.

josephsl avatar May 27 '24 02:05 josephsl

@josephsl I don't have it, but I'm going to write to the mailing list in Spanish inviting them to try the changes.

FelipeZanabria avatar May 27 '24 03:05 FelipeZanabria

Hi Felipe,

I see. Thanks for the pull request, and please do use Windows 10 or 11 yourself while writing the next pull request.

Thakns.

josephsl avatar May 27 '24 03:05 josephsl

I honestly cannot believe we are talking about making changes for Windows 7, which is out of support and has been for some time now, and when NVDA itself no longer supports anything but Windows 10 and 11, with Windows 10 having a sunset date just a hair over a year from now.

It is long past time to stop even considering Windows 7, and to a lesser extent 32-bit, and move on. You don't keep software vital and up to date by tortured attempts at backward compatibility, and particularly with out of support versions of Windows.

britechguy avatar May 27 '24 14:05 britechguy

@mwhapples have you any chance to test this PR on some 32bit java applications on 32 / 64 bit systems?

At least on my Windows 11 on a 64 bit system it works well with the 64bit java configuration window which indicates that the java access bridge for 64 bit works aas expected, so this PR doesn't seem to have any effect on 64 bit. So if we can test that for 32bit works as well, then I guess it should be fine.

Adriani90 avatar May 27 '24 19:05 Adriani90

I also tested with JRE 8.411 32bit, it still works fine on my 64 bit system.

So I think we need here testing on pure 32 bit systems I guess.

@FelipeZanabria do you have a test application we can test with? Or can you give some recommendations?

Adriani90 avatar May 27 '24 19:05 Adriani90

I also tested with JRE 8.411 32bit, it still works fine on my 64 bit system.

So I think we need here testing on pure 32 bit systems I guess.

@FelipeZanabria do you have a test application we can test with? Or can you give some recommendations?

@Adriani90 Thanks for trying and commenting. The Java control panel itself is enough, although I also tried the Linux sampler for Windows 32-bit. https://download.linuxsampler.org/packages/win32/snapshots/linuxsampler_20240326_setup.exe The installer covers both architectures, so you must have a 32-bit system for x32 to be installed. In the components to be installed, you must check Jsampler, which is the Java frontend application.

FelipeZanabria avatar May 27 '24 21:05 FelipeZanabria