Skip to content

Commit 385ee8e

Browse files
committed
Help dialog: remove variable shadowing between content script and background page
This was unnecessarily confusing and depended on the order in which scripts were imported.
1 parent b90e635 commit 385ee8e

File tree

2 files changed

+37
-39
lines changed

2 files changed

+37
-39
lines changed

content_scripts/vimium_frontend.js

Lines changed: 37 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -431,37 +431,43 @@ const checkIfEnabledForUrl = async () => {
431431
if (!isEnabledForUrl) HUD.hide(true, false);
432432
};
433433

434-
// If we are in the help dialog iframe, then HelpDialog is already defined with the necessary
435-
// functions.
436-
if (globalThis.HelpDialog == null) {
437-
globalThis.HelpDialog = {
438-
helpUI: null,
439-
isShowing() {
440-
return this.helpUI && this.helpUI.showing;
441-
},
442-
abort() {
443-
if (this.isShowing()) {
444-
return this.helpUI.hide(false);
445-
}
446-
},
434+
// If this content script is running in the help dialog's iframe, then use the HelpDialogPage's
435+
// methods to control the dialog. Otherwise, load the help dialog in a UIComponent iframe.
436+
const HelpDialog = {
437+
helpUI: null,
438+
439+
isShowing() {
440+
if (globalThis.isVimiumHelpDialogPage) return true;
441+
return this.helpUI && this.helpUI.showing;
442+
},
447443

448-
async toggle(request) {
449-
if (this.helpUI == null) {
450-
await DomUtils.documentComplete();
451-
this.helpUI = new UIComponent();
452-
this.helpUI.load("pages/help_dialog_page.html", "vimium-help-dialog-frame");
453-
}
454-
if (this.isShowing()) {
455-
this.helpUI.hide();
456-
} else {
457-
return this.helpUI.show(
458-
{ name: "activate" },
459-
{ focus: true, sourceFrameId: request.sourceFrameId },
460-
);
461-
}
462-
},
463-
};
464-
}
444+
abort() {
445+
if (globalThis.isVimiumHelpDialogPage) throw new Error("This should be impossible.");
446+
if (this.isShowing()) {
447+
return this.helpUI.hide(false);
448+
}
449+
},
450+
451+
async toggle(request) {
452+
// If we're in the help dialog page already and the user has typed a key to show the help
453+
// dialog, then we should hide it.
454+
if (globalThis.isVimiumHelpDialogPage) return HelpDialogPage.hide();
455+
456+
if (this.helpUI == null) {
457+
await DomUtils.documentComplete();
458+
this.helpUI = new UIComponent();
459+
this.helpUI.load("pages/help_dialog_page.html", "vimium-help-dialog-frame");
460+
}
461+
if (this.isShowing()) {
462+
this.helpUI.hide();
463+
} else {
464+
return this.helpUI.show(
465+
{ name: "activate" },
466+
{ focus: true, sourceFrameId: request.sourceFrameId },
467+
);
468+
}
469+
},
470+
};
465471

466472
const testEnv = globalThis.window == null;
467473
if (!testEnv) {
@@ -471,6 +477,7 @@ if (!testEnv) {
471477
}
472478

473479
Object.assign(globalThis, {
480+
HelpDialog,
474481
handlerStack,
475482
windowIsFocused,
476483
// These are exported for normal mode and link-hints mode.

pages/help_dialog_page.js

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,8 @@ function isAdvancedCommand(command, options) {
2929
(command.name == "reload" && options.includes("hard"));
3030
}
3131

32-
// This overrides the HelpDialogPage implementation in vimium_frontend.js. We provide aliases for
33-
// the two HelpDialogPage methods required by normalMode (isShowing() and toggle()).
3432
const HelpDialogPage = {
3533
dialogElement: null,
36-
isShowing() {
37-
return true;
38-
},
3934

4035
// This setting is pulled out of local storage. It's false by default.
4136
getShowAdvancedCommands() {
@@ -154,10 +149,6 @@ const HelpDialogPage = {
154149
UIComponentMessenger.postMessage({ name: "hide" });
155150
},
156151

157-
toggle() {
158-
this.hide();
159-
},
160-
161152
//
162153
// Advanced commands are hidden by default so they don't overwhelm new and casual users.
163154
//

0 commit comments

Comments
 (0)