Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Announce the version of the application with focus #1625

Closed
nvaccessAuto opened this Issue Jun 29, 2011 · 33 comments

Comments

Projects
None yet
1 participant

Reported by PZajda on 2011-06-29 18:23
It could be good if there were a globalCommand to allow NVDA to announce the product name and version of the application which has the focus.
Attached diff file allows that on source/globalCommands.py with the gesture NVDA+Shift+V.

Attachment globalCommands.diff added by PZajda on 2011-06-29 18:25
Description:
diff file of source/globalCommands.py. Note that the module win32process is required.

Comment 1 by mdcurran on 2011-06-29 23:38
What is the primary use case for this? I can possibly see how it may be useful for developers, but as far as users go, even sighted users in most applications would have to go to the application's About dialog to find out this information.
If its only for developers, then it would be more preferable that this info be added to the developer info printed to the log with NVDA+f1. This is so that we do not waste more keyboard gestures than we need to.

Comment 3 by PZajda on 2011-07-01 14:22
Hello, you're right, putting it in developers info could be preferable than adding another shortcut.
it should be written at the end of developers info with insert+F1, to avoid it to be lost with all these informations.
I've tried to make a patch, but I didn't succeed in putting it at the end of informations because of inheritances, there are always some informations at the end. The only solution I found is to put it as the first information, but it is not not the most important information, so I think putting it at the beginning is not a good idea.

Comment 4 by briang1 on 2011-07-01 17:14
Why the end? Surely if its for info, the logical place for this is before the info about the windw etc within said program?

Attachment DevInfoWithVersionInfo.patch added by PZajda on 2011-07-01 20:06
Description:
Patch for the file source/NVDAObjects/init.py adding product name and version just after the name of the app module when pressing NVDA+F1.

Comment 5 by PZajda on 2011-07-01 20:10
Here is a patch for source/NVDAObjects/init.py which add version information of the application to developers informations of NVDA Objects.

Comment 6 by jteh on 2013-02-13 08:41
In order to be accepted, this needs to be rewritten using ctypes, as we're avoiding additional dependency on pywin32.

Comment 7 by PZajda on 2013-09-27 20:34
Hi,

OK... I am trying to do that using ctypes but I don't find anyway to get a real path, I have only paths like \Device\HarddiskVolume1\folder\file.exe and I have to add another dependency to convert this type of paths to a regular one.

I'm still looking for a way to do that but it seems to be a little hard :)

Comment 8 by PZajda on 2013-09-29 12:35
OK... Now, I don't need to convert any path by using ctypes.windll.kernel32.getModuleFileNameEx but I have no name any more :) it is... a little better, when I'll know what is my problem I'll send a mail to the nvda-devel list.

Comment 9 by PZajda on 2013-09-30 20:06
OK, now I understand add-ons haven't the same access to the handle as NVDA :)

I made some changes to appModuleHandler.py:

  • I added a function named getProductNameAndVersion to retrieve these informations.
  • When an appModule is initialized and its processHandle is greater than 0 (which happens when NVDA starts), add productName and productVersion attribute to this appModule.

Next I'll add these attributes to the objects themselves and to the developers info to display these when pressing NVDA+F1.

Comment 10 by jteh (in reply to comment 9) on 2013-09-30 21:38
Replying to PZajda:

OK, now I understand add-ons haven't the same access to the handle as NVDA :)

Add-ons have exactly the same access.

Next I'll add these attributes to the objects themselves and to the developers info to display these when pressing NVDA+F1.

There's no need to add these to the objects, since they apply to the application as a whole, so just make devInfo retrieve them from the app module.

Thanks!

Comment 11 by PZajda (in reply to comment 10) on 2013-10-01 09:11
Replying to jteh:

Replying to PZajda:

OK, now I understand add-ons haven't the same access to the handle as NVDA :)

Add-ons have exactly the same access.

OK... After a better look I understand my error.

Next I'll add these attributes to the objects themselves and to the developers info to display these when pressing NVDA+F1.

There's no need to add these to the objects, since they apply to the application as a whole, so just make devInfo retrieve them from the app module.

OK, do you think it is good to let it to appModuleHandler or it is better to put it specifically in NVDAObject where we get developers info and modify the handle in oleacc?

I though about appModuleHandler because product name and version are not specific to the object, but concern the application itself.
If it is better for you not to put it in appModuleHandler I can move it.

Edit: if it is not a problem in appModule, it is better for me as it seems there is something else I don't understand with the object's processHandle which still denies me the access.

Comment 12 by jteh (in reply to comment 11) on 2013-10-01 09:58
Replying to PZajda:

OK, do you think it is good to let it to appModuleHandler or it is better to put it specifically in NVDAObject where we get developers info and modify the handle in oleacc?

It does make more sense in appModuleHandler.AppModule, but that means you can't use the NVDAObject's processHandle. Hmm. The problem is that requesting more rights for AppModule.processHandle might break apps running as admin with UAC enabled.

Edit: if it is not a problem in appModule, it is better for me as it seems there is something else I don't understand with the object's processHandle which still denies me the access.

If you're running Windows Vista or later, I just checked and found out that the Windows GetProcessHandleFromHwnd function doesn't provide PROCESS_QUERY_INFORMATION. That's rather annoying and means you can't use it anyway.

For now, keep up the work in AppModule. We can always open a new process handle using the process ID (AppModule.processID) if necessary.

Comment 13 by PZajda on 2013-10-01 12:45
OK, to summarize what I have finally done:

  • I deleted my getProductNameAndVersion function, which is only useful in this situation so everything is in appModuleHandler.appModule.init;
  • As I said yesterday, appModule now have two new attributes named productName and productVersion;
  • When pressing NVDA+F1, show two new line:
    • appModule.productName
    • appModule.productVersion

I preferred to put the appModule prefix to avoid confusion.

It is available on Bitbucket.
Git repos: https://bitbucket.org/pzajda/nvda
Branch: t1625

Comment 14 by jteh on 2013-10-01 23:22
I'd prefer this isn't done in the constructor, as that means it'll be fetched even when the user might not be interested. It'd be better to do this lazily in properties and then overwrite the properties with the values. I haven't read the code yet, but I'm guessing it's more efficient to do both at once, so something like this should do the trick:

def _setProductInfo(self):
 # Code to fetch product name and version
 self.productName = name
 self.productVersion = version

def _get_productName(self):
 self._setProductInfo()
 return self.productName

def _get_productVersion(self):
 self._setProductInfo()
 return self.productVersion

If you don't understand this (it's fairly complex), one of us can make this change.

Thanks for your work.

Comment 15 by PZajda on 2013-10-02 01:06
OK, it's done as you said: productName and productVersion attributes are not defined in the constructor anymore, but only if needed by using _setProductInfo which is called by _get_productName and _get_productVersion.

A little complex, but very instructive. Thanks!

Comment 16 by PZajda on 2013-10-02 13:12
Hi,

I tested it on Windows 7 64 bits and I noticed something strange: if I press NVDA+F1 in Dropbox, I have appModule.productName/Version with the right values.
But in Explorer or Notepad, both attributes have no value.
I'll try within adding some log info to see where the problem occurs.

Comment 17 by PZajda on 2013-10-02 14:11
In fact the reason is simple, getModuleFileNameEx cannot get informations about a 64 bits application from a 32 bits one.
I wanted to avoid using getProcessImageFileName because it returns device path, no win32 format and I've never succeeded in using queryDosDevice.

In fact, it seems we should use different function for about each version of Windows.

An user on MSDN suggests to use WMI to be sure of the result, which is for me the same thing as if I would use a plane to visit my neighbor...

For more informations:

http://msdn.microsoft.com/en-us/library/windows/desktop/ms683217%28v=vs.85%29.aspx

Comment 18 by PZajda on 2013-10-07 10:54
About testing with UAC: I installed the last update of Adobe Reader and during the installation NVDA read the window when focused but after that no NVDA commands are usable.

About the program name I'm back on it today, I'll try to avoid using WMI. :)

Edit:
The first point was a mistake from me. Sorry.

Comment 19 by PZajda on 2013-10-16 17:30
Hi,

Now it is OK for Windows Vista and higher, I use QueryFullProcessImageName if the version is greater than 5, if I don't make a mistake it is Vista or higher.

I tested on Windows 7 64 bits, with both 32 and 64 bits apps and it seems to work now.

There is still a bad thing: it won't work on Windows Xp 64 bits, still this problem with paths format when using GetProcessImageFileName.

To summarize: it works on Windows XP 32 bits, Vista and 7 32/64 bits, it should work on Windows 8 but I cannot test by myself.
I didn't and cannot test with Vista but MSDN specifies it is OK.
They don't give any precision about Windows 8.

To remind:

Patrick

Comment 20 by jteh on 2013-10-16 22:19
Very nice work. Thanks!

Code review:

To make the code easier to read (less indentation, etc.), you can handle error conditions inside an if block but leave the rest of the code outside, since errors are the exceptional condition. For example:

        if not self.processHandle:
            raise Exception("processHandle is 0")
        # Choose the right function to use to get the executable file name
...

You accidentally create the l variable twice:

                    l = ctypes.c_uint()
                    l = ctypes.c_uint()

When fetching the codepage, you only want the first one, so it's more efficient to just grab the first 4 bytes. You don't need to use array.tolist(); you can just pass it to the tuple constructor. Finally, since you're always using it as a string, you need only do that once. So:

                        codepage = array.array('H', ctypes.string_at(r.value, 4))
                        codepage = "%04x%04x" % tuple(codepage)
                        # Extract product name and put it to self.productName
                        ctypes.windll.version.VerQueryValueW(res, (u'\\StringFileInfo\\%s\\'
                            + u"ProductName") % codepage, ctypes.byref(r), ctypes.byref(l))

Btw, Python syntax allows you to continue the same string on a subsequent line without concatenating two strings together, which is more efficient. So, you can do:

(u"foo"
 u"bar")

instead of:

(u"foo"
 + u"bar")

Even so, I'd probably rewrite the above call as:

                        ctypes.windll.version.VerQueryValueW(res,
                            u'\\StringFileInfo\\%s\\ProductName' % codepage,
                            ctypes.byref(r), ctypes.byref(l))

I'd probably use RuntimeError instead of Exception.

nit: "No version informations." should be "No version information"

Thanks again!

Comment 21 by PZajda on 2013-10-17 16:12
Thanks you.
I've followed your corrections and pushed them on Bitbucket.

Comment 24 by jteh on 2013-10-18 07:51
Thanks. I need to build signed with uiAccess and see how it copes with an app running as admin. I suspect that will cause us some trouble, but there may be nothing we can do about it.

Comment 25 by PZajda (in reply to comment 24) on 2013-10-24 12:55
Hi,

Replying to jteh:

I suspect that will cause us some trouble, but there may be nothing we can do about it.

After another read of MSDN pages about GetModuleFileNameEx and QueryFullProcessImageName, there may be something we can do about it: if I understood the problem correctly, the problem is the PROCESS_VM_READ access right, which is only useful for GetModuleFileName, which is only used for Windows earlier than Vista.

The solution can be to give PROCESS_VM_READ access right to processHandle according to the Windows version.

I made a new commit (rev 9248f4e) where I open a process handle according to the Windows version.

I quickly tested on Windows Xp 32-bits and Windows 7 64 bits and it seems to work.

Comment 26 by mdcurran on 2013-11-14 06:56
This code works fine with a signed copy and uiAccess with any admin app.
This looks pretty good to be merged as is. But I have one last question:
According to the msdn article for GetModulefileNameEx: http://msdn.microsoft.com/en-us/library/windows/desktop/ms683198%28v=vs.85%29.aspx
It says that it was moved to kernel32 in Windows 2008 r2. It was in psapi before that.
Therefore, I think that the check for GetModuleFileNameEx in kernel32 can be removed as that check only runs for xp.

Comment 27 by PZajda (in reply to comment 26) on 2013-11-14 09:52
Hi,
Replying to mdcurran:

This code works fine with a signed copy and uiAccess with any admin app.

This looks pretty good to be merged as is. But I have one last question:

According to the msdn article for GetModulefileNameEx: http://msdn.microsoft.com/en-us/library/windows/desktop/ms683198%28v=vs.85%29.aspx

It says that it was moved to kernel32 in Windows 2008 r2. It was in psapi before that.

Therefore, I think that the check for GetModuleFileNameEx in kernel32 can be removed as that check only runs for xp.

OK, I've just done and committed it.

Comment 28 by Michael Curran <mick@... on 2013-11-15 03:44
In [96f25a2]:

Merge branch 't1625' into next. Incubates #1625

Changes:
Added labels: incubating

Comment 29 by mdcurran on 2013-11-15 03:46
t1625 in NV Access's git repository is a pull from bitbucket on this ticket, but rebased on current master. Also now merged to next for testing of course.

Comment 30 by Michael Curran <mick@... on 2013-12-02 00:13
In [9c729a7]:

Merge branch 't1625'. Fixes #1625

Changes:
Removed labels: incubating
State: closed

Comment 31 by mdcurran on 2013-12-02 00:14
Changes:
Milestone changed from None to 2014.1

Comment 32 by Agent Golder on 2014-08-06 09:30
Is it possible any of you to creat a global plugin doing this or help me creating it myself please?
Or els, please tell me where to put the above attached .diff file.
So in this way the command would be optional to the users who want it.

Comment 34 by bdorer on 2014-08-06 20:38
There is allready an Addon for that:
http://jeff.vrnw.org/Add-Ons/sayProductNameAndVersion.nvda-addon

Comment 35 by Agent Golder on 2014-08-06 21:59
Thank you a lot!

@nvaccessAuto nvaccessAuto added this to the 2014.1 milestone Nov 10, 2015

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