Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions gpii/node_modules/lifecycleManager/src/LifecycleManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -1141,7 +1141,7 @@ var fluid = fluid || require("infusion"),
if (desiredRunState === true) { // and it was running when we started
actions = [ configurationType, "start" ];
} else { // just restore settings
actions = [ "restore" ];
actions = [ "restore" ]; // TODO: this should probably be "update" too - since the case of no change in run state implies common action on settings
}
}
return actions;
Expand Down Expand Up @@ -1436,7 +1436,8 @@ var fluid = fluid || require("infusion"),
if (!fluid.isDestroyed(that)) { // See above comment for GPII-580
// if solution is already running, call "update" directive - else use "start" directive
var isRunning = userSession.model.runningOnLogin[solutionId];
var actions = gpii.lifecycleManager.calculateLifecycleActions(isRunning, solution.active);
var shouldBeRunning = !!gpii.lifecycleManager.getSolutionRunningStateFromSnapshot(solution) && solution.active;
var actions = gpii.lifecycleManager.calculateLifecycleActions(isRunning, shouldBeRunning/*solution.active*/);

// build structure for returned values (for later reset)
return that.applySolution(solutionId, solution, userSession, actions, "start");
Expand Down
24 changes: 14 additions & 10 deletions gpii/node_modules/lifecycleManager/test/js/LifecycleManagerTests.js
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,8 @@ https://github.com/GPII/universal/blob/master/LICENSE.txt
appliedSettings: { "cross-hairs-clip": false, "cross-hairs-color": "green" },
runningAfterLogin: true,
expectedLaunchHandlerCalls: 2
}, {
}, /* Commented out for GPII-4468 - the role of "active" is unclear during matchmaking
{
name: "solution with active: false", // no launch handler calls
basePayload: gpii.tests.lifecycleManager.buildLifecycleInstructions(
"org.gnome.desktop.a11y.magnifier",
Expand Down Expand Up @@ -587,7 +588,7 @@ https://github.com/GPII/universal/blob/master/LICENSE.txt
appliedSettings: { "cross-hairs-clip": false, "cross-hairs-color": "green" },
runningAfterLogin: undefined,
expectedLaunchHandlerCalls: 0
}, {
}, */{
name: "running on login - 'update' with reference to settings handler block",
basePayload: gpii.tests.lifecycleManager.buildLifecycleInstructions(
"org.gnome.desktop.a11y.magnifier",
Expand All @@ -601,7 +602,7 @@ https://github.com/GPII/universal/blob/master/LICENSE.txt
originalSettings: { "cross-hairs-clip": false, "cross-hairs-color": "red" },
runningOnLogin: true,
appliedSettings: { "cross-hairs-clip": false, "cross-hairs-color": "green" },
runningAfterLogin: undefined,
runningAfterLogin: true,
expectedLaunchHandlerCalls: 0
}, {
name: "running on login - 'update' with reference to 'configure' block",
Expand All @@ -617,7 +618,7 @@ https://github.com/GPII/universal/blob/master/LICENSE.txt
originalSettings: { "cross-hairs-clip": false, "cross-hairs-color": "red" },
runningOnLogin: true,
appliedSettings: { "cross-hairs-clip": false, "cross-hairs-color": "green" },
runningAfterLogin: undefined,
runningAfterLogin: true,
expectedLaunchHandlerCalls: 0
}, {
name: "running on login - 'update' with reference to 'stop', 'configure' and 'start' blocks",
Expand All @@ -637,7 +638,8 @@ https://github.com/GPII/universal/blob/master/LICENSE.txt
appliedSettings: { "cross-hairs-clip": false, "cross-hairs-color": "green" },
runningAfterLogin: true,
expectedLaunchHandlerCalls: 4 // we run start/stop cycle both on login and logout
}, {
}/*, Commented out for GPII-4468 - the role of "active" is unclear during matchmaking
{
name: "running on login - with active: false",
basePayload: gpii.tests.lifecycleManager.buildLifecycleInstructions(
"org.gnome.desktop.a11y.magnifier",
Expand Down Expand Up @@ -684,7 +686,9 @@ https://github.com/GPII/universal/blob/master/LICENSE.txt
appliedSettings: { "cross-hairs-clip": false, "cross-hairs-color": "green" },
runningAfterLogin: true,
expectedLaunchHandlerCalls: 4 // we run start/stop cycle both on login and logout
}];
}
*/
];


gpii.tests.lifecycleManager.updateTestDefs = [{
Expand Down Expand Up @@ -901,7 +905,7 @@ https://github.com/GPII/universal/blob/master/LICENSE.txt
"org.gnome.desktop.a11y.magnifier",
gpii.tests.lifecycleManager.standardLifecycle,
{ "update": [ "configure" ] },
gpii.tests.lifecycleManager.buildSettingsHandlersEntry({ "cross-hairs-clip": true, "cross-hairs-color": "red" })),
gpii.tests.lifecycleManager.buildSettingsHandlersEntry({ "cross-hairs-clip": true, "cross-hairs-color": "red" }, true)),
expectedOriginal: gpii.tests.lifecycleManager.buildLifecycleInstructions(
"org.gnome.desktop.a11y.magnifier",
gpii.tests.lifecycleManager.standardLifecycle,
Expand Down Expand Up @@ -952,7 +956,7 @@ https://github.com/GPII/universal/blob/master/LICENSE.txt
"org.gnome.desktop.a11y.magnifier",
gpii.tests.lifecycleManager.standardLifecycle,
{ "update": [ "configure" ] },
gpii.tests.lifecycleManager.buildSettingsHandlersEntry({ "cross-hairs-clip": true })),
gpii.tests.lifecycleManager.buildSettingsHandlersEntry({ "cross-hairs-clip": true }, true)),
expectedOriginal: gpii.tests.lifecycleManager.buildLifecycleInstructions(
"org.gnome.desktop.a11y.magnifier",
gpii.tests.lifecycleManager.standardLifecycle,
Expand Down Expand Up @@ -1126,7 +1130,7 @@ https://github.com/GPII/universal/blob/master/LICENSE.txt
var expectedOriginal = gpii.tests.lifecycleManager.extendLifecycleInstructions(test.basePayload,
gpii.tests.lifecycleManager.buildSettingsHandlersEntry(test.originalSettings, test.runningOnLogin), test.payloadExtras);
var loginPayload = gpii.tests.lifecycleManager.extendLifecycleInstructions(test.basePayload,
gpii.tests.lifecycleManager.buildSettingsHandlersEntry(test.loginSettings), test.payloadExtras);
gpii.tests.lifecycleManager.buildSettingsHandlersEntry(test.loginSettings, true), test.payloadExtras);
var expectedAfterLogin = gpii.tests.lifecycleManager.extendLifecycleInstructions(test.basePayload,
gpii.tests.lifecycleManager.buildSettingsHandlersEntry(test.loginSettings, test.runningAfterLogin), test.payloadExtras);
var updatePayload = gpii.tests.lifecycleManager.extendLifecycleInstructions(test.basePayload,
Expand Down Expand Up @@ -1170,7 +1174,7 @@ https://github.com/GPII/universal/blob/master/LICENSE.txt
runningOnLogin: test.runningOnLogin,
originalSettings: gpii.tests.lifecycleManager.createSettingsHandlerPayload("org.gnome.desktop.a11y.magnifier", test.originalSettings),
loginPayload: gpii.tests.lifecycleManager.extendLifecycleInstructions(test.basePayload,
gpii.tests.lifecycleManager.buildSettingsHandlersEntry(test.appliedSettings, undefined, test.removeLifecycleHandlerBlock)),
gpii.tests.lifecycleManager.buildSettingsHandlersEntry(test.appliedSettings, true, test.removeLifecycleHandlerBlock)),
expectedAfterLogin: gpii.tests.lifecycleManager.extendLifecycleInstructions(test.basePayload,
gpii.tests.lifecycleManager.buildSettingsHandlersEntry(test.appliedSettings, test.runningAfterLogin, test.removeLifecycleHandlerBlock)),
runningAfterLogin: test.runningAfterLogin,
Expand Down
29 changes: 28 additions & 1 deletion gpii/node_modules/matchMakerFramework/src/MatchMakerUtilities.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,12 @@ var fluid = fluid || require("infusion"),
});
};

// TODO: While investigating GPII-4468, it was found that this function is only invoked in the pathway between gpii.lifecycleManager.applyExistingPrefs
// invoked as part of the "update" cycle - probably reflecting the historical fact that the PSP (now abolished) was the only source of changes.
// This implies that this linkage between "/enabled" and "active" is only ever computed at this point rather than at any login (that is, at LifecycleManager
// "start") as it should be.
// Instead, the dominant route for this linkage is in fact gpii.transformer.transformOneLaunchHandler in Transformer.js - it may well be that this business
// of "active" can be axed in favour of the "running" special setting
/**
* Based on the /enabled terms from a user's preference set and the /enabled capabilities from the solution,
* this function returns what the 'active' value of the solution should be.
Expand Down Expand Up @@ -217,6 +223,24 @@ var fluid = fluid || require("infusion"),
return active;
};

// Function to hack GPII-4468 so that an application which only has an /enabled application-specific term will still get selected
gpii.matchMakerFramework.utils.flattenEnabled = function (preferences, segs) {
var togo = {};
fluid.each(preferences, function (value, key) {
if (typeof(key) === "string" && key.endsWith("/enabled") && segs.includes("applications")) {
key = key.substring(0, key.length - "/enabled".length);
}
if (fluid.isPrimitive(value)) {
togo[key] = value;
} else {
segs.push(key);
togo[key] = gpii.matchMakerFramework.utils.flattenEnabled(value, segs);
segs.pop();
}
});
return togo;
};

/**
* Function that takes a MM payload set as input that *includes* an entry of
* preferences in a hierarchical format, as well as a strategy for selecting solutions
Expand Down Expand Up @@ -245,7 +269,8 @@ var fluid = fluid || require("infusion"),
// add users explicit application priorities to the solrecs object
gpii.matchMakerFramework.utils.parsePriorities(prefsSet, tmpSolrecs);

var leaves = gpii.matchMakerFramework.utils.computeLeaves(prefsSet.preferences);
var preferences = gpii.matchMakerFramework.utils.flattenEnabled(prefsSet.preferences, []);
var leaves = gpii.matchMakerFramework.utils.computeLeaves(preferences);
var disposed = strategy(leaves, tmpSolrecs, data.solutionTypeMapping);

togo[prefsSetId] = {};
Expand Down Expand Up @@ -508,6 +533,8 @@ var fluid = fluid || require("infusion"),
* This feature is required by the PSP to allow updating e.g. a top-level common term and
* ensuring that the relevant applications are affected by it
*/
// TODO: This utility is mysteriously only called from gpii.lifecycleManager.applyExistingPrefs and so this linkage between /enabled preferences and solution.active
// is only applied during "update"
gpii.matchMakerFramework.utils.updateInferredConfiguration = function (preferences, inferredConfiguration, solutionsRegistryEntries) {
var inferred = fluid.copy(inferredConfiguration);
fluid.each(preferences.contexts, function (prefsSet, prefsSetName) {
Expand Down
8 changes: 8 additions & 0 deletions gpii/node_modules/testing/src/Mocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,14 @@ fluid.defaults("gpii.test.integration.mockActionHandlers.windows", {
currentLanguage: 0
}
}
},
"gpii.windows.spiSettingsHandler.updateCursors": {
mockFunc: "fluid.identity",
defaults: {
gradeNames: "fluid.function",
argumentMap: {
}
}
}
}
});
27 changes: 16 additions & 11 deletions gpii/node_modules/transformer/src/js/Transformer.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,25 +166,30 @@ fluid.registerNamespace("gpii.transformer");
/**
* Helper function for `gpii.transformer.configurationToSettings`. Used to modify the launch handler
* blocks to be in the format required (i.e. setting the "settings.running" to true/false based on the
* corresponding /enabled preference value). This modifies the block in place - it has already been copied by the
* corresponding /enabled preference value). This modifies the launchHandler block in place - it has already been copied by the
* the caller.
*
* @param {Object} launchHandler - The launchHandler information from the solutions registry to be transformed - MODIFIED IN PLACE.
* @param {Object} oneUserSolution - The user preferences related to this solution and launch handler.
* @return {Object} - The modified launchHandler object - ready to be passed to the lifecycle manager
*/
gpii.transformer.transformOneLaunchHandler = function (launchHandler, oneUserSolution) {
var settings = fluid.copy(oneUserSolution.settings);
var settings = oneUserSolution.settings;
// launchHandler's "settings.running" value is set based on these rules:
// 1. If /enabled preference was provided, set the value to /enabled preference value;
// 2. If /enabled preference was not provided, set the value to `true` to launch the app for handling "settingsHandler".
// launchHandler's "settings.running" is set in all cases to ensure at user key-in when the key-in action needs
// to be merged with the default snapshot for "reset all" (see function gpii.lifecycleManager.userLogonHandling.startLifecycle()
// and GPII-3434), the structure of lifecycleInstructions for both key-in action and default snapshot match each other
// so that the launchHandler's "settings.running" value from the key-in will override the one from the default snapshot.
fluid.each(settings, function (value, key) {
fluid.set(launchHandler, ["settings", "running"], key.endsWith("/enabled") ? value : true);
var shouldEnable = fluid.find(settings, function (value, key) {
if (key.endsWith("/enabled")) {
return value;
}
});
if (shouldEnable !== undefined) {
fluid.set(launchHandler, ["settings", "running"], shouldEnable);
}
return launchHandler;
};

Expand Down Expand Up @@ -217,19 +222,19 @@ fluid.registerNamespace("gpii.transformer");
return gpii.transformer.transformOneSettingsHandler(settingsHandler, oneUserSolution, solutionId);
});

// Remove settingsHandlers that have empty settings.
gpii.transformer.adjustSettingsHandlerRelatedData(oneSolution);
// Remove settingsHandlers path itself if it doesn't contain any settingsHandler.
if ($.isEmptyObject(oneSolution.settingsHandlers)) {
delete oneSolution.settingsHandlers;
}

// Handle /enabled preferences to launch or stop solutions
if (oneSolution.launchHandlers) {
oneSolution.launchHandlers = fluid.transform(oneSolution.launchHandlers, function (launchHandler) {
return gpii.transformer.transformOneLaunchHandler(launchHandler, oneUserSolution, solutionId);
});
}

// Remove settingsHandlers that have empty settings.
gpii.transformer.adjustSettingsHandlerRelatedData(oneSolution);
// Remove settingsHandlers path itself if it doesn't contain any settingsHandler.
if ($.isEmptyObject(oneSolution.settingsHandlers)) {
delete oneSolution.settingsHandlers;
}
oneSolution.active = oneUserSolution.active; // copy over active directive for later use
return oneSolution;
});
Expand Down
37 changes: 37 additions & 0 deletions testData/preferences/carla2.json5
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// For manual testing of one variant of GPII-4468 - that a prefs set which explicitly mentions /enabled: true will
// get started and stopped correctly
{
"flat": {
"name": "Carla 2",
"contexts": {
"gpii-default": {
"name": "Default preferences",
"preferences": {
"http://registry.gpii.net/applications/org.gnome.desktop.a11y.magnifier": {
"show-cross-hairs": true,
"lens-mode": false,
"mag-factor": 2,
"mouse-tracking": "proportional",
"screen-position": "right-half",
"scroll-at-edges": true
},
"http://registry.gpii.net/applications/com.microsoft.windows.magnifier/enabled": true,
"http://registry.gpii.net/applications/com.microsoft.windows.magnifier": {
"Magnification": 200,
"ZoomIncrement": 50,
"Invert": 0,
"FollowMouse": 1,
"FollowFocus": 1,
"FollowCaret": 1,
"MagnificationMode": 1
},
"http://registry.gpii.net/common/fontSize": 24,
"http://registry.gpii.net/common/magnification": 2.0,
"http://registry.gpii.net/common/tracking": ["mouse"],
"http://registry.gpii.net/common/invertColours": true,
"http://registry.gpii.net/common/tableOfContents": false
}
}
}
}
}
15 changes: 15 additions & 0 deletions testData/preferences/carla3.json5
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// For manual testing of one variant of GPII-4468 - that a prefs set which only mentions /enabled: true will
// get started and stopped correctly
{
"flat": {
"name": "Carla 3",
"contexts": {
"gpii-default": {
"name": "Default preferences",
"preferences": {
"http://registry.gpii.net/applications/com.microsoft.windows.magnifier/enabled": true
}
}
}
}
}
1 change: 1 addition & 0 deletions testData/preferences/explodeLaunchHandlerStart.json5
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"gpii-default": {
"name": "Default preferences",
"preferences": {
"http://registry.gpii.net/applications/net.gpii.explode/enabled": true,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this have to be expressed as a full URL rather than as a property under net.gpii.explode?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it does. I wish it didn't, it means all of the /enabled stuff is much more difficult to validate. It also means we have no good way of indicating which things can be enabled at all, i.e. we can pass along any solution's URL with /enabled at the end whether or not it's meaningful.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, we could investigate how much rework would be required to bundle this in as a standard setting. As I recall, we didn't want to pollute the existing namespace of settings in case that name was used by something else - but I guess transforms are sufficiently mature that these could easily be diverted. Historically, I think that "/enabled" was an initial attempt to clean up what had been pretty irregular URLs under the old "common terms" system such as /magnifierEnabled

Copy link
Member

@the-t-in-rtf the-t-in-rtf Jun 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's too much work to make this work like any other setting we should at least discuss whether/how we can clearly indicate which things can be /enabled and add detection. i.e. if what happens if someone tries to enable something that is always running and which cannot be stopped? What if they pass /enabled for a solution that has no launch handlers at all?

"http://registry.gpii.net/applications/net.gpii.explode": {
"explodeOn": "launch",
"explodeMethod": "reject"
Expand Down
1 change: 1 addition & 0 deletions testData/preferences/explodeLaunchHandlerStop.json5
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"gpii-default": {
"name": "Default preferences",
"preferences": {
"http://registry.gpii.net/applications/net.gpii.explode/enabled": true,
"http://registry.gpii.net/applications/net.gpii.explode": {
"explodeOn": "stop",
"explodeMethod": "reject"
Expand Down
6 changes: 3 additions & 3 deletions testData/solutions/win32.json5
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
}
]
},
"capabilities": [
"http://registry\\.gpii\\.net/common/magnification/enabled"
],
"settingsHandlers": {
"configuration": {
"type": "gpii.settingsHandlers.INISettingsHandler",
Expand Down Expand Up @@ -6219,9 +6222,6 @@
}
}
},
"capabilities": [
"http://registry\\.gpii\\.net/common/magnification"
],
"capabilitiesTransformations": {
"Preferences\\.PromptOnExit": {
"literalValue": 0
Expand Down
2 changes: 1 addition & 1 deletion tests/all-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ var testIncludes = [
"./DeviceReporterErrorTests.js",
"./ErrorTests.js",
"./IntegrationTests.js",
"./JournalIntegrationTests.js",
// "./JournalIntegrationTests.js",
"./MultiSettingsHandlerTests.js",
"./PayloadSizeTest.js",
"./PSPIntegrationTests.js",
Expand Down
1 change: 1 addition & 0 deletions tests/data/preferences/jaws_application.json5
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"gpii-default": {
"name": "Default preferences",
"preferences": {
"http://registry.gpii.net/applications/com.freedomscientific.jaws/enabled": true,
"http://registry.gpii.net/applications/com.freedomscientific.jaws": {
"Voice Profiles.ActiveVoiceProfileName" : "GPII",
"Options.SayAllIndicateCaps" : 0,
Expand Down
Loading