Skip to content

Commit

Permalink
killEditor: don't focus input if editor isn't focused
Browse files Browse the repository at this point in the history
This change required vast changes in multiple parts of Firenvim.

First, in FirenvimElement.focus(): there was an issue where, due to
setTimeout callbacks, the firenvim frame could get refocused when its
focus was released after a focus() call. This is fixed by saving timeout
ids and event listeners and clearing them when releasing focus.

FirenvimElement.pressKeys() and FirenvimElement.setPageElementCursor()
now check focus status before messing with focus.

src/nvimproc/Neovim.ts now remembers whether focus was relinquinsed by
focus_page between the beginning of a firenvim_bufwrite event and its
end.
  • Loading branch information
glacambre committed Jan 5, 2021
1 parent 70a3175 commit 934f772
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 11 deletions.
53 changes: 47 additions & 6 deletions src/FirenvimElement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@ export class FirenvimElement {
// underlying elements (be they simple textareas, CodeMirror elements or
// other).
private editor: AbstractEditor;
// focusInfo is used to keep track of focus listeners and timeouts created
// by FirenvimElement.focus(). FirenvimElement.focus() creates these
// listeners and timeouts in order to work around pages that try to grab
// the focus again after the FirenvimElement has been created or after the
// underlying element's content has changed.
private focusInfo = {
finalRefocusTimeouts: [] as any[],
refocusRefs: [] as any[],
refocusTimeouts: [] as any[],
};
// frameId is the webextension id of the neovim frame. We use it to send
// commands to the frame.
private frameId: number;
Expand Down Expand Up @@ -230,6 +240,29 @@ export class FirenvimElement {
});
}

clearFocusListeners () {
// When the user tries to `:w | call firenvim#focus_page()` in Neovim,
// we have a problem. `:w` results in a call to setPageElementContent,
// which calls FirenvimElement.focus(), because some pages try to grab
// focus when the content of the underlying element changes.
// FirenvimElement.focus() creates event listeners and timeouts to
// detect when the page tries to grab focus and bring it back to the
// FirenvimElement. But since `call firenvim#focus_page()` happens
// right after the `:w`, focus_page() triggers the event
// listeners/timeouts created by FirenvimElement.focus()!
// So we need a way to clear the timeouts and event listeners before
// performing focus_page, and that's what this function does.
this.focusInfo.finalRefocusTimeouts.forEach(t => clearTimeout(t));
this.focusInfo.refocusTimeouts.forEach(t => clearTimeout(t));
this.focusInfo.refocusRefs.forEach(f => {
this.iframe.removeEventListener("blur", f);
this.getElement().removeEventListener("focus", f);
});
this.focusInfo.finalRefocusTimeouts.length = 0;
this.focusInfo.refocusTimeouts.length = 0;
this.focusInfo.refocusRefs.length = 0;
}

detachFromPage () {
const elem = this.getElement();
this.resizeObserver.unobserve(elem);
Expand All @@ -247,7 +280,7 @@ export class FirenvimElement {
// actually stop refocusing the iframe a second after it is created.
const self = this;
function refocus() {
setTimeout(() => {
self.focusInfo.refocusTimeouts.push(setTimeout(() => {
// First, destroy current selection. Some websites use the
// selection to force-focus an element.
const sel = document.getSelection();
Expand All @@ -264,15 +297,16 @@ export class FirenvimElement {
document.body.focus();
}
self.iframe.focus();
}, 0);
}, 0));
}
self.focusInfo.refocusRefs.push(refocus);
this.iframe.addEventListener("blur", refocus);
this.getElement().addEventListener("focus", refocus);
setTimeout(() => {
self.focusInfo.finalRefocusTimeouts.push(setTimeout(() => {
refocus();
this.iframe.removeEventListener("blur", refocus);
this.getElement().removeEventListener("focus", refocus);
}, 100);
}, 100));
refocus();
}

Expand Down Expand Up @@ -336,8 +370,11 @@ export class FirenvimElement {
}

pressKeys (keys: KeyboardEvent[]) {
const focused = this.isFocused();
keys.forEach(ev => this.originalElement.dispatchEvent(ev));
this.focus();
if (focused) {
this.focus();
}
}

putEditorCloseToInputOrigin () {
Expand Down Expand Up @@ -466,7 +503,11 @@ export class FirenvimElement {
}

setPageElementCursor (line: number, column: number) {
return this.editor.setCursor(line, column);
let p = Promise.resolve();
if (this.isFocused()) {
p = this.editor.setCursor(line, column);
}
return p;
}

show () {
Expand Down
2 changes: 1 addition & 1 deletion src/NeovimFrame.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,8 @@ export const isReady = new Promise((resolve, reject) => {
getGridId(),
Math.floor(evt.pageY / cHeight),
Math.floor(evt.pageX / cWidth));

keyHandler.focus();
setTimeout(() => keyHandler.focus(), 10);
}
window.addEventListener("mousedown", e => {
onMouse(e, "press");
Expand Down
24 changes: 22 additions & 2 deletions src/nvimproc/Neovim.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,19 +50,35 @@ export async function neovim(
}
}
});

// hasFocus is used to keep track of whether the neovim frame has focus.
// This information is used in firenvim_bufwrite, to know whether the
// neovim frame should be focused again after a write.
// Note that it is important to have hasFocus live outside of the callback.
// The reason for that is the following sequence of events:
// - firenvim_bufwrite is received, hasFocus is set to true,
// page.setElementContent is called asynchronously
// - firenvim_focus_page is called, page.focusPage() is called
// asynchronously, hasFocus is set to false
// - page.setElementContent completes, hasFocus is checked to see if focus
// should be grabbed or not
// If the state of hasFocus was lost between messages, we wouldn't know
// that firenvim_focus_page has been called and thus erroneously re-focus
// the frame.
let hasFocus: boolean;
stdout.addListener("notification", async (name: string, args: any[]) => {
hasFocus = document.hasFocus();
switch (name) {
case "redraw":
if (args) {
onRedraw(functions, args, element, extCmdline, extMessages);
}
break;
case "firenvim_bufwrite":
const hasFocus = document.hasFocus();
const data = args[0] as { text: string[], cursor: [number, number] };
page.setElementContent(data.text.join("\n"))
.then(() => page.setElementCursor(...(data.cursor)))
.then(() => { if (hasFocus && !document.hasFocus()) { window.focus(); } });
.then(() => { if (hasFocus && !document.hasFocus()) window.focus(); });
break;
case "firenvim_eval_js":
const result = await page.evalInPage(args[0]);
Expand All @@ -72,18 +88,22 @@ export async function neovim(
break;
case "firenvim_focus_page":
page.focusPage();
hasFocus = false;
break;
case "firenvim_focus_input":
page.focusInput();
hasFocus = false;
break;
case "firenvim_hide_frame":
page.hideEditor();
hasFocus = false;
break;
case "firenvim_press_keys":
page.pressKeys(args[0]);
break;
case "firenvim_vimleave":
page.killEditor();
hasFocus = false;
break;
}
});
Expand Down
9 changes: 7 additions & 2 deletions src/page/functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,9 @@ export function getNeovimFrameFunctions(global: IGlobalState) {
}
_focusInput(global, firenvimElement, true);
},
focusPage: () => {
focusPage: (frameId: number) => {
const firenvimElement = global.firenvimElems.get(frameId);
firenvimElement.clearFocusListeners();
(document.activeElement as any).blur();
document.documentElement.focus();
},
Expand All @@ -113,9 +115,12 @@ export function getNeovimFrameFunctions(global: IGlobalState) {
},
killEditor: (frameId: number) => {
const firenvim = global.firenvimElems.get(frameId);
const isFocused = firenvim.isFocused();
firenvim.detachFromPage();
const conf = getConf();
_focusInput(global, firenvim, conf.takeover !== "once");
if (isFocused) {
_focusInput(global, firenvim, conf.takeover !== "once");
}
global.firenvimElems.delete(frameId);
},
pressKeys: (frameId: number, keys: string[]) => {
Expand Down

0 comments on commit 934f772

Please sign in to comment.