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

Provide a tool to re-register particular COM interfaces that may become unregistered or broken by uninstallers, breaking platform accessibility #8075

Merged
merged 8 commits into from
Apr 20, 2018

Conversation

michaelDCurran
Copy link
Member

Link to issue number:

Closes #2807

Summary of the issue:

Installers for products such as Adobe Reader and MathPlayer, incorrectly register their own dlls/typelibs as the COM proxy for certain COM interfaces such as IAccessible, IDispatch, IEnumVARIANT or IServiceProvider. While installed, there is no user impact, however once the product is uninstalled, the original registration is not put back and the registry is now pointing to a missing COM proxy for these interfaces. This causes these interfaces to be not marshallable and therefore accessibility is completely broken in software such as Firefox, chrome, Internet Explorer, and even some core parts of Windows such as the Task bar.

Description of how this pull request fixes the issue:

Based on the research provided by @jcsteh in #2807, this PR adds a tool to NVDA that re-registers the affected COM interfaces with their correct Windows-provided COM proxy dlls.
The tool automatically runs at the end of NVDA installation, but is also available from the Tools menu.

Testing performed:

On windows 10:

  • Manually edited the registry to point IAccessible and IDispatch to non-existant COM proxies.
  • Confirmed that accessibility was broken: Could not use Internet Explorer documents at all.
  • Installed NVDA to successfully correct the COM interfaces.
  • Could again use Internet Explorer
    On Windows 7:
  • Installed Adobe Reader
  • Uninstalled Adobe Reader
  • Confirmed that accessibility was broken: Could not use Internet Explorer documents at all.
  • Installed NVDA to successfully correct the COM interfaces.
  • Could again use Internet Explorer

Known issues with pull request:

This should fix all the cases we are aware of where following the instructions in #2807 fixed problems for people. There could be other COM interfaces that get broken that we don't yet know about. But these could be added to the tool in future once we have evidence of a fix.

Change log entry:

Bug fixes:

@LeonarddeR
Copy link
Collaborator

I wonder whether it is preferred here to have a confirmation question before the tool runs. Also, could you please commit the reg files as textual files rather than binary?

@michaelDCurran
Copy link
Member Author

michaelDCurran commented Mar 12, 2018 via email

@jcsteh
Copy link
Contributor

jcsteh commented Mar 12, 2018

@leonardder commented on Mar 12, 2018, 2:58 PM GMT+10:

I wonder whether it is preferred here to have a confirmation question before the tool runs.

That's tricky. It will always run at install time... and we really don't want to ask the user at that point because it should just happen without the user needing to understand it. We're not doing anything that shouldn't actually already be done on a non-broken system. As to whether to do it when you manually run the tool, I suppose I can see why you'd want to do that... but on the other hand, if we're not asking at install time, why ask here? Note that you'll get asked for UAC elevation, so you can always say no at that point.

Also, could you please commit the reg files as textual files rather than binary?

As Mick noted, they're already text, but UTF-16, which git doesn't recognise as text. Unfortunately, this is what regedit does natively. Regedit does support another format which is 8 bit instead of 16 bit, but that is labelled "Win9x/NT4 Registration Files", which raises some unanswered questions about what you lose when you use that.

@@ -0,0 +1,68 @@
import os
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a copyright header.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also perhaps a docstring for this package.

if OSMajorMinor==(6,1): # Windows 7
registerServer(os.path.join(programFiles,'Internet Explorer','ieproxy.dll'))
if is64bit:
registerServer(os.path.join(programFilesX86,'Internet Explorer','ieproxy.dll'),wow64=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

If its 64 bit windows 7 then register the ieproxy.dll from both "program files/internet explorer/" and "program files (x86)/internet explorer"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct. There are COM proxies for both 64 bit and 32 bit on 64 bit machines. The first registration will be 32 bit on 32 bit machines and 64 bit on 64 bit machines. Then if the machine is 64 bit, we also need to register the 32 bit one, so that 32 bit apps (including NVDA) can work. Note that IServiceProvider is used in more places than just Internet Explorer, its just that MS chose to store it in ieproxy.dll on Windows 7.

@@ -114,7 +114,7 @@ def run(self):

def build_manifest(self, target, template):
mfest, rid = build_exe.py2exe.build_manifest(self, target, template)
if getattr(target, "script", None) == "nvda.pyw":
if getattr(target, "script", "").endswith(".pyw"):
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this change about?

OSMajorMinor=winVersion.winVersion[:2]
log.debug("Fixing COM registration for Windows %s.%s, %s"%(OSMajorMinor[0],OSMajorMinor[1],"64 bit" if is64bit else "32 bit"))
# Commands taken from NVDA issue #2807 comment https://github.com/nvaccess/nvda/issues/2807#issuecomment-320149243
# OLEACC (MSAA) proxies
Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume this implies registering oleacc.dll doesn't have the desired result?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct. See @jcsteh's comment in #2807 on this: #2807 (comment)

@Brian1Gaff
Copy link

Brian1Gaff commented Mar 16, 2018 via email

@Qchristensen
Copy link
Member

I noticed in this build that the NVDA icon has reverted to the old one, was that deliberate? My bigger concern was just in case more critical things have been rolled back since the icon change was done awhile ago.

@LeonarddeR
Copy link
Collaborator

@Qchristensen commented on 16 mrt. 2018 22:57 CET:

I noticed in this build that the NVDA icon has reverted to the old one, was that deliberate? My bigger concern was just in case more critical things have been rolled back since the icon change was done awhile ago.

See #8091.

It might be more likely that #7104 is the cause of this, since that pr also involves changes in misc deps.

@Qchristensen
Copy link
Member

Yes it looks like that might have been it since I realised after writing this that the change happened in 14945 which incubated #7104

This happened to still work on x64 as there is a 32 bit copy of regedit in c:\windows\syswow64, and as NVDA itself is 32 bit it always found it.
Fixes #8089
michaelDCurran added a commit that referenced this pull request Mar 28, 2018
michaelDCurran added a commit that referenced this pull request Mar 28, 2018
@LeonarddeR
Copy link
Collaborator

I'd like to suggest the following change:

  1. Get rid of oleaccProxyWow64.reg
  2. Use either sysroot\regedit.exe or syswow64\regedit.exe based on architecture, with the same registry patch, just like with regsvr32

There is also regedt32.exe, that resides in system32 or syswow64 respectively.

@michaelDCurran
Copy link
Member Author

michaelDCurran commented Mar 29, 2018 via email

@@ -114,7 +114,7 @@ def run(self):

Copy link
Collaborator

Choose a reason for hiding this comment

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

Update copyright headers while here?

@LeonarddeR
Copy link
Collaborator

@michaelDCurran commented on 30 mrt. 2018 00:16 CEST:

I'm happy to make this change purely to get rid of a reg file, but do
you believe there is any functional advantage for doing this?

Not really, though an advantage might be that we can ensure for these patches or possibly added patches in the future that they do exactly the same thing. It would need to be tested, though.

My understanding is that the 32 bit version of regedit will write
directly into the wow6432node subtree automatically, thus I think you
are correct that we don't need the second file.

Looks like we have the same understanding here.

Do you actually know what this patch is supposed to do on Windows 10 for ARM? Is there something like wow64 there?

* Update copyright in setup.py
* Use the same reg file for registering oleacc proxy for both 64 and 32 bit, but just use a different version of regedit.exe for each.
michaelDCurran added a commit that referenced this pull request Apr 5, 2018
@cary-rowen
Copy link
Contributor

This feature seems to be disabled at this time. Can you investigate this?
@michaelDCurran

@cary-rowen
Copy link
Contributor

The discussion about the problem is included on groups.io, using this PR can not solve the problem, but manually performing the steps in #2807 can successfully solve the problem
https://nvda.groups.io/g/nvda/topic/nvda_doesn_t_read_context/82006419?p=,,,20,0,0,0::recentpostdate%2Fsticky,,,20,2,0,82006419

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.

NVDA stops reading webpages in all browser - saying "unknown" or "pane"
9 participants