App modules: add additional app properties such as app path, immersive process, and app architecture by josephsl · Pull Request #8644 · nvaccess/nvda

josephsl · 2018-08-18T00:30:24Z

Link to issue number:

Fixes #7894

Summary of the issue:

Adds additional app properties such as path, immersive process, and architecture (WoW64).

Description of how this pull request fixes the issue:

Adds the following properties:

appPath: returns full executable path for the current application.
isWindowsStoreApp: looks for a package info structure to determine if an app is a Store application (see a later comment as to why this route is used).
appArchitecture: uses kernel32.dll's IsWoW64Process2 (Windows 10 Version 1511 and later) to return a string denoting which machien the app was written for, suitable for working with x86/x64 processes on ARM64.

Testing performed:

Tested with various apps, both Win32 desktop apps and universal apps.

Known issues with pull request:

None

Change log entry:

Changes for developers:
Additional properties were added to app modules, including path for the executable (appPath), is a Windows Store app (isWindowsStoreApp), and machine architecture for the app (appArchitecture). (#7894)

josephsl · 2018-08-18T00:31:53Z

Hi,

I'll ask for a review once appArchitecture property is implemented and tested.

Thanks.

LeonarddeR · 2018-08-20T08:10:36Z

+		These include Windows store apps on Windows 8 and 8.1, and Universal Windows Platform (UWP) apps on Windows 10.
+		@rtype: bool
+		"""
+		if winVersion.winVersion.major and winVersion.winVersion.minor == (6, 1):


This line is wrong. It should be:

if (winVersion.winVersion.major, winVersion.winVersion.minor) == (6, 1):

I prefer

if (winVersion.winVersion.major, winVersion.winVersion.minor) < (6, 2):

josephsl · 2018-08-20T12:39:19Z

Hi, the reason for checking for equality was because we support Windows 7 SP1 and higher. I’ll edit this to make it clearer. Thanks.

francipvb · 2019-06-14T03:00:55Z

What about Version fields such major, minor and revision?

josephsl · 2019-06-14T03:03:27Z

Hi, not quite possible due to so many versioning schemes in use (for example, major.minor, year.major, major.minor.build, major.minor.revision, etc). Thanks.

francipvb · 2019-06-14T15:05:34Z

Hello @josephsl,

Hi, not quite possible due to so many versioning schemes in use (for example, major.minor, year.major, major.minor.build, major.minor.revision, etc). Thanks.

Anyway, it can be nice to have such features, regardless the version scheme used by the application. Repeating code to parse the actual version of the running application for every app module that requires it is not a good way to do.

Another thing to consider is that the scheme major.minor is a standard under windows and it is relatively safe to add major and minor.

Cheers,

josephsl · 2019-06-14T15:16:48Z

Hi, technically, it is possible to obtain this information. But imagine a case where a user may wish to find out NVDA’s own version via app module methods. Although it is possible to use versionInfo module for this, at first glance, due to the way version string is constructed, major.minor isn’t obvious. Also, note that what is considered major.minor could be year.major i.e. for NVDA, appModule.major will be reported as 2019 instead of 2, for instance. Not that I’m against this idea – I want to see various cases discussed before proceeding with this (by the way, I have suspended work on a PR for this due to high priority work at the moment).

francipvb · 2019-06-14T15:27:42Z

Hi, technically, it is possible to obtain this information. But imagine a case where a user may wish to find out NVDA’s own version via app module methods. Although it is possible to use versionInfo module for this, at first glance, due to the way version string is constructed, major.minor isn’t obvious. Also, note that what is considered major.minor could be year.major i.e. for NVDA, appModule.major will be reported as 2019 instead of 2, for instance. Not that I’m against this idea – I want to see various cases discussed before proceeding with this (by the way, I have suspended work on a PR for this due to high priority work at the moment).

What about naming these "first", "mid" and "last" or something like? I think that these three parts are the most widely queried. Probably the best approach is to implement a new lightweight class to store these values, constructed from a plain string such productVersion ones, replacing it by this hypothetical structure. Sorry for the private responses, I was missing to respond to Github notification email.

francipvb · 2019-06-14T15:47:46Z

 	chooseNVDAObjectOverlayClasses._isBase = True

+	def _get_appPath(self):
+		"""Returns the full path for the executable e.g. 'C:\\Windows\\explorer.exe' for Explorer.


Sorry but I don't see the utility of this under an appModule.

josephsl · 2019-06-14T15:48:01Z

Hi,

Version class: this can be fetched easily for immersive apps as all that's required will be parsing the user mode app model structure.

Regarding first, mid, last: there are three issues with this:

Not exactly user friendly: although great for developers who knows what they stand for, for users, they may wish to write it as appModule.version_major instead of appModule.version_first as major/minor/build/revision terms are more prominent.
Does not describe schemes fully: what if the only version info expressions are first, first.last, first.mid, mid.last, etc.? At least version_first is guaranteed, but even then, there are system components that won't return a meaningful version info at all (see below).
No version info for processes with NULL handle: in order to fetch product info (or for that matter, anything with processes), a process handle (hProcess) must be obtained. There are situations where this fails, the most prominent example being desktop window object held by CSRSS (Client/Server Runtime Subsystem/Windows subsystem process). At least the PID can be obtained, but when trying to obtain version info, NVDA (or rather, Python) records a traceback, saying that process handle is 0 (note that process handles are relative to the calling application, not system-wide), meaning any proposal regarding parsing version info will fail for CSRSS. There are other system components that won't even show up via Windows API (although they are running), nor if they do show up somehow, hProcess will still be NULL (system process (part of ntoskrnl.exe, PID 0) will refuse to yield version info (for these, you must use sys.getwindowsversion and build versioning info). Note that we cannot catalog all sorts of system processes where hProcess will be NULL - things are changing fast.

Thanks.

josephsl · 2019-06-14T15:50:02Z

Hi,

Actually, yes there is a utility for appPath: virtual machine hosts, executable path for immersive apps, and eventually, for running unit tests.

Thanks.

josephsl · 2019-08-16T17:01:16Z

Hi,

There is a related issue that was discovered while working on Windows 10 Calculator app module: no product info for certain universal apps, and I'm testing a potential solution. Because that issue is different from this PR, an issue/PR pair will be created that will reference this pull request.

Thanks.

josephsl · 2019-09-18T17:31:28Z

Hi,

Update on Windows Store app property: originally it asked user32::IsImmersiveProcess and answered "yes" or "no" depending on return value (1 or 0, respectively). However, this method isn't complete because:

What Windows reports as immersive app is actually not the case for File Explorer and other apps. In case of File Explorer, this is because it is a .Net (managed) application.
What Windows reports as a native (non-immersive) app turns out to be a Store app, the most notable being Office 365 package obtained from Microsoft Store.

A more accurate way is probing for app package info structure that ships with a Store package. Based on an app model XML, this structure records the following information:

Package name (publisher.name)
Product version (a four-part string separated by a dot).
Architecture: the intended architecture for this app (which can be "neutral" (can run anywhere) or a processor architecture).
Package language: the language the package is designed for (for many apps, it is either not defined (an empty string) or "neutral").
App ID: a unique app model identifier for this package.

This structure is returned by kernel32::GetPackageFullName function as a string representation (see #10108 for more details). Every Store package (native or managed, including Store version of NVDA) will return this structure despite immersive process function returning values that do not fully reflect what the underlying package is. There might be a use case for IsImmersiveProcess - perhaps looking to see if we're dealing with a managed application.

Note that the above remark is irrelevant on Windows 7 as it does not come with package info functions nor immersive process checker routine.

Thanks.

josephsl · 2020-04-14T19:34:04Z

Hi,

Suspended until further notice in light of #11006.

Thanks.

Adriani90 · 2020-04-14T20:58:21Z

@josephsl I understand #11006 addresses new pull requests, not already existing pull requests. If we don't continue work on existing pull requests, then we run into the same issue later on.
I suggest if possible, continue work on existing pull requests, so that the backlog can be reviewed and finalized as soon as possible.

josephsl · 2020-04-14T21:10:43Z

Hi, the reason for suspending it for now is to let higher priority PR’s to get a chance to get reviewed. Although this PR is important, it isn’t a high priority work. Thanks.

…re also supported. Re nvaccess#7894. Add appModuleHandler.AppModule.appPath property used to obtain the full path for the executable (e.g. C:\Windows\explorer.exe). Also, in app module docstring, clarify that app module packages (appmodname/__init__.py) are also supported.

AppModule.isWindowsStoreApp will call user32.dll::IsImmersiveProcess to determine if the app is hosted inside a Windows RT container. This is useful for dealing with universla apps on Windows 10.

appModule.appArchitecture will return the architecture for which the app is written. It can be x86 (32-bit app on 32-bit Windows), AMD64 (x64 app), or ARM64 (64-bit ARM app). This is accomplished by use of kernel32.dll's IsWow64Process2 (available on Windows 10 build 10586 and later).

appModule.isWindowsStoreApp: check if it is below Windows 8, also fixed tuple syntax. appModule.appArchitecture: variable renames to make it clear.

…. re nvaccess#7894. Thanks to work on ARM64 detection in appModule.is64BitProcess: use kernel32::isWow64Process2 directly and obtain either AMD64 or ARM64, falling back to AMD64 if that Windows API function isn't present (the function is present in Windows 10 Version 1511 (build 10586) and later).

…on of 32-bit ARM apps. Re nvaccess#7894. Windows 10 Version 1903 includes foundatoins to support 32-bit ARM apps. Because this means two 32-bit architectures must be handled, rewrite app architecture method to: 1. Record architecture to arch names inside a map. 2. Call kernel32::IsWow64Process2 first and obtain arch name fromthe arch values map. 3. If IsWow64process2 fails for some reason (Windows 7 through 10 Version 1507), return appropriate architecture name based on 64-bit process status.

Fixes include inline comments and spaces around dictionary operator.

… info for Microsoft Store apps. Re nvaccess#7894. User32.dll::IsImmersiveProcess won't quite cut it for Store apps: * File Explorer is seen as an immersive app but it is not really a Store application. * Converted desktop apps are seen as not immersive but they are Store apps. The more accurate way to determine this information is looking for app package info, which can be done by calling kernel32.dll::GetPackageFullName. This returns a string representation of app name, version, architecture, language, and app ID (the first two are used in nvaccess#4259). Every Store app comes with app model XML file that provides data for this structure (Store version of NVDA is no exception). Thus use this method, which is now part of a private function in the base app module so not only product name and version setter can use it, but also Store app property can call this method.

…fo function. Re nvaccess#7894. Use the dedicated package info function for obtaining product name and version: * If the package info is a string, info will be parsed. * If it is None, executable file info will be fetched.

…nderlying process is a Windows Store app. Re nvaccess#7894. User32.dll::IsImmersiveProcess won't work all the way, but package info fetching will (on Windows 8 and later). Therefore use the package info method to determine if a given process is a Windows Store app.

…ndowsversion(). Re nvaccess#7894.

josephsl · 2020-04-14T21:46:06Z

Hi,

The PR is trivial enough for a fast forward review, but I imagine there might be scenarios where testing might be required. Even then, I think there are PR's that are more urgent than this one (depends on PR tags).

Thanks.

feerrenrut · 2020-04-15T10:53:01Z

Suspended until further notice in light of #11006.

Please continue to work on this PR. #11006 requests no new PR's. This PR already exists and is thus in the backlog of PR's that we want to address before accepting new ones. We are trying to stop PR's from going stale, and then becoming hard to merge.

Forgive me for replying to you multiple times, I want to avoid the community being mislead.

josephsl · 2020-04-15T10:55:06Z

Rebasing was successful, so it is ready for review again.

michaelDCurran · 2020-07-06T02:22:04Z

+		# Others such as Store version of Office are not truly hosted apps but are distributed via Store.
+		length = ctypes.c_uint()
+		buf = ctypes.windll.kernel32.GetPackageFullName(self.processHandle, ctypes.byref(length), None)
+		packageFullName = ctypes.create_unicode_buffer(buf)


The argument to create_unicode_buffer should be length, not buf I believe?
According to https://docs.microsoft.com/en-us/windows/win32/api/appmodel/nf-appmodel-getpackagefullname
GetPackageFullname returns ERROR_SUCCESS or an error code. The needed length is written to the length parameter.

josephsl · 2020-07-06T02:53:25Z

Hi, you’re right. Thanks for clarifying this.

…er to obtain actual package info length (addressing review action). Re nvaccess#7894.

josephsl changed the title ~~App modules: add additional app properties such ah path, immersive process, and app architecture~~ App modules: add additional app properties such as app path, immersive process, and app architecture Aug 18, 2018

josephsl requested a review from michaelDCurran August 18, 2018 02:03

LeonarddeR reviewed Aug 20, 2018

View reviewed changes

josephsl force-pushed the i7894appModProperties branch from 09fff58 to c4d12d4 Compare January 3, 2019 18:52

LeonarddeR added the AfterPython3Transition label Apr 24, 2019

feerrenrut removed the AfterPython3Transition label May 8, 2019

feerrenrut self-requested a review May 9, 2019 17:45

francipvb reviewed Jun 14, 2019

View reviewed changes

josephsl force-pushed the i7894appModProperties branch from a7e0c01 to 90e5d60 Compare June 21, 2019 05:11

josephsl force-pushed the i7894appModProperties branch from 029715c to aef37aa Compare July 12, 2019 15:31

josephsl force-pushed the i7894appModProperties branch 2 times, most recently from 8547b4a to 6ea4767 Compare August 2, 2019 19:54

josephsl force-pushed the i7894appModProperties branch from 6ea4767 to 9a5b082 Compare September 18, 2019 18:42

feerrenrut added audience/nvda-dev PR or issue is relevant to NVDA / Add-on developers enhancement Addon/API Changes to NVDA's API for addons to support addon development. labels Apr 8, 2020

josephsl added 12 commits April 14, 2020 14:18

App modules: add isWindowsStoreApp property. Re nvaccess#7894.

e1251c0

AppModule.isWindowsStoreApp will call user32.dll::IsImmersiveProcess to determine if the app is hosted inside a Windows RT container. This is useful for dealing with universla apps on Windows 10.

App module properties: address review comments. Re nvaccess#7894.

e8bd190

appModule.isWindowsStoreApp: check if it is below Windows 8, also fixed tuple syntax. appModule.appArchitecture: variable renames to make it clear.

App architecture: mention ARM32. re nvaccess#7894.

3eb9766

Lint fixes.

2220304

Fixes include inline comments and spaces around dictionary operator.

Windows Store app property: use platform version tuple from sys.getwi…

e827cfc

…ndowsversion(). Re nvaccess#7894.

josephsl force-pushed the i7894appModProperties branch from 386e220 to e827cfc Compare April 14, 2020 21:34

dpy013 mentioned this pull request Apr 23, 2020

Touch interaction: allow users to disable touch support completely #10557

Merged

feerrenrut assigned michaelDCurran Jul 3, 2020

michaelDCurran requested changes Jul 6, 2020

View reviewed changes

Merge branch 'master' into i7894appModProperties

4c8db91

App modules/immersive package info: use length instead of unused buff…

aea0f92

…er to obtain actual package info length (addressing review action). Re nvaccess#7894.

michaelDCurran approved these changes Jul 6, 2020

View reviewed changes

michaelDCurran merged commit 4799068 into nvaccess:master Jul 6, 2020

nvaccessAuto added this to the 2020.3 milestone Jul 6, 2020

michaelDCurran added a commit that referenced this pull request Jul 6, 2020

Update what's new for pr #8644

fc4625b

michaelDCurran mentioned this pull request Jul 6, 2020

App modules/WWAHost: use package functions to determine product name/version and which app module to load #9119

Merged

josephsl deleted the i7894appModProperties branch July 19, 2020 22:09

Uh oh!

Conversation

josephsl commented Aug 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Link to issue number:

Summary of the issue:

Description of how this pull request fixes the issue:

Testing performed:

Known issues with pull request:

Change log entry:

Uh oh!

josephsl commented Aug 18, 2018

Uh oh!

LeonarddeR Aug 20, 2018

Choose a reason for hiding this comment

Uh oh!

josephsl commented Aug 20, 2018 via email • edited by feerrenrut Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

francipvb commented Jun 14, 2019

Uh oh!

josephsl commented Jun 14, 2019 via email • edited by feerrenrut Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

francipvb commented Jun 14, 2019

Uh oh!

josephsl commented Jun 14, 2019 via email • edited by feerrenrut Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

francipvb commented Jun 14, 2019 via email • edited by feerrenrut Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

francipvb Jun 14, 2019

Choose a reason for hiding this comment

Uh oh!

josephsl commented Jun 14, 2019

Uh oh!

josephsl commented Jun 14, 2019

Uh oh!

josephsl commented Aug 16, 2019

Uh oh!

josephsl commented Sep 18, 2019

Uh oh!

josephsl commented Apr 14, 2020

Uh oh!

Adriani90 commented Apr 14, 2020

Uh oh!

josephsl commented Apr 14, 2020 via email • edited by feerrenrut Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

josephsl commented Apr 14, 2020

Uh oh!

feerrenrut commented Apr 15, 2020

Uh oh!

josephsl commented Apr 15, 2020 via email • edited by feerrenrut Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michaelDCurran Jul 6, 2020

Choose a reason for hiding this comment

Uh oh!

josephsl commented Jul 6, 2020 via email • edited by feerrenrut Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

josephsl commented Aug 18, 2018 •

edited

Loading

josephsl commented Aug 20, 2018 via email •

edited by feerrenrut

Loading

josephsl commented Jun 14, 2019 via email •

edited by feerrenrut

Loading

josephsl commented Jun 14, 2019 via email •

edited by feerrenrut

Loading

francipvb commented Jun 14, 2019 via email •

edited by feerrenrut

Loading

josephsl commented Apr 14, 2020 via email •

edited by feerrenrut

Loading

josephsl commented Apr 15, 2020 via email •

edited by feerrenrut

Loading

josephsl commented Jul 6, 2020 via email •

edited by feerrenrut

Loading