From c7b4c2beaf7cb26a7856a1a4d84d6a912fccd5e5 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 2 Mar 2017 17:33:58 -0800 Subject: [PATCH 1/9] Require ctrl for link clicks Fixes #578 --- src/Linkifier.ts | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/Linkifier.ts b/src/Linkifier.ts index 0b144cfd..41a362b5 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -218,9 +218,13 @@ export class Linkifier { const element = this._document.createElement('a'); element.textContent = uri; if (handler) { - element.addEventListener('click', () => { - // Only execute the handler if the link is not flagged as invalid - if (!element.classList.contains(INVALID_LINK_CLASS)) { + element.addEventListener('click', (event: KeyboardEvent) => { + // Don't execute the handler if the link is flagged as invalid + if (element.classList.contains(INVALID_LINK_CLASS)) { + return; + } + // Require ctrl on click + if (event.ctrlKey) { handler(uri); } }); @@ -228,6 +232,13 @@ export class Linkifier { element.href = uri; // Force link on another tab so work is not lost element.target = '_blank'; + element.addEventListener('click', (event: KeyboardEvent) => { + // Require ctrl on click + if (!event.ctrlKey) { + event.preventDefault(); + return false; + } + }); } return element; } From 012051c1db693b72bb9fea2b61a3590bd2accb54 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 2 Mar 2017 17:35:10 -0800 Subject: [PATCH 2/9] Fix matchIndex on http --- src/Linkifier.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Linkifier.ts b/src/Linkifier.ts index 1218bdb6..23c68063 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -54,7 +54,7 @@ export class Linkifier { this._rows = rows; this._rowTimeoutIds = []; this._linkMatchers = []; - this.registerLinkMatcher(strictUrlRegex, null, 1); + this.registerLinkMatcher(strictUrlRegex, null, { matchIndex: 1 }); } /** From 15a867d2b702cf20d50a254521197b2ea52087a8 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 3 Mar 2017 12:37:34 -0800 Subject: [PATCH 3/9] Support cmd+click on mac --- src/Linkifier.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Linkifier.ts b/src/Linkifier.ts index 23c68063..93248c30 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -2,6 +2,7 @@ * @license MIT */ +import { isMac } from './utils/Browser'; import { LinkMatcherOptions } from './Interfaces'; import { LinkMatcher, LinkMatcherHandler, LinkMatcherValidationCallback } from './Types'; @@ -239,7 +240,7 @@ export class Linkifier { return; } // Require ctrl on click - if (event.ctrlKey) { + if (isMac ? event.metaKey : event.ctrlKey) { handler(uri); } }); @@ -249,7 +250,7 @@ export class Linkifier { element.target = '_blank'; element.addEventListener('click', (event: KeyboardEvent) => { // Require ctrl on click - if (!event.ctrlKey) { + if (isMac ? !event.metaKey : !event.ctrlKey) { event.preventDefault(); return false; } From 1ecc5674ac92d6876ac63b552dd49bbc46f06feb Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 4 Mar 2017 14:01:34 -0800 Subject: [PATCH 4/9] Add fail handler --- src/Interfaces.ts | 6 ++++++ src/Linkifier.ts | 20 ++++++++++++++------ src/Types.ts | 1 + 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/Interfaces.ts b/src/Interfaces.ts index ca228ce0..27ea4d4c 100644 --- a/src/Interfaces.ts +++ b/src/Interfaces.ts @@ -85,6 +85,12 @@ export interface LinkMatcherOptions { * false if invalid. */ validationCallback?: LinkMatcherValidationCallback; + /** + * A callback that fires when a link click fails due to ctrl or cmd (mac) not + * being pressed. This allows hinting to the user after a likely failed + * click. + */ + clickFailCallback?: (event: MouseEvent) => any; /** * The priority of the link matcher, this defines the order in which the link * matcher is evaluated relative to others, from highest to lowest. The diff --git a/src/Linkifier.ts b/src/Linkifier.ts index 93248c30..6667af0c 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -100,6 +100,7 @@ export class Linkifier { handler, matchIndex: options.matchIndex, validationCallback: options.validationCallback, + clickFailCallback: options.clickFailCallback, priority: options.priority || 0 }; this._addLinkMatcherToList(matcher); @@ -158,7 +159,7 @@ export class Linkifier { const matcher = this._linkMatchers[i]; const uri = this._findLinkMatch(text, matcher.regex, matcher.matchIndex); if (uri) { - const linkElement = this._doLinkifyRow(rowIndex, uri, matcher.handler); + const linkElement = this._doLinkifyRow(rowIndex, uri, matcher.handler, matcher.clickFailCallback); // Fire validation callback if (linkElement && matcher.validationCallback) { matcher.validationCallback(uri, isValid => { @@ -180,14 +181,14 @@ export class Linkifier { * @param {handler} handler The handler to trigger when the link is triggered. * @return The link element if it was added, otherwise undefined. */ - private _doLinkifyRow(rowIndex: number, uri: string, handler?: LinkMatcherHandler): HTMLElement { + private _doLinkifyRow(rowIndex: number, uri: string, handler?: LinkMatcherHandler, clickFailHandler?: (event: MouseEvent) => any): HTMLElement { // Iterate over nodes as we want to consider text nodes const nodes = this._rows[rowIndex].childNodes; for (let i = 0; i < nodes.length; i++) { const node = nodes[i]; const searchIndex = node.textContent.indexOf(uri); if (searchIndex >= 0) { - const linkElement = this._createAnchorElement(uri, handler); + const linkElement = this._createAnchorElement(uri, handler, clickFailHandler); if (node.textContent.length === uri.length) { // Matches entire string @@ -230,11 +231,11 @@ export class Linkifier { * @param {string} uri The uri of the link. * @return {HTMLAnchorElement} The link. */ - private _createAnchorElement(uri: string, handler: LinkMatcherHandler): HTMLAnchorElement { + private _createAnchorElement(uri: string, handler: LinkMatcherHandler, failHandler: (event: MouseEvent) => any): HTMLAnchorElement { const element = this._document.createElement('a'); element.textContent = uri; if (handler) { - element.addEventListener('click', (event: KeyboardEvent) => { + element.addEventListener('click', (event: MouseEvent) => { // Don't execute the handler if the link is flagged as invalid if (element.classList.contains(INVALID_LINK_CLASS)) { return; @@ -242,15 +243,22 @@ export class Linkifier { // Require ctrl on click if (isMac ? event.metaKey : event.ctrlKey) { handler(uri); + } else { + if (failHandler) { + failHandler(event); + } } }); } else { element.href = uri; // Force link on another tab so work is not lost element.target = '_blank'; - element.addEventListener('click', (event: KeyboardEvent) => { + element.addEventListener('click', (event: MouseEvent) => { // Require ctrl on click if (isMac ? !event.metaKey : !event.ctrlKey) { + if (failHandler) { + failHandler(event); + } event.preventDefault(); return false; } diff --git a/src/Types.ts b/src/Types.ts index 2dbb47ef..d3e99dd0 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -8,6 +8,7 @@ export type LinkMatcher = { handler: LinkMatcherHandler, matchIndex?: number, validationCallback?: LinkMatcherValidationCallback, + clickFailCallback?: (event: MouseEvent) => any, priority?: number }; export type LinkMatcherHandler = (uri: string) => void; From 5e2791f981279b539615e49d614902363d3138c9 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 4 Mar 2017 14:01:40 -0800 Subject: [PATCH 5/9] Revert "Add fail handler" This reverts commit 1ecc5674ac92d6876ac63b552dd49bbc46f06feb. --- src/Interfaces.ts | 6 ------ src/Linkifier.ts | 20 ++++++-------------- src/Types.ts | 1 - 3 files changed, 6 insertions(+), 21 deletions(-) diff --git a/src/Interfaces.ts b/src/Interfaces.ts index 27ea4d4c..ca228ce0 100644 --- a/src/Interfaces.ts +++ b/src/Interfaces.ts @@ -85,12 +85,6 @@ export interface LinkMatcherOptions { * false if invalid. */ validationCallback?: LinkMatcherValidationCallback; - /** - * A callback that fires when a link click fails due to ctrl or cmd (mac) not - * being pressed. This allows hinting to the user after a likely failed - * click. - */ - clickFailCallback?: (event: MouseEvent) => any; /** * The priority of the link matcher, this defines the order in which the link * matcher is evaluated relative to others, from highest to lowest. The diff --git a/src/Linkifier.ts b/src/Linkifier.ts index 6667af0c..93248c30 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -100,7 +100,6 @@ export class Linkifier { handler, matchIndex: options.matchIndex, validationCallback: options.validationCallback, - clickFailCallback: options.clickFailCallback, priority: options.priority || 0 }; this._addLinkMatcherToList(matcher); @@ -159,7 +158,7 @@ export class Linkifier { const matcher = this._linkMatchers[i]; const uri = this._findLinkMatch(text, matcher.regex, matcher.matchIndex); if (uri) { - const linkElement = this._doLinkifyRow(rowIndex, uri, matcher.handler, matcher.clickFailCallback); + const linkElement = this._doLinkifyRow(rowIndex, uri, matcher.handler); // Fire validation callback if (linkElement && matcher.validationCallback) { matcher.validationCallback(uri, isValid => { @@ -181,14 +180,14 @@ export class Linkifier { * @param {handler} handler The handler to trigger when the link is triggered. * @return The link element if it was added, otherwise undefined. */ - private _doLinkifyRow(rowIndex: number, uri: string, handler?: LinkMatcherHandler, clickFailHandler?: (event: MouseEvent) => any): HTMLElement { + private _doLinkifyRow(rowIndex: number, uri: string, handler?: LinkMatcherHandler): HTMLElement { // Iterate over nodes as we want to consider text nodes const nodes = this._rows[rowIndex].childNodes; for (let i = 0; i < nodes.length; i++) { const node = nodes[i]; const searchIndex = node.textContent.indexOf(uri); if (searchIndex >= 0) { - const linkElement = this._createAnchorElement(uri, handler, clickFailHandler); + const linkElement = this._createAnchorElement(uri, handler); if (node.textContent.length === uri.length) { // Matches entire string @@ -231,11 +230,11 @@ export class Linkifier { * @param {string} uri The uri of the link. * @return {HTMLAnchorElement} The link. */ - private _createAnchorElement(uri: string, handler: LinkMatcherHandler, failHandler: (event: MouseEvent) => any): HTMLAnchorElement { + private _createAnchorElement(uri: string, handler: LinkMatcherHandler): HTMLAnchorElement { const element = this._document.createElement('a'); element.textContent = uri; if (handler) { - element.addEventListener('click', (event: MouseEvent) => { + element.addEventListener('click', (event: KeyboardEvent) => { // Don't execute the handler if the link is flagged as invalid if (element.classList.contains(INVALID_LINK_CLASS)) { return; @@ -243,22 +242,15 @@ export class Linkifier { // Require ctrl on click if (isMac ? event.metaKey : event.ctrlKey) { handler(uri); - } else { - if (failHandler) { - failHandler(event); - } } }); } else { element.href = uri; // Force link on another tab so work is not lost element.target = '_blank'; - element.addEventListener('click', (event: MouseEvent) => { + element.addEventListener('click', (event: KeyboardEvent) => { // Require ctrl on click if (isMac ? !event.metaKey : !event.ctrlKey) { - if (failHandler) { - failHandler(event); - } event.preventDefault(); return false; } diff --git a/src/Types.ts b/src/Types.ts index d3e99dd0..2dbb47ef 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -8,7 +8,6 @@ export type LinkMatcher = { handler: LinkMatcherHandler, matchIndex?: number, validationCallback?: LinkMatcherValidationCallback, - clickFailCallback?: (event: MouseEvent) => any, priority?: number }; export type LinkMatcherHandler = (uri: string) => void; From aafb5333b9af22c28bf9143dc90d9ce60e3f03dd Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 4 Mar 2017 14:08:05 -0800 Subject: [PATCH 6/9] Give event to handler --- src/Linkifier.ts | 18 +++++++----------- src/Types.ts | 2 +- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/Linkifier.ts b/src/Linkifier.ts index 93248c30..d144fefc 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -92,7 +92,7 @@ export class Linkifier { */ public registerLinkMatcher(regex: RegExp, handler: LinkMatcherHandler, options: LinkMatcherOptions = {}): number { if (this._nextLinkMatcherId !== HYPERTEXT_LINK_MATCHER_ID && !handler) { - throw new Error('handler cannot be falsy'); + throw new Error('handler must be defined'); } const matcher: LinkMatcher = { id: this._nextLinkMatcherId++, @@ -234,25 +234,21 @@ export class Linkifier { const element = this._document.createElement('a'); element.textContent = uri; if (handler) { - element.addEventListener('click', (event: KeyboardEvent) => { + element.addEventListener('click', (event: MouseEvent) => { // Don't execute the handler if the link is flagged as invalid if (element.classList.contains(INVALID_LINK_CLASS)) { return; } - // Require ctrl on click - if (isMac ? event.metaKey : event.ctrlKey) { - handler(uri); - } + + return handler(event, uri); }); } else { element.href = uri; // Force link on another tab so work is not lost element.target = '_blank'; - element.addEventListener('click', (event: KeyboardEvent) => { - // Require ctrl on click - if (isMac ? !event.metaKey : !event.ctrlKey) { - event.preventDefault(); - return false; + element.addEventListener('click', (event: MouseEvent) => { + if (handler) { + return handler(event, uri); } }); } diff --git a/src/Types.ts b/src/Types.ts index 2dbb47ef..634e2fdc 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -10,5 +10,5 @@ export type LinkMatcher = { validationCallback?: LinkMatcherValidationCallback, priority?: number }; -export type LinkMatcherHandler = (uri: string) => void; +export type LinkMatcherHandler = (event: MouseEvent, uri: string) => void; export type LinkMatcherValidationCallback = (uri: string, callback: (isValid: boolean) => void) => void; From ab34d53d6b1b0c941e29475121adb567e9ae54bf Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 4 Mar 2017 14:09:13 -0800 Subject: [PATCH 7/9] Remove unused import --- src/Linkifier.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Linkifier.ts b/src/Linkifier.ts index d144fefc..8f011cb2 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -2,7 +2,6 @@ * @license MIT */ -import { isMac } from './utils/Browser'; import { LinkMatcherOptions } from './Interfaces'; import { LinkMatcher, LinkMatcherHandler, LinkMatcherValidationCallback } from './Types'; From 1f518b0b455109a193a936f520e4271db966ee68 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 4 Mar 2017 14:44:11 -0800 Subject: [PATCH 8/9] Allow http link handler to fallback to standard handler --- src/Linkifier.ts | 21 ++++++++++----------- src/Types.ts | 2 +- src/xterm.js | 1 - 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/Linkifier.ts b/src/Linkifier.ts index 8f011cb2..1cdf0f4a 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -229,19 +229,10 @@ export class Linkifier { * @param {string} uri The uri of the link. * @return {HTMLAnchorElement} The link. */ - private _createAnchorElement(uri: string, handler: LinkMatcherHandler): HTMLAnchorElement { + private _createAnchorElement(uri: string, handler: LinkMatcherHandler, isHypertextLinkHandler: boolean): HTMLAnchorElement { const element = this._document.createElement('a'); element.textContent = uri; - if (handler) { - element.addEventListener('click', (event: MouseEvent) => { - // Don't execute the handler if the link is flagged as invalid - if (element.classList.contains(INVALID_LINK_CLASS)) { - return; - } - - return handler(event, uri); - }); - } else { + if (isHypertextLinkHandler) { element.href = uri; // Force link on another tab so work is not lost element.target = '_blank'; @@ -250,6 +241,14 @@ export class Linkifier { return handler(event, uri); } }); + } else { + element.addEventListener('click', (event: MouseEvent) => { + // Don't execute the handler if the link is flagged as invalid + if (element.classList.contains(INVALID_LINK_CLASS)) { + return; + } + return handler(event, uri); + }); } return element; } diff --git a/src/Types.ts b/src/Types.ts index 634e2fdc..9e8cd762 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -10,5 +10,5 @@ export type LinkMatcher = { validationCallback?: LinkMatcherValidationCallback, priority?: number }; -export type LinkMatcherHandler = (event: MouseEvent, uri: string) => void; +export type LinkMatcherHandler = (event: MouseEvent, uri: string) => boolean | void; export type LinkMatcherValidationCallback = (uri: string, callback: (isValid: boolean) => void) => void; diff --git a/src/xterm.js b/src/xterm.js index a08e9dd1..37c00ed8 100644 --- a/src/xterm.js +++ b/src/xterm.js @@ -1295,7 +1295,6 @@ Terminal.prototype.attachHypertextLinkHandler = function(handler) { this.refresh(0, this.rows - 1); } - /** * Registers a link matcher, allowing custom link patterns to be matched and * handled. From 235ae2fbb98ac95efc4f55517d755f148462c8a3 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 4 Mar 2017 15:02:08 -0800 Subject: [PATCH 9/9] Fix http link handler logic --- src/Linkifier.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Linkifier.ts b/src/Linkifier.ts index 1cdf0f4a..a34edbc2 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -157,7 +157,7 @@ export class Linkifier { const matcher = this._linkMatchers[i]; const uri = this._findLinkMatch(text, matcher.regex, matcher.matchIndex); if (uri) { - const linkElement = this._doLinkifyRow(rowIndex, uri, matcher.handler); + const linkElement = this._doLinkifyRow(rowIndex, uri, matcher.handler, matcher.id === HYPERTEXT_LINK_MATCHER_ID); // Fire validation callback if (linkElement && matcher.validationCallback) { matcher.validationCallback(uri, isValid => { @@ -179,14 +179,14 @@ export class Linkifier { * @param {handler} handler The handler to trigger when the link is triggered. * @return The link element if it was added, otherwise undefined. */ - private _doLinkifyRow(rowIndex: number, uri: string, handler?: LinkMatcherHandler): HTMLElement { + private _doLinkifyRow(rowIndex: number, uri: string, handler: LinkMatcherHandler, isHttpLinkMatcher: boolean): HTMLElement { // Iterate over nodes as we want to consider text nodes const nodes = this._rows[rowIndex].childNodes; for (let i = 0; i < nodes.length; i++) { const node = nodes[i]; const searchIndex = node.textContent.indexOf(uri); if (searchIndex >= 0) { - const linkElement = this._createAnchorElement(uri, handler); + const linkElement = this._createAnchorElement(uri, handler, isHttpLinkMatcher); if (node.textContent.length === uri.length) { // Matches entire string