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

josephsl · 2019-01-03T05:19:30Z

Link to issue number:

Partly fixes #4259
Fixes #4569

Summary of the issue:

Adds ability to let NVDA load appropriate app modules for apps hosted by WWAHost.

Description of how this pull request fixes the issue:

Enhancements to WWAHost app module:

App module imports: uses kernel32.dll::GetApplicationUserModelId to fetch app model, returning the app name converted to lowercase.
Browse mode: focus mode will be entered no matter which app is in use.

Testing performed:

Tested on Windows 8.1 and 10 with various web apps hosted by WWAHost.exe, including Store app on Windows 8.1 and Twitter PWA (progressive web app) on Windows 10.

Known issues with pull request:

App modules wishing to support apps hosted by WWAHost must import contents of wwahost app module (at least the app module class) to take advantage of services implemented, including future additions and changes.

Change log entry:

Bug fixes:

On Windows 8 and later, apps hosted inside wwahost.exe now shows correct product name and version for the hosted app. (Windows Store apps: use app title as product name instead of wwahost #4259)

Although note that the above is an optional bug fix entry, as the change is already part of NVDA since 2019.3.

Changes for developers:

It is now possible to create app modules for apps hosted inside wwahost.exe on Windows 8 and later. (Load specific app modules for apps hosted by wwahost.exe #4569)

Thanks.

…on. Re nvaccess#4259. AppModule.productName/productVersion looks at product name/version fields for the executable itself. For WWAHost, this means a generic product name (Microsoft Windows operating system) and version (Windows major.minor.build.revision) will be returned, thus not giving an accurate picture as to which app is being hosted and the guest app version. Thankfully, there is a function (kernel32.dll::GetPackageFullName) that will return a serialized package info text, which does return the actual product name and version of the hosted app. The package info structure consits of name, version, publisher, architecture and other relevant information for a package given the process ID for the app container, and is joined via an underscore character (_), with the first two indecies being publisher.name and version, respectively. Thus modified AppModule._setProductInfo for wwahost to fetch the app info via the function described above. This then allows the correct name and version of the hosted app to be returned. Note that in some cases, product version is 1.0.0.0 (especially Store app in Windows 8 and 8.1), but this can be ignored.

…4569. Just like Javaw (hosted Java apps): different app modules for apps hosted by WWAHost are desirable for handling individual apps. Thus fetch which app module to load via a call to kernel32.dll::GetApplicationUserModelId. The app model text consists of app family name, an exclamation point, and the app name. Because Python can't import files with exclamation marks in the middle of their name, just return the lowercase version of app name (latter half of the app model text). Note that app modules for WWAHost apps must import contents of wwahost app module in order to take advantage of package-specific product name/version field, as well as other potential services in the future.

josephsl · 2019-01-03T05:21:26Z

Hi,

To @jcsteh: although Mick was requested for review, I'd like to request a second look from you (if you have time) since you're the one who gave me advice on how to proceed with this.

Thanks.

LeonarddeR · 2019-01-04T13:22:34Z

source/appModules/wwahost.py

+"""App module host for Windows 8.x and 10 web apps hosted by wwahost.exe.
+In Windows 8, apps written in Javascript are executed inside WWAHost, including some WinRT apps.
+In Windows 10, progressive web apps (PWA) and friends are hosted inside this process.
+App modules wishing to support apps hosted inside this process must import contents of this app module.


Do you mean subclassing here? Also, what would such appModules look like? It would help to have some documentation for people to know how to create appModules like this.

LeonarddeR · 2019-01-04T13:23:39Z

source/appModules/wwahost.py

+	if not appModel.value:
+		raise LookupError
+	# Convert this into lowercase to make the file name consistent with other NVDA app modules.
+	return appModel.value.split("!")[-1].lower()


What does the explanation mark mean here?

josephsl · 2019-01-04T15:08:29Z

Hi,

Subclassing: yep, I guess you're right in saying that app modules must be subclassed (will edit the documentation).

App modules for these: in Windows 10, they're really web apps, so they can utilize EdgeHTML code, along with adding keyboard shortcuts, fixing labels and such. In case of Windows 8.x, app modules for these can add support for native commands and workarounds.

Regarding exclamation point: this is part of the app model string and is used by Windows to communicate with the app in question. A typical app model text consists of family name and app name, joined together via an exclamation mark (familyName!appName). Because Python can't import files with an exclamation point in the middle of the name (another case of #5323), and to present a more friendly app name, app model text is split around the exclamation point.

To test this pull request, try using a progressive web app in Windows 10 such as Twitter app.

Thanks.

Reviewed by Leonard de Ruijter (Babbage): clarify docstring, and give an explanation as to how app model text is constructed and reasoning behind returning only a portion of it.

jcsteh · 2019-01-10T23:31:36Z

developerGuide.t2t

@@ -228,6 +229,44 @@ Events will be covered in greater detail later.

 As with other examples in this guide, remember to delete the created app module when you are finished testing and then restart NVDA or reload plugins, so that original functionality is restored.

++ App modules for hosted apps ++
+Some executables host various apps inside.
+These include javaw.exe for running various java programs and wwahost.exe for web apps in Windows 8 and later.


I think we need to be more specific than just "web apps" here. I'm concerned people might think we're referring to web apps even on the web. Perhaps we could say "some web apps in the Windows Store"? Note "some" as well, because not all web apps in the Store are hosted in wwahost.

jcsteh · 2019-01-10T23:36:24Z

developerGuide.t2t

+
+Note that wwahost.exe hosts web applications or apps behaving as such.
+You should create an app module for apps hosted inside it if you believe the hosted app is not a true web application.
+This is the case for built-in apps such as Mail and Calendar in Windows 8 and 8.1.


I know what you're trying to say here, but I don't think this paragraph will be understood by most people. First, the base wwahost app module (which they are subclassing) already disables browse mode. Second, "true web application" is a pretty hard thing to define. Arguably, any app that uses web technologies is a true web app. The question is more whether users want to interact with the content primarily as a document or primarily using the app's own keyboard support. I would suggest you just remove this paragraph altogether and let devs make the decision on their own. If they feel the need to write an app module for a wwahost app and are reading this section, they've already made the decision; this guidance isn't helpful at that point and might just confuse them.

jcsteh · 2019-01-10T23:39:44Z

developerGuide.t2t

+
+If an app runs inside a host executable, the name of the app module must be the name as defined by the host executable, which can be found through AppModule.appName property.
+For example, an app module for a java app named "test" hosted inside javaw.exe must be named test.py.
+For apps hosted inside wwahost, not only the app module name must be the name of the loaded app, but the app module must subclass the app module class found in wwahost.


nit: "not only the app module name must be the name of the loaded app" -> "not only must the app module name be the name of the loaded app" (move the word "must")

jcsteh · 2019-01-10T23:47:13Z

source/appModules/wwahost.py

+	processHandle = winKernel.openProcess(winKernel.SYNCHRONIZE|winKernel.PROCESS_QUERY_INFORMATION,False,processId)
+	length = ctypes.c_uint()
+	buf = winKernel.kernel32.GetApplicationUserModelId(processHandle, ctypes.byref(length), None)
+	appModel = ctypes.create_unicode_buffer(buf)


GetApplicationUserModelId returns an error code, not a length. You should create a buffer using length.value. You can ignore the error code in this case. In any case, I don't recommend using the name buf to store a length; it's confusing.

jcsteh · 2019-01-10T23:47:58Z

source/appModules/wwahost.py

+			raise RuntimeError("processHandle is 0")
+		length = ctypes.c_uint()
+		buf = winKernel.kernel32.GetPackageFullName(self.processHandle, ctypes.byref(length), None)
+		packageFullName = ctypes.create_unicode_buffer(buf)


Same as above re length.

…via length.value. Re nvaccess#4259. Comment from James Teh (Mozilla): try using length.value instead of a dedicated buffer variable, as the function used returns nonimportant error codes instead of actual length.

josephsl · 2019-01-11T01:13:27Z

Hi,

To @jcsteh: I can see why the explanatory paragraph can become confusing, as not all apps are hosted inside wwahost.exe.

I also tested length.value idea and it does work (thanks for the correction).

Note that if this PR comes into master, we would lose application role assignment workaround from Windows 8.x days, but it can be restored for individual apps if needed, resolving one or more bugs also.

Thanks.

josephsl · 2019-08-14T05:52:32Z

Hi,

I'll merge in latest work from master, including ability to tell NVDA that content hosted by WWAHost should not become a browse mode document, and taking care of linting issues that might arise as a result. Then I think it should be ready for a more general review.

Thanks.

…ult. Although content appears to be a webpage, it is actually part of an app hosted inside WWAHost. This includes apps from Windows 8.x days, as well as progressive web apps (PWA's) that are not really browse mode documents. Apps requiring browse mode may override 'disable browse mode by default' flag to suit their needs.

Lint fixes include splitting long lines and comment style.

josephsl · 2019-09-08T03:07:50Z

Hi,

Change of plan: PR #10114 will deal with product name and version, as that PR also affects this one, too. This then simplifies pull request by adding a facility to let NVDA detect appropriate modules for ones hosted by WWAHost. The content of developer guide introduced with this PR won't change.

Thanks.

A more generic way of setting product name and version for hosted apps, including those hosted within WWAHost, will be used from base app module, thus this setter method is no longer needed.

feerrenrut · 2020-04-08T13:25:55Z

Since this is related: I'd like to see a change to the way that we choose app modules. Rather than match the name of the appModule with the process name, I'd like a new function introduced on appModules that is given some details (or a way of getting details about the process) and returns True if it is a match. This could help to prevent having multiple appModules files for the same application, it could also give us more flexibility with supporting multiple versions of the same application.

josephsl · 2020-04-08T16:49:36Z

Hi,

Before resolving merge conflicts, regarding app module specificiations for hosted apps:

Ad @LeonarddeR or osmeone pointed out, we might as well inspect app package data to see which app should be loaded based on the following:

App vendor
App name
App version
Architecture (if required)

This will work with Windows Runtime/UWP/WWA Host apps, not necessarily for Java GUI (javaw) and windowed Python (pyw) apps. Making matters more interesting is support for hosted apps in Windows 10 Version 2004 where an app can specify another app as host when run.

I think it might be best to do a separate PR for importing app modules based on specs - preferably from app hosts as they may have different ways to locate specs unless a unified approach can be taken.

Thanks.

feerrenrut · 2020-04-08T16:57:06Z

I think it might be best to do a separate PR for importing app modules based on specs - preferably from app hosts as they may have different ways to locate specs unless a unified approach can be taken.

I agree it should be out of scope for this PR. I was thinking we could give the control to a module level function to decide whether it supports a given app. Perhaps the function would return the appropriate class (type or instance as appropriate) that supports the given process. I'm thinking about how we can encourage re-use in the appmodules but also enable supporting multiple versions of applications or the same version with multiple process names. But this should probably be discussed on an issue specific to that idea.

josephsl · 2020-04-08T17:13:41Z

Hi, in the interim, I think this PR is ready to go once conflicts are resolved unless folks have other concerns. Thanks.

jcsteh · 2020-04-08T22:13:05Z

Since this is related: I'd like to see a change to the way that we choose app modules. Rather than match the name of the appModule with the process name, I'd like a new function introduced on appModules that is given some details (or a way of getting details about the process) and returns True if it is a match. This could help to prevent having multiple appModules files for the same application, it could also give us more flexibility with supporting multiple versions of the same application.

Perhaps I'm misunderstanding, but do you mean we'd no longer load module based on process name at all? That would mean we'd have to load all app modules (87 in core alone) when NVDA starts, and whenever an app starts, we'd have to run the check function in (at worst) every app module. That sounds pretty expensive to me (processing, i/o, memory) and is why we haven't done things like this in the past.

As an aside, my first attempt at braille display detection required all drivers to be imported to check whether they matched. I abandoned that (in favour of the centralised bdDetect approach we have now) because of this problem: loading all drivers wastes time and memory, especially if we're only going to use one of them.

feerrenrut · 2020-04-09T07:05:16Z

That sounds pretty expensive to me (processing, i/o, memory) and is why we haven't done things like this in the past.

Certainly a concern that would need to be measured and mitigated. At this stage I'm just thinking about a "wish list" to make development and maintenance of appmodules easier.

michaelDCurran · 2020-07-06T04:20:40Z

@josephsl Is there any further work required on this pr since pr #8644 was merged? or is it unrelated.

josephsl · 2020-07-06T04:27:16Z

Hi, let me make one change: buf should not be passed into Unicode buffer creation function (#8644 comment). Thanks.

josephsl · 2020-07-06T05:05:20Z

Hi,

Actually, the change I was talking about above is already there (no change needed). And yes, this PR is different than #8633, so it can be merged.

Thanks.

michaelDCurran · 2020-07-06T06:41:38Z

source/appModules/wwahost.py


 class AppModule(appModuleHandler.AppModule):

+	# WWAHost app content is treated as part of an app, not a browse mode document.
+	disableBrowseModeByDefault = True
+
 	def event_NVDAObject_init(self,obj):


If I understand correctly, all code from here to the end of the file can be removed. This code coerces the root of a Win8 metro app to ROLE_APPLICATION from ROLE_DOCUMENT. this was done simply to disable automatic browse mode for these apps. Since however browse mode is disabled above for all wwahost apps, this code is now completely redundant... Unless I'm missing something?

josephsl · 2020-07-06T06:47:58Z

Hi, it can be considered redundant, although this means we won’t get NVDA to say “application” when a WWA Host app gains focus. I think the situation is different than 2012 (I distinctly remember testing this back then) given that Windows 10 PWA’s hosted inside WWA Host uses EdgeHTML. Thanks.

michaelDCurran · 2020-07-06T06:57:09Z

Fair enough. However, under Known Issues in the description, you still have "Also, this breaks application role strategy used in Windows 8.x, which means we need to think of a way to let this trick work, or re-examine app role strategy." Is there actually a breaking change with this pr that will affect existing code? Can you either clarify that statement further or remove it?

… mode off flag. Comment from Mick Curran (NV Access): in 2012, WWA Host app module was reponsible for turning what was essentially a web application into a proper 'application'. This was needed prior to introduction of browse mode flag for app modules. Now that this flag is present, there is no need for role coercion anymore (users can toggle browse mode instead).

josephsl · 2020-07-06T07:23:45Z

Hi,

Removed - I didn't see any third-party app modules being affected by this - haven't come across potential ones in add-ons community.

Thanks.

josephsl added 3 commits January 2, 2019 20:55

App modules/WWAHost: update copyright header.

ad6ff9f

josephsl requested a review from michaelDCurran January 3, 2019 05:19

LeonarddeR reviewed Jan 4, 2019

View reviewed changes

josephsl added 2 commits January 4, 2019 07:16

appModules/WWAHost: docstring updates. Re nvaccess#4569.

990308c

Reviewed by Leonard de Ruijter (Babbage): clarify docstring, and give an explanation as to how app model text is constructed and reasoning behind returning only a portion of it.

Dev guide: add sections and an example on hosted apps. Re nvaccess#4569.

9144784

jcsteh reviewed Jan 10, 2019

View reviewed changes

josephsl added 3 commits January 10, 2019 16:40

Merge branch 'master' into wwaHostAppInfo

05ba905

Dev guide: remove confusing paragraph on wwahost notess.

46e463a

LeonarddeR added the AfterPython3Transition label Apr 24, 2019

feerrenrut removed the AfterPython3Transition label May 8, 2019

josephsl added 4 commits August 13, 2019 23:09

Merged master

7e1ea20

WWAHost: lint fixes.

eafb125

Lint fixes include splitting long lines and comment style.

Merge branch 'master' into wwahostAppInfo

b8e75b3

WWAHost: remove product name and version setter. Re nvaccess#4259.

25b8d06

A more generic way of setting product name and version for hosted apps, including those hosted within WWAHost, will be used from base app module, thus this setter method is no longer needed.

This was referenced Sep 8, 2019

Load specific app modules for apps hosted by wwahost.exe #4569

Closed

WWAHost/app window: separate app window role coercion from app module #9117

Open

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

This comment has been minimized.

Sign in to view

josephsl added 2 commits April 8, 2020 10:21

Merged master

3f7763a

appModules/wwahost: update copyright header

9d11ced

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

Merge branch 'master' into wwahostAppInfo

4c2c2d9

michaelDCurran reviewed Jul 6, 2020

View reviewed changes

michaelDCurran approved these changes Jul 6, 2020

View reviewed changes

michaelDCurran merged commit 7b1d680 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 #9119

1e5a1b9

josephsl deleted the wwahostAppInfo branch July 19, 2020 22:08

lukaszgo1 mentioned this pull request Mar 1, 2022

Improve flexibility of appModules discovery by allowing to map arbitrary appModule to a given executable. #13366

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

josephsl commented Jan 3, 2019 •

edited

josephsl commented Jan 3, 2019

LeonarddeR Jan 4, 2019

LeonarddeR Jan 4, 2019

josephsl commented Jan 4, 2019

jcsteh Jan 10, 2019

jcsteh Jan 10, 2019

jcsteh Jan 10, 2019

jcsteh Jan 10, 2019

jcsteh Jan 10, 2019

josephsl commented Jan 11, 2019

josephsl commented Aug 14, 2019

josephsl commented Sep 8, 2019

feerrenrut commented Apr 8, 2020

This comment has been minimized.

josephsl commented Apr 8, 2020

feerrenrut commented Apr 8, 2020

josephsl commented Apr 8, 2020 via email •

edited by feerrenrut

jcsteh commented Apr 8, 2020

feerrenrut commented Apr 9, 2020

michaelDCurran commented Jul 6, 2020

josephsl commented Jul 6, 2020 via email •

edited by feerrenrut

josephsl commented Jul 6, 2020

michaelDCurran Jul 6, 2020

josephsl commented Jul 6, 2020 via email •

edited by feerrenrut

michaelDCurran commented Jul 6, 2020 via email

josephsl commented Jul 6, 2020

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

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

Conversation

josephsl commented Jan 3, 2019 • edited

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:

josephsl commented Jan 3, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josephsl commented Jan 4, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josephsl commented Jan 11, 2019

josephsl commented Aug 14, 2019

josephsl commented Sep 8, 2019

feerrenrut commented Apr 8, 2020

This comment has been minimized.

josephsl commented Apr 8, 2020

feerrenrut commented Apr 8, 2020

josephsl commented Apr 8, 2020 via email • edited by feerrenrut

jcsteh commented Apr 8, 2020

feerrenrut commented Apr 9, 2020

michaelDCurran commented Jul 6, 2020

josephsl commented Jul 6, 2020 via email • edited by feerrenrut

josephsl commented Jul 6, 2020

Choose a reason for hiding this comment

josephsl commented Jul 6, 2020 via email • edited by feerrenrut

michaelDCurran commented Jul 6, 2020 via email

josephsl commented Jul 6, 2020

josephsl commented Jan 3, 2019 •

edited

josephsl commented Apr 8, 2020 via email •

edited by feerrenrut

josephsl commented Jul 6, 2020 via email •

edited by feerrenrut

josephsl commented Jul 6, 2020 via email •

edited by feerrenrut