1818418 - [MSIX] Allow to use Windows UserChoice default mechanisms for MSIX packages

Status: RESOLVED FIXED

(Firefox :: Installer, enhancement, P3)

Description

[Nick Alexander :nalexander [he/him]]

Set default by UserChoice doesn't work in MSIX packages. It would be nice if it did.

Comment 1

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1818418 - Use `reg.exe` to implement MSIX 1-click set-to-default. r=mhughes

This sketches how to use reg.exe to delete and recreate the relevant
HKCU registry key-value pairs.

This needs at least three additional things:

1. The appropriate Prog IDs need to be determined from the registry.

   In the JS console, something like the following snippet enumerates
   Prog IDs created by the MSIX installer for the given package.
   Doing this in C++ should allow to replace my hard-coded values
   present in this WIP.

2. The WDBA needs to be taught to recognize when its run in an app
   package context and to alter its behaviour. I tested this
   outside of the app package bubble so that I could iterate
   quickly. I've not yet verified that invoking it from Firefox
   proper succeeds, although I'm pretty confident that it will.

3. ShellService.jsm also checks that the Prog IDs exist and won't
   invoke default-browser-agent.exe from within the app package
   bubble. This might be updated as part of 2. automatically, but it
   might also need to share the registry enumeration of 1.

var testKey = Cc["@mozilla.org/windows-registry-key;1"].createInstance(Ci.nsIWindowsRegKey);

    testKey.open(
      Ci.nsIWindowsRegKey.ROOT_KEY_CURRENT_USER,
      "SOFTWARE\\Classes",
      Ci.nsIWindowsRegKey.ACCESS_ALL
    );

var winPackageFamilyName = Services.sysinfo.getProperty("winPackageFamilyName");

var [packageName, packageHash] = winPackageFamilyName.split("_");

for (var i = 0; i < testKey.childCount; i++) {
  var name = testKey.getChildName(i);
  if (name.startsWith("AppX")) {
  	var innerKey = Cc["@mozilla.org/windows-registry-key;1"].createInstance(Ci.nsIWindowsRegKey);
		try {
    innerKey.open(
      Ci.nsIWindowsRegKey.ROOT_KEY_CURRENT_USER,
      `SOFTWARE\\Classes\\${name}\\Shell\\open`,
      Ci.nsIWindowsRegKey.ACCESS_READ
    );
    var packageId = innerKey.readStringValue("PackageId");

    if (packageId.startsWith(packageName) && packageId.endsWith(packageHash)) {
      // Distinguish protocols (http, https) from file extensions (.html, .pdf).
      var contractId = innerKey.readStringValue("ContractId");

      var classKey = Cc["@mozilla.org/windows-registry-key;1"].createInstance(Ci.nsIWindowsRegKey);
      classKey.open(
          Ci.nsIWindowsRegKey.ROOT_KEY_CURRENT_USER,
          `SOFTWARE\\Classes\\${name}`,
          Ci.nsIWindowsRegKey.ACCESS_READ
      );

        try {
          protocol = "";
        var protocol = classKey.readStringValue("");
        } catch (ee) {
        }

      var commandKey = innerKey.openChild("command", Ci.nsIWindowsRegKey.ACCESS_READ);
      console.log(name, protocol, commandKey.readStringValue("DelegateExecute"));
    }
    } catch (e) {
      // console.log("error", name, e);
    }
    finally {
      innerKey.close();
    }
  }
}

Comment 2

[Nick Rishel [:nrishel]]

Your application starts a utility to perform tasks. Avoid starting command utilities such as PowerShell and Cmd.exe. In fact, if users install your application onto a system that runs the Windows 10 S, then your application won't be able to start them at all. This could block your application from submission to the Microsoft Store because all apps submitted to the Microsoft Store must be compatible with Windows 10 S.

Starting a utility can often provide a convenient way to obtain information from the operating system, access the registry, or access system capabilities. However, you can use UWP APIs to accomplish these sorts of tasks instead. Those APIs are more performant because they don't need a separate executable to run, but more importantly, they keep the application from reaching outside of the package. The app's design stays consistent with the isolation, trust, and security that comes with an application that you've packaged, and your application will behave as expected on systems running Windows 10 S.

https://learn.microsoft.com/en-us/windows/msix/desktop/desktop-to-uwp-prepare

Looks like reaching outside of the package in this way is explicitly supported for non-S Mode packages even if it's frowned upon. Since the S Mode requirement has since been dropped from the Windows Store I think we can assume we're in the clear.

Comment 3

[Nick Alexander :nalexander [he/him]]

(In reply to Nick Rishel [:nrishel] from comment #2)

Looks like reaching outside of the package in this way is explicitly supported for non-S Mode packages even if it's frowned upon. Since the S Mode requirement has since been dropped from the Windows Store I think we can assume we're in the clear.

Great sleuthing!

Comment 4

[Nick Rishel [:nrishel]]

Attachment: WIP: Bug 1818418 - Dynamically find MSIX ProgIDs. r=nalexander

Comment 6

[Nick Rishel [:nrishel]]

Attachment: Bug 1818418 - Pre 1: Include cleanup. r=nalexander

Comment 7

[Nick Rishel [:nrishel]]

Attachment: Bug 1818418 - Pre 2: Remove comment documenting command that's no longer available. r=nalexander.

Depends on D192176

Comment 8

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1818418 - TO FOLD - Tweaks to make things function. r=mhughes

Depends on D170717

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e0f1fa408c70
Use `reg.exe` to implement MSIX 1-click set-to-default. r=mhughes

Comment 10

[Nick Alexander :nalexander [he/him]]

No need to keep this internal now that it's landed.

Comment 11

[Atila Butkovits]

Backed out for causing bustages at WindowsUserChoice.cpp

Backout link: https://hg.mozilla.org/integration/autoland/rev/bf251a888e3d325af5d533856cecbb2c87ed26c3

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=e0f1fa408c701549e471dbb85f2fc6301e20eaa5&selectedTaskRun=EMwb6Q8IQjSkgghrrQANYw.0

Failure log: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=e0f1fa408c701549e471dbb85f2fc6301e20eaa5&selectedTaskRun=EMwb6Q8IQjSkgghrrQANYw.0

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/45ab2db62a65
Use `reg.exe` to implement MSIX 1-click set-to-default. r=mhughes

Comment 13

[Noemi Erli[:noemi_erli]]

Backed out changeset 45ab2db62a65 (Bug 1818418) for causing bustages in WindowsUserChoice.h

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&selectedTaskRun=UAm8qhxnT3ysEO8UFOPixA.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&searchStr=windows%2C2012%2Cx64%2Cdebug%2Cbuild-win64-non-unified%2Fplain%2Cbp-nu&revision=45ab2db62a6586d772adb701f247ee2f33d059d6

Log: https://treeherder.mozilla.org/logviewer?job_id=434963720&repo=autoland&lineNumber=131611

Backout: https://hg.mozilla.org/integration/autoland/rev/79d803473a6763f34e4d611fd63a6c47b9b879d0

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c6d6218c85ae
Use `reg.exe` to implement MSIX 1-click set-to-default. r=mhughes

Comment 15

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/c6d6218c85ae

Comment 16

[Bogdan Maris, Desktop Test Engineering]

We did a spot check on the latest MSIX Nightly build in order to see if/how this works and we can confirm that MSIX build can be set as the default browser and behaves as one once it is added as default.

Comment 17

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1818418 - Use `reg.exe` to implement MSIX 1-click set-to-default. r=mhughes

This approach uses reg.exe to delete and recreate the relevant HKCU
registry key-value pairs.

Original Revision: https://phabricator.services.mozilla.com/D170717

Comment 18

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1818418 - Use `reg.exe` to implement MSIX 1-click set-to-default. r=mhughes

This approach uses reg.exe to delete and recreate the relevant HKCU
registry key-value pairs.

Original Revision: https://phabricator.services.mozilla.com/D170717

Comment 19

[Phabricator Automation]

Uplift Approval Request

• Needs manual QE test: yes
• Explanation of risk level: This rearranges, but does not change, the non-MSIX functionality. QE has tested in Nightly, and there is very little difference in this functionality across release channels. Finally, failure of the 1-click set-to-default mechanism simply launches the Windows (multi-click) UI -- which is not a catastrophic outcome.
• Fix verified in Nightly: yes
• User impact if declined: No 1-click set-to-default in MSIX/Microsoft Store builds
• Steps to reproduce for manual QE testing: QE is already aware of steps
• String changes made/needed: No
• Risk associated with taking this patch: Low
• Code covered by automated testing: no
• Is Android affected?: no

Comment 20

[Dianna Smith [:diannaS]]

Comment on attachment 9362488 [details]
Bug 1818418 - Use reg.exe to implement MSIX 1-click set-to-default. r=mhughes

i think this was an accidental duplicate

Comment 21

[Phabricator Automation]

Uplift Approval Request

• Code covered by automated testing: no
• Needs manual QE test: yes
• String changes made/needed: No
• Is Android affected?: no
• User impact if declined: No 1-click set-to-default in MSIX/Microsoft Store builds
• Fix verified in Nightly: yes
• Steps to reproduce for manual QE testing: QE is already aware of steps
• Explanation of risk level: his rearranges, but does not change, the non-MSIX functionality. QE has tested in Nightly, and there is very little difference in this functionality across release channels. Finally, failure of the 1-click set-to-default mechanism simply launches the Windows (multi-click) UI -- which is not a catastrophic outcome.
• Risk associated with taking this patch: Low

Comment 22

[Dianna Smith [:diannaS]]

Comment on attachment 9362489 [details]
Bug 1818418 - Use reg.exe to implement MSIX 1-click set-to-default. r=mhughes

this will ride the 121 train for now

Comment 23

[Michael Hughes]

FX 121 Release store build with the changes for this bug rolled back: ms-windows-store://pdp/?productid=9NFQ5K7N2T6C

Comment 24

[Ryan VanderMeulen [:RyanVM]]

(In reply to Michael Hughes from comment #23)

FX 121 Release store build with the changes for this bug rolled back: ms-windows-store://pdp/?productid=9NFQ5K7N2T6C

Any luck testing this?

Comment 25

[Bogdan Maris, Desktop Test Engineering]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #24)

(In reply to Michael Hughes from comment #23)

FX 121 Release store build with the changes for this bug rolled back: ms-windows-store://pdp/?productid=9NFQ5K7N2T6C

Any luck testing this?

Yes, we tested this as soon as the build was available (only wrote in slack thread, sorry for that) and can confirm that using the build provided (Fx 121), 1-click set-to-default in MSIX does not work meaning that this is fixed using the build ms-windows-store://pdp/?productid=9NFQ5K7N2T6C.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

Backed out for 121.0.1 due to bug 1863980. This is still in place for 122+.
https://hg.mozilla.org/releases/mozilla-release/rev/484be1feb7138af0917df98dda85050e0a3317a8

Comment 27

[Donal Meehan [:dmeehan]]

:amir for tracking purposes, this was backed out in central and beta (at the time Fx123 and Fx122) see comments in https://phabricator.services.mozilla.com/D197747

For tracking purposes, you might want to reopen this?

Comment 28

[Bogdan Maris, Desktop Test Engineering]

I am going ahead and drop the qa-verify+ flag for now since the work here is done at this time, if something changes and qa is needed again please re-add the flag.