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

Announce the version of the application with focus #1625

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

Announce the version of the application with focus #1625

nvaccessAuto opened this issue Jun 29, 2011 · 33 comments

Comments

@nvaccessAuto
Copy link

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.

@nvaccessAuto
Copy link
Author

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.

@nvaccessAuto
Copy link
Author

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.

@nvaccessAuto
Copy link
Author

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.

@nvaccessAuto
Copy link
Author

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?

@nvaccessAuto
Copy link
Author

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.

@nvaccessAuto
Copy link
Author

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.

@nvaccessAuto
Copy link
Author

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.

@nvaccessAuto
Copy link
Author

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 :)

@nvaccessAuto
Copy link
Author

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.

@nvaccessAuto
Copy link
Author

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.

@nvaccessAuto
Copy link
Author

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!

@nvaccessAuto
Copy link
Author

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.

@nvaccessAuto
Copy link
Author

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.

@nvaccessAuto
Copy link
Author

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

@nvaccessAuto
Copy link
Author

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.

@nvaccessAuto
Copy link
Author

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!

@nvaccessAuto
Copy link
Author

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.

@nvaccessAuto
Copy link
Author

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

@nvaccessAuto
Copy link
Author

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.

@nvaccessAuto
Copy link
Author

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

@nvaccessAuto
Copy link
Author

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!

@nvaccessAuto
Copy link
Author

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

@nvaccessAuto
Copy link
Author

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.

@nvaccessAuto
Copy link
Author

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.

@nvaccessAuto
Copy link
Author

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.

@nvaccessAuto
Copy link
Author

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.

@nvaccessAuto
Copy link
Author

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

@nvaccessAuto
Copy link
Author

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.

@nvaccessAuto
Copy link
Author

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

@nvaccessAuto
Copy link
Author

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

@nvaccessAuto
Copy link
Author

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.

@nvaccessAuto
Copy link
Author

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

@nvaccessAuto
Copy link
Author

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

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

No branches or pull requests

1 participant