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
64 changes: 36 additions & 28 deletions addon/implementation.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,6 @@ const WINDOW_TYPES = {
};

// Function called by Mousetrap to test if it should stop processing a key event
//
// This function is based on the default callback in Mousetrap but is extended
// to include more text input fields that are specific to Thunderbird.
// Additionally, it does not ignore text fields if the first key includes
// modifiers other than shift.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I don't think this comment needed to be removed?

function stopCallback(e, element, combo, seq) {
let tagName = element.tagName.toLowerCase();
// Uncomment the following line to debug why tbkeys is triggering in an input
Expand Down Expand Up @@ -98,36 +93,49 @@ function stopCallback(e, element, combo, seq) {
//
// win is the window in which the command should be executed
//
// command should be a string formatted as type:body where type is cmd, func,
// tbkeys, unset, or eval and body is the type-specific content of the command
// command should be a string formatted as type:body OR an array of strings.
// Modified to support multiple commands in an array.
Comment on lines +96 to +97

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'd like to keep the description of the string formatting (line wrapping might not be right for this suggestion):

Suggested change
// command should be a string formatted as type:body OR an array of strings.
// Modified to support multiple commands in an array.
// command should be a string or arrays of strings with
// each string formatted as type:body where type is cmd, func,
// tbkeys, unset, or eval and body is the type-specific content of the command

function buildKeyCommand(win, command) {
let callback = function () {
// window is defined here so that it is available for use with eval() in
// the non-lite version of tbkeys
// eslint-disable-next-line no-unused-vars
let window = win;

let cmdType = command.split(":", 1)[0];
let cmdBody = command.slice(cmdType.length + 1);
switch (cmdType) {
case "cmd":
win.goDoCommand(cmdBody);
break;
case "func":
win[cmdBody]();
break;
case "tbkeys":
builtins[cmdBody](win);
break;
case "memsg":
Services.obs.notifyObservers(null, "tbkeys-memsg", cmdBody);
break;
case "unset":
break;
default:
eval(command);
break;
// Internal helper to execute a single command string
const runSingleCommand = (cmdStr) => {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can we pull this definition outside of buildKeyCommand?

let cmdType = cmdStr.split(":", 1)[0];
let cmdBody = cmdStr.slice(cmdType.length + 1);
switch (cmdType) {
case "cmd":
win.goDoCommand(cmdBody);
break;
case "func":
win[cmdBody]();
break;
case "tbkeys":
builtins[cmdBody](win);
break;
case "memsg":
Services.obs.notifyObservers(null, "tbkeys-memsg", cmdBody);
break;
case "unset":
break;
default:
eval(cmdStr);
break;
}
};

// If command is an array, iterate and execute each. Otherwise, execute once.
if (Array.isArray(command)) {
for (let singleCmd of command) {
runSingleCommand(singleCmd);
}
} else {
runSingleCommand(command);
}

return false;
};

Expand Down Expand Up @@ -191,7 +199,7 @@ var TBKeys = {
// Set all keybindings for all windows
//
// keyBindings has the structure:
// {windowType: {keySequence: keyCommand}}
// {windowType: {keySequence: keyCommand}}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Maybe undo this change unless there was a reason for it?

// keyBindings should have all WINDOW_TYPES values
bindKeys: function (keyBindings) {
this.init();
Expand Down
2 changes: 1 addition & 1 deletion addon/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
"id": "Keys",
"type": "object",
"description": "Mapping of key sequences to commands",
"additionalProperties": { "type": "string" }
"additionalProperties": { "type": "any" }

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

We are going from allowing strings to allow arrays of strings. Can we keep the schema more restricted to those cases instead of using any?

Suggested change
"additionalProperties": { "type": "any" }
"additionalProperties": { "type": ["array", "string"], "items": {"type": "string"} }

}
]
}
Expand Down