-
-
Notifications
You must be signed in to change notification settings - Fork 614
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
Fix Java Access Bridge compatibility on 32 bit systems #16557
base: master
Are you sure you want to change the base?
Fix Java Access Bridge compatibility on 32 bit systems #16557
Conversation
See test results for failed build of commit 3d1837b138 |
See test results for failed build of commit 833372aadc |
source/JABHandler.py
Outdated
@@ -43,6 +44,9 @@ | |||
import config | |||
from utils.security import isRunningOnSecureDesktop | |||
|
|||
#: Verification of the architecture of the running system | |||
is_64Bit = maxsize > 2**32 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
given #16330, could you make this a system wide constant in winAPI.constants
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See also #16330 (comment), please use os.environ["PROCESSOR_ARCHITECTURE"].endswith("64")
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
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? |
…ystems in an NVDA lib folder. The way of checking the system's architecture has also been modified for better readability.
See test results for failed build of commit 95b6ced447 |
Yes. |
source/JABHandler.py
Outdated
@@ -43,6 +43,9 @@ | |||
import config | |||
from utils.security import isRunningOnSecureDesktop | |||
|
|||
#: Verification of the architecture of the running system | |||
is_64Bit = os.environ["PROCESSOR_ARCHITECTURE"].endswith("64") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As requested earlier, given #16330, could you make this a system wide constant in winAPI.constants?
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.
|
Thanks @FelipeZanabria, can you please pull in the latest changes from nvaccess/javaAccessBridge32-bin#2 and also update
|
Please also add a message to changes.md like this one Line 205 in 57ce236
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I'm going to apply some minor changes. When the final build completes, please do a final round of testing and then we can merge
Some wider community testing would be also be welcome, for using Java Access Bridge Applications on various combinations of:
|
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. |
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 I don't have it, but I'm going to write to the mailing list in Spanish inviting them to try the changes. |
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. |
This comment has been minimized.
This comment has been minimized.
@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. |
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. |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
@FelipeZanabria - can you link to this thread? |
Here is the thread: |
@seanbudd it seems that no one on the Spanish mailing list uses 32-bit Windows, except for one who also uses w7, which doesn't help moving forward. |
@FelipeZanabria - could you send a message to the mailing list? |
I didn't want to subscribe just for that, but I'm going to find it and do this. |
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
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: