Fix hint bug.

In hinting.ts, pipe() and pipe_elements() assumed that the resolve
function they passed to hintPage() would always be called, which wasn't
always the case (e.g. when a users goes into hint mode but presses
`<Esc>`).

This caused unresolved promises to linger in the tab. When the tab was
closed, an error was thrown about the message manager being
disconnected. This was caught by Tridactyl and displayed in the command
line.

We're fixing this bug by passing no-op functions as onSelect to
hintPage() and explicitly passing the resolve function. The resolve
function is then saved in HintState and called when destroying
HintState.

We parametrize reset() in order to be able to distinguish between resets
caused by a hint being selected and by the user pressing `<Esc>`. This
is necessary because we need to know when the function should resolve
the last focused hint and when it shouldn't.

We then add a bunch of null handling in excmds.ts:hint() in order to
make sure not to introduce other bugs.

This fixes https://github.com/cmcaine/tridactyl/issues/855.
This commit is contained in:
glacambre 2018-08-01 20:41:01 +02:00
parent 083abf029e
commit 2f6bd1726e
No known key found for this signature in database
GPG key ID: B9625DB1767553AC
3 changed files with 59 additions and 24 deletions

View file

@ -2903,7 +2903,9 @@ export async function hint(option?: string, selectors?: string, ...rest: string[
// Open in background
if (option === "-b") {
let [link, hintCount] = await hinting.pipe(DOM.HINTTAGS_selectors)
let result = await hinting.pipe(DOM.HINTTAGS_selectors)
if (result === null) return null
let [link, hintCount] = result
link.focus()
if (link.href) {
let containerId = await activeTabContainerId()
@ -2927,17 +2929,22 @@ export async function hint(option?: string, selectors?: string, ...rest: string[
// Yank link
else if (option === "-y") {
run_exstr("yank " + (await hinting.pipe(DOM.HINTTAGS_selectors))[0]["href"])
let result = await hinting.pipe(DOM.HINTTAGS_selectors)
if (result === null) return null
run_exstr("yank " + result[0]["href"])
}
// Yank text content
else if (option === "-p") {
run_exstr("yank " + (await hinting.pipe_elements(DOM.elementsWithText()))["textContent"])
let result = await hinting.pipe_elements(DOM.elementsWithText())
if (result === null) return null
run_exstr("yank " + result["textContent"])
}
// Yank link alt text
else if (option === "-P") {
let link = await hinting.pipe_elements(DOM.getElemsBySelector("[title], [alt]", [DOM.isVisible]))
if (link === null) return link
run_exstr("yank " + (link.title ? link.title : link.alt))
}
@ -2945,15 +2952,28 @@ export async function hint(option?: string, selectors?: string, ...rest: string[
else if (option === "-#") {
let anchorUrl = new URL(window.location.href)
let link = await hinting.pipe_elements(DOM.anchors())
if (link === null) return null
anchorUrl.hash = link.id || link.name
run_exstr("yank " + anchorUrl.href)
} else if (option === "-c") DOM.simulateClick((await hinting.pipe(selectors))[0])
} else if (option === "-c") {
let result = await hinting.pipe(selectors)
if (result === null) return null
DOM.simulateClick(result[0])
}
// Deprecated: hint exstr
else if (option === "-W") run_exstr(selectors + " " + rest.join(" ") + " " + (await hinting.pipe(DOM.HINTTAGS_selectors))[0])
else if (option === "-pipe") return (await hinting.pipe(selectors))[0][rest.join(" ")]
else if (option === "-br") {
else if (option === "-W") {
let result = await hinting.pipe(DOM.HINTTAGS_selectors)
if (result === null) return null
run_exstr(selectors + " " + rest.join(" ") + " " + result[0])
} else if (option === "-pipe") {
let result = await hinting.pipe(selectors)
if (result === null) return null
return result[0][rest.join(" ")]
} else if (option === "-br") {
while (true) {
let [_, hintCount] = await hint("-b")
let result = await hint("-b")
if (result === null) return null
let [_, hintCount] = result
if (hintCount < 2) break
}
}
@ -2970,7 +2990,11 @@ export async function hint(option?: string, selectors?: string, ...rest: string[
else if (option === "-r") hinting.hintRead()
else if (option === "-w") hinting.hintPageWindow()
else if (option === "-wp") hinting.hintPageWindowPrivate()
else DOM.simulateClick((await hinting.pipe(DOM.HINTTAGS_selectors))[0])
else {
let result = await hinting.pipe(DOM.HINTTAGS_selectors)
if (result === null) return null
DOM.simulateClick(result[0])
}
}
// how 2 crash pc

View file

@ -36,11 +36,11 @@ class HintState {
public filter = ""
public hintchars = ""
constructor(public filterFunc: HintFilter) {
constructor(public filterFunc: HintFilter, public resolve: (Hint) => void) {
this.hintHost.classList.add("TridactylHintHost", "cleanslate")
}
destructor() {
destructor(abort) {
// Undo any alterations of the hinted elements
for (const hint of this.hints) {
hint.hidden = true
@ -48,6 +48,9 @@ class HintState {
// Remove all hints from the DOM.
this.hintHost.remove()
if (abort) this.resolve(null)
else this.resolve(this.focusedHint)
}
}
@ -57,11 +60,13 @@ let modeState: HintState = undefined
export function hintPage(
hintableElements: Element[],
onSelect: HintSelectedCallback,
buildHints: HintBuilder = defaultHintBuilder(),
filterHints: HintFilter = defaultHintFilter(),
resolve = () => {},
) {
let buildHints: HintBuilder = defaultHintBuilder()
let filterHints: HintFilter = defaultHintFilter()
state.mode = "hint"
modeState = new HintState(filterHints)
modeState = new HintState(filterHints, resolve)
buildHints(hintableElements, onSelect)
if (modeState.hints.length) {
@ -421,9 +426,12 @@ function filterHintsVimperator(fstr, reflow = false) {
}
}
/** Remove all hints, reset STATE. */
function reset() {
modeState.destructor()
/** Remove all hints, reset STATE.
* If abort is true, we're resetting because the user pressed escape.
* If it is false, we're resetting because the user selected a hint.
**/
function reset(abort = false) {
modeState.destructor(abort)
modeState = undefined
state.mode = "normal"
}
@ -509,9 +517,10 @@ export async function pipe(
): Promise<[any, number]> {
let hintCount = hintables(selectors, true).length
let hint = await new Promise(resolve => {
hintPage(hintables(selectors, true), resolve)
hintPage(hintables(selectors, true), () => {}, resolve)
})
return [(hint as any).target, hintCount]
if (hint) return [(hint as any).target, hintCount]
else return null
// Promise takes function which it calls immediately with another function
// as its argument. When this second function is called, it gives its
// argument to the promise as its value
@ -519,9 +528,10 @@ export async function pipe(
export async function pipe_elements(elements: any = DOM.elementsWithText) {
let hint = await new Promise(resolve => {
hintPage(elements, resolve)
hintPage(elements, () => {}, resolve)
})
return (hint as any).target
if (hint) return (hint as any).target
else return null
}
/** Hint images, opening in the same tab, or in a background tab

View file

@ -8,8 +8,9 @@ async function selectFocusedHint() {
return await messageActiveTab("hinting_content", "selectFocusedHint")
}
async function reset() {
return await messageActiveTab("hinting_content", "reset")
/* abort = true means that we're resetting because the user pressed escape */
async function reset(abort = false) {
return await messageActiveTab("hinting_content", "reset", [abort])
}
/** Type for "hint save" actions:
@ -30,7 +31,7 @@ import { MsgSafeKeyboardEvent } from "./msgsafe"
export function parser(keys: MsgSafeKeyboardEvent[]) {
const key = keys[0].key
if (key === "Escape") {
reset()
reset(true)
} else if (["Enter", " "].includes(key)) {
selectFocusedHint()
} else {