diff --git a/ash/webui/shortcut_customization_ui/resources/accelerator_lookup_manager.js b/ash/webui/shortcut_customization_ui/resources/accelerator_lookup_manager.js index 029b5b33637cc9..e28f7fb1baf74a 100644 --- a/ash/webui/shortcut_customization_ui/resources/accelerator_lookup_manager.js +++ b/ash/webui/shortcut_customization_ui/resources/accelerator_lookup_manager.js @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import {assertNotReached} from 'chrome://resources/js/assert.m.js'; +import {assert, assertNotReached} from 'chrome://resources/js/assert.m.js'; import {addSingletonGetter} from 'chrome://resources/js/cr.m.js'; import {fakeActionNames} from './fake_data.js'; @@ -205,29 +205,39 @@ export class AcceleratorLookupManager { /** * @param {!AcceleratorSource} source * @param {number} action - * @param {AcceleratorKeys} accelerator + * @param {AcceleratorKeys} keys */ - removeAccelerator(source, action, accelerator) { - const foundIdx = this.getAcceleratorInfoIndex_(source, action, accelerator); + removeAccelerator(source, action, keys) { + const foundAccel = this.getAcceleratorInfoFromKeys_(source, action, keys); - if (foundIdx === -1) { - // Can only attempt to remove an existing accelerator. - assertNotReached(); - } - - const accelInfos = this.getAccelerators(source, action); - const foundAccel = accelInfos[foundIdx]; + // Can only remove an existing accelerator. + assert(foundAccel != null); if (foundAccel.locked) { // Not possible to remove a locked accelerator manually. assertNotReached(); } + const accelInfos = this.getAccelerators(source, action); + const foundIdx = this.getAcceleratorInfoIndex_(source, action, keys); // Remove accelerator from main map. accelInfos.splice(foundIdx, 1); // Remove from reverse lookup. - this.reverseAcceleratorLookup_.delete(JSON.stringify(accelerator)); + this.reverseAcceleratorLookup_.delete(JSON.stringify(keys)); + } + + /** + * @param {!AcceleratorSource} source + * @param {number} action + * @param {!AcceleratorKeys} keys + * @return {boolean} + */ + isAcceleratorLocked(source, action, keys) { + const accel = this.getAcceleratorInfoFromKeys_(source, action, keys); + assert(accel); + + return accel.locked; } /** @@ -249,11 +259,8 @@ export class AcceleratorLookupManager { const accelInfos = this.getAccelerators(source, action); const foundIdx = this.getAcceleratorInfoIndex_(source, action, accelKeys); - // Check if the accelerator is locked, disable if locked. - if (accelInfos[foundIdx].locked) { - accelInfos[foundIdx].state = AcceleratorState.kDisabledByUser; - return; - } + // Cannot remove a locked accelerator. + assert(!accelInfos[foundIdx].locked); // Otherwise, remove the accelerator. accelInfos.splice(foundIdx, 1); @@ -281,6 +288,23 @@ export class AcceleratorLookupManager { return -1; } + /** + * @param {!AcceleratorSource} source + * @param {number} action + * @param {!AcceleratorKeys} keys + * @return {?AcceleratorInfo} + */ + getAcceleratorInfoFromKeys_(source, action, keys) { + const foundIdx = this.getAcceleratorInfoIndex_(source, action, keys); + + if (foundIdx === -1) { + return null; + } + + const accelInfos = this.getAccelerators(source, action); + return accelInfos[foundIdx]; + } + reset() { this.acceleratorLookup_.clear(); this.acceleratorNameLookup_.clear(); @@ -290,4 +314,3 @@ export class AcceleratorLookupManager { } addSingletonGetter(AcceleratorLookupManager); -; \ No newline at end of file diff --git a/ash/webui/shortcut_customization_ui/resources/accelerator_view.js b/ash/webui/shortcut_customization_ui/resources/accelerator_view.js index 5c1d70597acfb0..0d57ca64f0fb4c 100644 --- a/ash/webui/shortcut_customization_ui/resources/accelerator_view.js +++ b/ash/webui/shortcut_customization_ui/resources/accelerator_view.js @@ -306,11 +306,24 @@ export class AcceleratorViewElement extends PolymerElement { if (foundId !== undefined) { // TODO(jimmyxgong): Fetch name of accelerator with real implementation. const uuidParams = foundId.split('-'); + const conflictSource = + /** @type {!AcceleratorSource} */(parseInt(uuidParams[0], 10)); + const conflictAction = parseInt(uuidParams[1], 10); const conflictAccelName = this.lookupManager_.getAcceleratorName( - /**source=*/ parseInt(uuidParams[0], 10), - /**action=*/ parseInt(uuidParams[1], 10)); + conflictSource, conflictAction); + + // Cannot override a locked action. + if (!this.shortcutProvider_.isMutable(conflictSource) || + this.lookupManager_.isAcceleratorLocked( + conflictSource, conflictAction, pendingKeys)) { + // TODO(jimmyxgong): i18n this string. + this.statusMessage = 'Shortcut is used by \"' + conflictAccelName + + '\". Press a new shortcut to replace.'; + this.hasError = true; + return; + } + // TODO(jimmyxgong): i18n this string. - // TODO(jimmyxgong): Handle attempting to override a locked action. this.statusMessage = 'Shortcut is used by ' + conflictAccelName + '. Press a new shortcut or press the same one again to use it for ' + 'this action instead.'; diff --git a/chrome/test/data/webui/chromeos/shortcut_customization/accelerator_lookup_manager_test.js b/chrome/test/data/webui/chromeos/shortcut_customization/accelerator_lookup_manager_test.js index 9365a072518616..0d6ba2b356fb26 100644 --- a/chrome/test/data/webui/chromeos/shortcut_customization/accelerator_lookup_manager_test.js +++ b/chrome/test/data/webui/chromeos/shortcut_customization/accelerator_lookup_manager_test.js @@ -167,48 +167,6 @@ export function acceleratorLookupManagerTest() { }); }); - test('ReplacePreexistingLockedAccelerator', () => { - provider.setFakeAcceleratorConfig(fakeAcceleratorConfig); - return provider.getAllAcceleratorConfig().then((result) => { - assertDeepEquals(fakeAcceleratorConfig, result); - - manager.setAcceleratorLookup(result); - - // Get Snap Window Left accelerator, which has a locked accelerator. - const snapWindowLeftAction = 0; - const ashMap = fakeAcceleratorConfig.get(AcceleratorSource.kAsh); - const snapWindowLeftAccels = ashMap.get(snapWindowLeftAction); - // Modifier.Alt + key::219 ('[') - const overridenAccel = snapWindowLeftAccels[0].accelerator; - // Verify state is kEnabled. - assertEquals(AcceleratorState.kEnabled, snapWindowLeftAccels[0].state); - - // Replace New Desk shortcut with Alt+'['. - const newDeskAction = 2; - const oldNewDeskAccel = ashMap.get(newDeskAction)[0].accelerator; - - replaceAndVerify( - AcceleratorSource.kAsh, newDeskAction, oldNewDeskAccel, - overridenAccel); - - // Verify that the New Desk shortcut now has the ALT + '[' accelerator. - const newDeskLookup = - manager.getAccelerators(AcceleratorSource.kAsh, newDeskAction); - assertEquals(1, newDeskLookup.length); - assertEquals( - JSON.stringify(overridenAccel), - JSON.stringify(newDeskLookup[0].accelerator)); - - // Verify that Snap Window Left's accelerator is not removed (since it is, - // locked). But it's state is updated to kDisabledByUser. - const snapWindowLeftLookup = - manager.getAccelerators(AcceleratorSource.kAsh, snapWindowLeftAction); - assertEquals(1, snapWindowLeftLookup.length); - assertEquals( - AcceleratorState.kDisabledByUser, snapWindowLeftLookup[0].state); - }); - }); - test('AddBasicAccelerator', () => { provider.setFakeAcceleratorConfig(fakeAcceleratorConfig); return provider.getAllAcceleratorConfig().then((result) => {