Look for Firefox in HKCU as well as HKLM (fix #73)

[mccreery]

Fix for #73. After revising my approach I'm fairly certain it works correctly. I extracted a couple of functions from normalizeBinary for getting the relevant registry values as promises and test in turn both registry hives before falling back on the standard program files path as before.

Tested by editing the registry to the following circumstances:

• Both keys in place, opens Firefox correctly
• Only HKLM key in place, old path found as before
• Both keys missing, falls back on program files as before

[Rob--W]

Implementation looks OK.

Could you add a unit test to verify the expected behavior in test/run/utils.js ?

[mccreery]

Could you add a unit test to verify the expected behavior in test/run/utils.js ?

I'm not sure how to do this since I probably shouldn't be modifying registry keys for automated tests. Any ideas?

[Rob--W]

Could you add a unit test to verify the expected behavior in test/run/utils.js ?

I'm not sure how to do this since I probably shouldn't be modifying registry keys for automated tests. Any ideas?

That test file mocks the get method of the winreg module, so that a dummy value is returned instead of the real keys.
You can find examples by searching for .get = in test/run/test.utils.js. I encourage you to see multiple examples to get a better idea, but here is one slightly more complicated one:

node-fx-runner/test/run/test.utils.js

Lines 163 to 177 in f1f0358

	var binary = sandbox.require("../../lib/utils", {
	requires: {"winreg": function(options) {
	var value = "Normal or beta";
	if (options.key.toLowerCase().indexOf("nightly") != -1) {
	value = "nightly";
	}
	if (options.key.toLowerCase().indexOf("aurora") != -1) {
	value = "aurora";
	}
	this.get = function(_, fn) {
	fn(null, {value: value});
	};
	}},
	locals: {process: {env: {"ProgramFiles": "envPath32", "ProgramFiles(x86)": "envPath64"}}}
	}).normalizeBinary;

[mccreery]

I have added a new test for the expected behaviour and slightly modified the old registry test to ensure it checks the old behaviour* is consistent (so if you only have keys in HKLM it still finds the path).

Edit as per this comment:
*The 'old behaviour' I describe is that if all the required keys are present in HKLM (which would have worked before this PR) but not in HKCU, the path from HKLM will be used. I updated the test to ensure that checking HKCU is not a regression for users who don't have the HKCU keys (for whatever reason).

[Rob--W]

I suppose that you changed the implementation to veify that the logic still works as intended when there is a HKLM-only registry key (i.e. without HKCU)? Can you clarify that intent in the comment, since it isn't obvious.

[mccreery]

That's right. If you have all the correct keys in HKLM and some missing in HKCU the intended behaviour (as I see it) is to ignore HKCU and fall back on HKLM for all the keys. I also edited the comment above clarifying this. The code fails any access to HKCU to mimic the missing keys.

[Rob--W]

I meant in a code comment, not here :)

[mccreery]

Haha, I thought it was a strange ask :P Will do.

[Rob--W]

Thanks for the patch and quick responses!