From c758e96693b473939e8e5047fbb3c62818a9841d Mon Sep 17 00:00:00 2001 From: Geoff Lankow Date: Thu, 19 Jan 2023 12:18:07 +1300 Subject: [PATCH] Bug 1811139 - Fix accelerator key for tree view selection on MacOS. r=aleca Differential Revision: https://phabricator.services.mozilla.com/D167212 --HG-- extra : amend_source : ed7f31a4077e0822eff41c9644d5c41097fe93e6 --- .../content/widgets/tree-view-listbox.mjs | 20 +++++++--- .../test/browser/browser_treeViewListbox.js | 39 ++++++++++--------- 2 files changed, 34 insertions(+), 25 deletions(-) diff --git a/mail/base/content/widgets/tree-view-listbox.mjs b/mail/base/content/widgets/tree-view-listbox.mjs index dc76209ff1..187e31c321 100644 --- a/mail/base/content/widgets/tree-view-listbox.mjs +++ b/mail/base/content/widgets/tree-view-listbox.mjs @@ -2,10 +2,18 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, you can obtain one at http://mozilla.org/MPL/2.0/. */ +const { AppConstants } = ChromeUtils.importESModule( + "resource://gre/modules/AppConstants.sys.mjs" +); const { JSTreeSelection } = ChromeUtils.import( "resource:///modules/JsTreeSelection.jsm" ); +// Account for the mac OS accelerator key variation. +// Use these strings to check keyboard event properties. +const accelKeyName = AppConstants.platform == "macosx" ? "metaKey" : "ctrlKey"; +const otherKeyName = AppConstants.platform == "macosx" ? "ctrlKey" : "metaKey"; + // Animation variables for expanding and collapsing child lists. let reducedMotionMedia = matchMedia("(prefers-reduced-motion)"); @@ -711,7 +719,7 @@ class TreeViewListbox extends HTMLTableSectionElement { return; } - if (event.ctrlKey && event.shiftKey) { + if (event[accelKeyName] && event.shiftKey) { return; } @@ -788,7 +796,7 @@ class TreeViewListbox extends HTMLTableSectionElement { return; } - if (event.ctrlKey) { + if (event[accelKeyName]) { this._toggleSelected(index); } else if (event.shiftKey) { this._selectRange(index); @@ -798,7 +806,7 @@ class TreeViewListbox extends HTMLTableSectionElement { }); this.addEventListener("keydown", event => { - if (event.altKey || event.metaKey) { + if (event.altKey || event[otherKeyName]) { return; } @@ -883,11 +891,11 @@ class TreeViewListbox extends HTMLTableSectionElement { if (newIndex != undefined) { newIndex = this._clampIndex(newIndex); - if (newIndex != null && (!event.ctrlKey || !event.shiftKey)) { + if (newIndex != null && (!event[accelKeyName] || !event.shiftKey)) { // Else, if both modifiers pressed, do nothing. if (event.shiftKey) { this._selectRange(newIndex); - } else if (event.ctrlKey) { + } else if (event[accelKeyName]) { // Change focus, but not selection. this.currentIndex = newIndex; } else { @@ -900,7 +908,7 @@ class TreeViewListbox extends HTMLTableSectionElement { if (event.key == " ") { if (this.currentIndex != -1 && !event.shiftKey) { - if (event.ctrlKey) { + if (event[accelKeyName]) { this._toggleSelected(this.currentIndex); } else { this._selectSingle(this.currentIndex); diff --git a/mail/base/test/browser/browser_treeViewListbox.js b/mail/base/test/browser/browser_treeViewListbox.js index 1be69d506e..8ff873ce53 100644 --- a/mail/base/test/browser/browser_treeViewListbox.js +++ b/mail/base/test/browser/browser_treeViewListbox.js @@ -163,8 +163,7 @@ async function subtestKeyboardAndMouse() { async function clickOnRow(index, modifiers = {}, expectEvent = true) { if (modifiers.shiftKey) { info(`clicking on row ${index} with shift key`); - } else if (modifiers.ctrlKey) { - // XXX: On macOS instead this should use the metaKey? + } else if (modifiers.accelKey) { info(`clicking on row ${index} with ctrl key`); } else { info(`clicking on row ${index}`); @@ -214,23 +213,23 @@ async function subtestKeyboardAndMouse() { // Select multiple rows by ctrl-clicking. - await clickOnRow(5, { ctrlKey: true }); + await clickOnRow(5, { accelKey: true }); checkCurrent(5); checkSelected(2, 5); - await clickOnRow(1, { ctrlKey: true }); + await clickOnRow(1, { accelKey: true }); checkCurrent(1); checkSelected(1, 2, 5); - await clickOnRow(5, { ctrlKey: true }); + await clickOnRow(5, { accelKey: true }); checkCurrent(5); checkSelected(1, 2); - await clickOnRow(1, { ctrlKey: true }); + await clickOnRow(1, { accelKey: true }); checkCurrent(1); checkSelected(2); - await clickOnRow(2, { ctrlKey: true }); + await clickOnRow(2, { accelKey: true }); checkCurrent(2); checkSelected(); @@ -239,6 +238,8 @@ async function subtestKeyboardAndMouse() { async function pressKey(key, modifiers = {}, expectEvent = true) { if (modifiers.shiftKey) { info(`pressing ${key} with shift key`); + } else if (modifiers.accelKey) { + info(`pressing ${key} with accel key`); } else { info(`pressing ${key}`); } @@ -260,7 +261,7 @@ async function subtestKeyboardAndMouse() { checkCurrent(1); checkSelected(1); - await pressKey("VK_UP", { ctrlKey: true }, false); + await pressKey("VK_UP", { accelKey: true }, false); checkCurrent(0); checkSelected(1); @@ -274,28 +275,28 @@ async function subtestKeyboardAndMouse() { checkCurrent(0); checkSelected(0); - await pressKey("VK_DOWN", { ctrlKey: true }, false); + await pressKey("VK_DOWN", { accelKey: true }, false); checkCurrent(1); checkSelected(0); - await pressKey("VK_DOWN", { ctrlKey: true }, false); + await pressKey("VK_DOWN", { accelKey: true }, false); checkCurrent(2); checkSelected(0); // Multi select with Ctrl+Space. - await pressKey(" ", { ctrlKey: true }); + await pressKey(" ", { accelKey: true }); checkCurrent(2); checkSelected(0, 2); - await pressKey("VK_DOWN", { ctrlKey: true }, false); + await pressKey("VK_DOWN", { accelKey: true }, false); checkCurrent(3); checkSelected(0, 2); - await pressKey("VK_DOWN", { ctrlKey: true }, false); + await pressKey("VK_DOWN", { accelKey: true }, false); checkCurrent(4); checkSelected(0, 2); - await pressKey(" ", { ctrlKey: true }); + await pressKey(" ", { accelKey: true }); checkCurrent(4); checkSelected(0, 2, 4); @@ -305,7 +306,7 @@ async function subtestKeyboardAndMouse() { checkSelected(3); // Can select none using Ctrl+Space. - await pressKey(" ", { ctrlKey: true }); + await pressKey(" ", { accelKey: true }); checkCurrent(3); checkSelected(); @@ -313,7 +314,7 @@ async function subtestKeyboardAndMouse() { checkCurrent(4); checkSelected(4); - await pressKey("VK_HOME", { ctrlKey: true }, false); + await pressKey("VK_HOME", { accelKey: true }, false); checkCurrent(0); checkSelected(4); @@ -341,16 +342,16 @@ async function subtestKeyboardAndMouse() { checkCurrent(1); checkSelected(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12); - await pressKey("VK_DOWN", { ctrlKey: true }, false); + await pressKey("VK_DOWN", { accelKey: true }, false); checkCurrent(2); checkSelected(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12); - await pressKey("VK_DOWN", { ctrlKey: true }, false); + await pressKey("VK_DOWN", { accelKey: true }, false); checkCurrent(3); checkSelected(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12); // Break the shift sequence by Ctrl+Space. - await pressKey(" ", { ctrlKey: true }); + await pressKey(" ", { accelKey: true }); checkCurrent(3); checkSelected(1, 2, 4, 5, 6, 7, 8, 9, 10, 11, 12);