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

Bundle the Java Access Bridge with NVDA. #10515

Merged
merged 1 commit into from Nov 20, 2019
Merged

Conversation

@jcsteh
Copy link
Contributor

jcsteh commented Nov 20, 2019

Link to issue number:

Fixes #7724.

Summary of the issue:

NVDA is a 32 bit application and requires the 32 bit Java Access Bridge client dll. This will happily talk to 64 bit Java applications. However, since Java 10, official 32 bit builds are no longer provided. This means that users have to find and install an unofficial community 32 bit build of Java themselves if they wish to access Java applications.

Description of how this pull request fixes the issue:

Bundle the JAB with NVDA:

  1. Include the dll via a Git submodule.
  2. Have SCons copy the dll into source/lib.
  3. Modify JABHandler to load it from the versioned lib directory if appropriate. This meant reorganising initialisation somewhat, as we need NVDAHelper to get the versioned lib path, but trying to use it at module level breaks due to circular import.
  4. Remove JABHandler's support for the legacy bridge, since it can no longer be used now that the proper 32 bit bridge is always present.

Testing performed:

Ensured no JAB was present in c:\windows\syswow64 and started Android Studio. Android Studio was accessible via JAB.

Known issues with pull request:

  1. Theoretically, there might be problems if a user is running a different version of the Java VM. In practice, the JAB isn't updated very often. Regardless, it isn't really possible to mitigate this. Also, the user would probably hit similar problems if they installed their own JAB anyway, since some apps now bundle their own Java VM and don't let you easily use the system Java VM.
  2. If the user has a 32 bit Java VM installed which is using the legacy JAB on the VM side, this might fail. (I'm not actually certain whether it will or not.) Now that 32 bit Java is no longer supported, this is highly unlikely. It was unlikely even before that, since this new JAB has been available for years now.
  3. The user still has to enable the Java Access Bridge in the Java VM. This has always been the case; it's nothing new. It's still annoying, though, especially if the user doesn't have a Java VM installed which adds jabswitch to the Ease of Access Center. It should actually be possible for us to enable this ourselves - jabswitch seems to flip a switch in the user's profile - but we'd need to figure out when to do this and I think this should be handled separately.

Change log entry:

Changes:
- The Java Access Bridge is now included with NVDA to enable access to Java applications. (#7724)

@jcsteh jcsteh requested a review from michaelDCurran Nov 20, 2019
@AppVeyorBot

This comment has been minimized.

Copy link

AppVeyorBot commented Nov 20, 2019

PR introduces Flake8 errors 😲

See test results for Failed build of commit b5fd9013ef

NVDA is a 32 bit application and requires the 32 bit Java Access Bridge client dll.
This will happily talk to 64 bit Java applications.
However, since Java 10, official 32 bit builds are no longer provided.
So that users don't have to find and install a community 32 bit build of Java themselves, bundle the JAB with NVDA.

1. Include the dll via a Git submodule.
2. Have SCons copy the dll into source/lib.
3. Modify JABHandler to load it from the versioned lib directory if appropriate. This meant reorganising initialisation somewhat, as we need NVDAHelper to get the versioned lib path, but trying to use it at module level breaks due to circular import.
4. Remove JABHandler's support for the legacy bridge, since it can no longer be used now that the proper 32 bit bridge is always present.
@jcsteh jcsteh force-pushed the jcsteh:bundleJab branch from 0077aec to 69e5e66 Nov 20, 2019
if legacyAccessBridge:
del cdll.windowsAccessBridge
else:
delattr(cdll,'windowsAccessBridge-32')

This comment has been minimized.

Copy link
@leonardder

leonardder Nov 20, 2019

Collaborator

Should this stay?

This comment has been minimized.

Copy link
@jcsteh

jcsteh Nov 20, 2019

Author Contributor

No. When you use cdll.LoadLibrary (which we now do so we can specify the full path), the ctypes loader cache isn't used:

__getattr__() has special behavior: It allows loading a shared library by accessing it as attribute of a library loader instance. The result is cached, so repeated attribute accesses return the same library each time.
LoadLibrary(name)
Load a shared library into the process and return it. This method always returns a new instance of the library.

https://docs.python.org/3/library/ctypes.html#ctypes.LibraryLoader

Copy link
Contributor

feerrenrut left a comment

This looks good, my recommendation is to merge for broader testing on alpha. My biggest concern is the source of the DLL, I'd prefer that we built it to remove the risk of unknown modifications, unless it isn't practical. I'd like to get an opinion from @michaelDCurran too.

@jcsteh

This comment has been minimized.

Copy link
Contributor Author

jcsteh commented Nov 20, 2019

@francipvb

This comment has been minimized.

Copy link
Contributor

francipvb commented Nov 20, 2019

Hello,

This attachment is the file changed by jabswitch in the user profile folder.

.accessibility.properties.txt

Hope this helps.

Cheers,

@michaelDCurran michaelDCurran merged commit 25de470 into nvaccess:master Nov 20, 2019
1 check passed
1 check passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@nvaccessAuto nvaccessAuto added this to the 2019.3 milestone Nov 20, 2019
michaelDCurran added a commit that referenced this pull request Nov 20, 2019
@jcsteh jcsteh deleted the jcsteh:bundleJab branch Nov 21, 2019
@akash07k

This comment has been minimized.

Copy link

akash07k commented Nov 22, 2019

@jcsteh

Thanks for this, so, will it work if we have jdk8 installed since the jab is from java version 13? will there be any conflicts? also, will we be able to expect more accessibility in java applications since the jab is new?
Also, now if we use open jdk 13 with android studio, will we be able to interact using NVDA? previously it wasn't possible

@jcsteh

This comment has been minimized.

Copy link
Contributor Author

jcsteh commented Nov 23, 2019

will it work if we have jdk8 installed since the jab is from java version 13? will there be any conflicts?

It seems to be fine - Android Studio uses JDK 8 and my (very brief) testing of that seems to work - but as I noted in the known issues section above, I can't be certain. There's also nothing we can do about that if it does turn out to b a problem.

also, will we be able to expect more accessibility in java applications since the jab is new?

No. The JAB hasn't had any significant updates in years and I doubt it ever will. On the flip side, that also means the chance of it breaking with different versions is minimal.

Also, now if we use open jdk 13 with android studio, will we be able to interact using NVDA?

Not sure what you mean exactly. Android Studio bundles its open OpenJDK 8. As noted above, that seems to work. If you mean using Android Studio with a different JDK, I don't know. I saw some documentation about that, but it seems to be pretty obscure to set up at best.

@akash07k

This comment has been minimized.

Copy link

akash07k commented Nov 23, 2019

will it work if we have jdk8 installed since the jab is from java version 13? will there be any conflicts?

It seems to be fine - Android Studio uses JDK 8 and my (very brief) testing of that seems to work - but as I noted in the known issues section above, I can't be certain. There's also nothing we can do about that if it does turn out to b a problem.
--Ya, I do agree with that. I wish we could have a proper accessible IDE for android development which shouldn't be based on java at all. But unfortunately I don't think that it will happen ever. Java accessibility can't be compared to other UI implementations such as winforms, WXPython etc. Since android studio is based on InteliJ, they won't change it sadly because it will change everything for everyone. :(

also, will we be able to expect more accessibility in java applications since the jab is new?

No. The JAB hasn't had any significant updates in years and I doubt it ever will. On the flip side, that also means the chance of it breaking with different versions is minimal.
--Oh, ok. then we are fine. One question though, now since NVDA will bundle the JAB library, will it work that we don't even install jdk on our machine since android studio already bundles that. Previously due to JAB dependency it was required to install JDK and JRE both so that we can JAB.

Also, now if we use open jdk 13 with android studio, will we be able to interact using NVDA?

Not sure what you mean exactly. Android Studio bundles its open OpenJDK 8. As noted above, that seems to work. If you mean using Android Studio with a different JDK, I don't know. I saw some documentation about that, but it seems to be pretty obscure to set up at best.
--Hmm, I was refering to use the own custom JDK with android studio, but I think it won't be of any benefit since android studio and their all counterparts are completely based on JDK 8 and in my opinion we won't get any additional benefits even though we use JDK13 with it. I don't know that why are they still stucked with so much older JDK versions. :( Sadly.

@jcsteh

This comment has been minimized.

Copy link
Contributor Author

jcsteh commented Nov 24, 2019

@akash07k

This comment has been minimized.

Copy link

akash07k commented Nov 24, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.