From 6198556e50cb8c22e91ecbd82becc0a05d2016b0 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Mon, 27 Feb 2017 08:35:56 -0800 Subject: [PATCH 01/12] Implement link validation Fixes #570 --- src/Interfaces.ts | 22 ++++++++++++++++++++++ src/Linkifier.ts | 43 +++++++++++++++++++++++++++++++++++-------- src/Types.ts | 5 +++++ src/xterm.css | 5 +++++ src/xterm.js | 7 +++---- 5 files changed, 70 insertions(+), 12 deletions(-) create mode 100644 src/Types.ts diff --git a/src/Interfaces.ts b/src/Interfaces.ts index 92094c95..324eec16 100644 --- a/src/Interfaces.ts +++ b/src/Interfaces.ts @@ -2,6 +2,8 @@ * @license MIT */ +import { LinkMatcherValidationCallback } from './Types'; + export interface IBrowser { isNode: boolean; userAgent: string; @@ -64,6 +66,26 @@ interface ICircularList { shiftElements(start: number, count: number, offset: number): void; } +export interface LinkMatcherOptions { + /** + * The index of the link from the regex.match(text) call. This defaults to 0 + * (for regular expressions without capture groups). + */ + matchIndex?: number; + /** + * A callback that validates an individual link, returning true if valid and + * false if invalid. + */ + validationCallback?: LinkMatcherValidationCallback; +} + +/** + * + * @param a + */ +function a(a) { +} + /** * Handles actions generated by the parser. */ diff --git a/src/Linkifier.ts b/src/Linkifier.ts index e0f5eb92..73ad04a5 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -2,9 +2,20 @@ * @license MIT */ +import { LinkMatcherOptions } from './Interfaces'; +import { LinkMatcherValidationCallback } from './Types'; + export type LinkHandler = (uri: string) => void; -type LinkMatcher = {id: number, regex: RegExp, matchIndex?: number, handler: LinkHandler}; +type LinkMatcher = { + id: number, + regex: RegExp, + handler: LinkHandler, + matchIndex?: number, + validationCallback?: LinkMatcherValidationCallback; +}; + +const INVALID_LINK_CLASS = 'xterm-invalid-link'; const protocolClause = '(https?:\\/\\/)'; const domainCharacterSet = '[\\da-z\\.-]+'; @@ -79,11 +90,10 @@ export class Linkifier { * this searches the textContent of the rows. You will want to use \s to match * a space ' ' character for example. * @param {LinkHandler} handler The callback when the link is called. - * @param {number} matchIndex The index of the link from the regex.match(text) - * call. This defaults to 0 (for regular expressions without capture groups). + * @param {LinkMatcherOptions} [options] Options for the link matcher. * @return {number} The ID of the new matcher, this can be used to deregister. */ - public registerLinkMatcher(regex: RegExp, handler: LinkHandler, matchIndex?: number): number { + public registerLinkMatcher(regex: RegExp, handler: LinkHandler, options: LinkMatcherOptions = {}): number { if (this._nextLinkMatcherId !== HYPERTEXT_LINK_MATCHER_ID && !handler) { throw new Error('handler cannot be falsy'); } @@ -91,7 +101,8 @@ export class Linkifier { id: this._nextLinkMatcherId++, regex, handler, - matchIndex + matchIndex: options.matchIndex, + validationCallback: options.validationCallback }; this._linkMatchers.push(matcher); return matcher.id; @@ -123,11 +134,20 @@ export class Linkifier { return; } const text = row.textContent; + // TODO: Onl execute handler if isValid for (let i = 0; i < this._linkMatchers.length; i++) { const matcher = this._linkMatchers[i]; const uri = this._findLinkMatch(text, matcher.regex, matcher.matchIndex); if (uri) { - this._doLinkifyRow(rowIndex, uri, matcher.handler); + const linkElement = this._doLinkifyRow(rowIndex, uri, matcher.handler); + // Fire validation callback + if (matcher.validationCallback) { + matcher.validationCallback(uri, isValid => { + if (!isValid) { + linkElement.classList.add(INVALID_LINK_CLASS); + } + }); + } // Only allow a single LinkMatcher to trigger on any given row. return; } @@ -139,8 +159,9 @@ export class Linkifier { * @param {number} rowIndex The index of the row to linkify. * @param {string} uri The uri that has been found. * @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?: LinkHandler): void { + private _doLinkifyRow(rowIndex: number, uri: string, handler?: LinkHandler): 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++) { @@ -165,6 +186,7 @@ export class Linkifier { // Matches part of string this._replaceNodeSubstringWithNode(node, linkElement, uri, searchIndex); } + return linkElement; } } } @@ -192,7 +214,12 @@ export class Linkifier { const element = document.createElement('a'); element.textContent = uri; if (handler) { - element.addEventListener('click', () => handler(uri)); + element.addEventListener('click', () => { + // Only execute the handler if the link is not flagged as invalid + if (!element.classList.contains(INVALID_LINK_CLASS)) { + handler(uri); + } + }); } else { element.href = uri; // Force link on another tab so work is not lost diff --git a/src/Types.ts b/src/Types.ts new file mode 100644 index 00000000..1eafa101 --- /dev/null +++ b/src/Types.ts @@ -0,0 +1,5 @@ +/** + * @license MIT + */ + +export type LinkMatcherValidationCallback = (uri: string, callback: (isValid: boolean) => void) => void; diff --git a/src/xterm.css b/src/xterm.css index f99eb688..0401e607 100644 --- a/src/xterm.css +++ b/src/xterm.css @@ -81,6 +81,11 @@ text-decoration: underline; } +.terminal a.xterm-invalid-link:hover { + cursor: default; + text-decoration: none; +} + .terminal:not(.xterm-cursor-style-underline):not(.xterm-cursor-style-bar) .terminal-cursor { background-color: #fff; color: #000; diff --git a/src/xterm.js b/src/xterm.js index d09d5fb2..69eb0a1f 100644 --- a/src/xterm.js +++ b/src/xterm.js @@ -1303,13 +1303,12 @@ Terminal.prototype.attachHypertextLinkHandler = function(handler) { * this searches the textContent of the rows. You will want to use \s to match * a space ' ' character for example. * @param {LinkHandler} handler The callback when the link is called. - * @param {number} matchIndex The index of the link from the regex.match(text) - * call. This defaults to 0 (for regular expressions without capture groups). + * @param {LinkMatcherOptions} [options] Options for the link matcher. * @return {number} The ID of the new matcher, this can be used to deregister. */ -Terminal.prototype.registerLinkMatcher = function(regex, handler, matchIndex) { +Terminal.prototype.registerLinkMatcher = function(regex, handler, options) { if (this.linkifier) { - var matcherId = this.linkifier.registerLinkMatcher(regex, handler, matchIndex); + var matcherId = this.linkifier.registerLinkMatcher(regex, handler, options); this.refresh(0, this.rows - 1); return matcherId; } From 4c99c032cac4a01841466aa010b92786ebd30b10 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Mon, 27 Feb 2017 09:47:36 -0800 Subject: [PATCH 02/12] Add tests --- src/Interfaces.ts | 10 +++++- src/Linkifier.phantom.ts | 76 ++++++++++++++++++++++++++++++++++++++++ src/Linkifier.ts | 23 +++++++----- src/Types.ts | 1 + src/xterm.js | 1 + test-harness.html | 1 + 6 files changed, 102 insertions(+), 10 deletions(-) create mode 100644 src/Linkifier.phantom.ts diff --git a/src/Interfaces.ts b/src/Interfaces.ts index 324eec16..ca9de632 100644 --- a/src/Interfaces.ts +++ b/src/Interfaces.ts @@ -2,7 +2,8 @@ * @license MIT */ -import { LinkMatcherValidationCallback } from './Types'; +import { LinkMatcherOptions } from './Interfaces'; +import { LinkMatcherHandler, LinkMatcherValidationCallback } from './Types'; export interface IBrowser { isNode: boolean; @@ -52,6 +53,13 @@ export interface ICharMeasure { measure(): void; } +export interface ILinkifier { + linkifyRow(rowIndex: number): void; + attachHypertextLinkHandler(handler: LinkMatcherHandler): void; + registerLinkMatcher(regex: RegExp, handler: LinkMatcherHandler, options?: LinkMatcherOptions): number; + deregisterLinkMatcher(matcherId: number): boolean; +} + interface ICircularList { length: number; maxLength: number; diff --git a/src/Linkifier.phantom.ts b/src/Linkifier.phantom.ts new file mode 100644 index 00000000..01de4efc --- /dev/null +++ b/src/Linkifier.phantom.ts @@ -0,0 +1,76 @@ +/** + * @license MIT + */ +import { ITerminal, ILinkifier } from './Interfaces'; + +declare var assert: Chai.Assert; +declare var Terminal: ITerminal; + +// Do not describe tests unless in PhantomJS environment +if (typeof Terminal !== 'undefined') { + + const Linkifier = (Terminal).Linkifier; + Linkifier.setTimeBeforeLinkifyForTest(0); + + describe('Linkifier', () => { + let container: HTMLElement; + let rows: HTMLElement[]; + let linkifier: ILinkifier; + + beforeEach(() => { + container = document.createElement('div'); + document.querySelector('#xterm').appendChild(container); + rows = []; + linkifier = new Linkifier(rows); + }); + + afterEach(() => { + while (rows.length) { + container.removeChild(rows.pop()); + } + document.querySelector('#xterm').removeChild(container); + }); + + function addRow(text: string) { + const element = document.createElement('div'); + element.textContent = text; + container.appendChild(element); + rows.push(element); + } + + function clickElement(element: Node) { + const event = document.createEvent('MouseEvent'); + event.initMouseEvent('click', true, true, window, 0, 0, 0, 0, 0, false, false, false, false, 0, null); + element.dispatchEvent(event); + } + + describe('validationCallback', () => { + it('should enable link if true', done => { + addRow('test'); + linkifier.registerLinkMatcher(/test/, () => done(), { + validationCallback: (url, cb) => { + cb(true); + assert.equal((rows[0].firstChild).tagName, 'A'); + setTimeout(() => clickElement(rows[0].firstChild), 0); + } + }); + linkifier.linkifyRow(0); + }); + + it('should disable link if false', done => { + addRow('test'); + linkifier.registerLinkMatcher(/test/, () => assert.fail(), { + validationCallback: (url, cb) => { + cb(false); + assert.equal((rows[0].firstChild).tagName, 'A'); + setTimeout(() => clickElement(rows[0].firstChild), 0); + } + }); + linkifier.linkifyRow(0); + // Allow time for the click to be performed + setTimeout(() => done(), 10); + }); + }); + }); + +} diff --git a/src/Linkifier.ts b/src/Linkifier.ts index 73ad04a5..9f29ba58 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -3,14 +3,12 @@ */ import { LinkMatcherOptions } from './Interfaces'; -import { LinkMatcherValidationCallback } from './Types'; - -export type LinkHandler = (uri: string) => void; +import { LinkMatcherHandler, LinkMatcherValidationCallback } from './Types'; type LinkMatcher = { id: number, regex: RegExp, - handler: LinkHandler, + handler: LinkMatcherHandler, matchIndex?: number, validationCallback?: LinkMatcherValidationCallback; }; @@ -43,7 +41,7 @@ const HYPERTEXT_LINK_MATCHER_ID = 0; * the costly operation of searching every row multiple times, pntentially a * huge aount of times. */ -const TIME_BEFORE_LINKIFY = 200; +let TIME_BEFORE_LINKIFY = 200; /** * The Linkifier applies links to rows shortly after they have been refreshed. @@ -79,7 +77,7 @@ export class Linkifier { * @param {LinkHandler} handler The handler to use, this can be cleared with * null. */ - public attachHypertextLinkHandler(handler: LinkHandler): void { + public attachHypertextLinkHandler(handler: LinkMatcherHandler): void { this._linkMatchers[HYPERTEXT_LINK_MATCHER_ID].handler = handler; } @@ -93,7 +91,7 @@ export class Linkifier { * @param {LinkMatcherOptions} [options] Options for the link matcher. * @return {number} The ID of the new matcher, this can be used to deregister. */ - public registerLinkMatcher(regex: RegExp, handler: LinkHandler, options: LinkMatcherOptions = {}): number { + public registerLinkMatcher(regex: RegExp, handler: LinkMatcherHandler, options: LinkMatcherOptions = {}): number { if (this._nextLinkMatcherId !== HYPERTEXT_LINK_MATCHER_ID && !handler) { throw new Error('handler cannot be falsy'); } @@ -161,7 +159,7 @@ 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?: LinkHandler): 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++) { @@ -210,7 +208,7 @@ export class Linkifier { * @param {string} uri The uri of the link. * @return {HTMLAnchorElement} The link. */ - private _createAnchorElement(uri: string, handler: LinkHandler): HTMLAnchorElement { + private _createAnchorElement(uri: string, handler: LinkMatcherHandler): HTMLAnchorElement { const element = document.createElement('a'); element.textContent = uri; if (handler) { @@ -283,4 +281,11 @@ export class Linkifier { this._replaceNode(node, leftTextNode, newNode, rightTextNode); } } + + public static setTimeBeforeLinkifyForTest(time: number) { + // This is necessary since it's needs to be used in PhantomJS. Ideally the + // time variable would be a protected static member and a TestLinkifier + // would expose it for the test. + TIME_BEFORE_LINKIFY = time; + } } diff --git a/src/Types.ts b/src/Types.ts index 1eafa101..038a1f7a 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -2,4 +2,5 @@ * @license MIT */ +export type LinkMatcherHandler = (uri: string) => void; export type LinkMatcherValidationCallback = (uri: string, callback: (isValid: boolean) => void) => void; diff --git a/src/xterm.js b/src/xterm.js index 69eb0a1f..4d5f0205 100644 --- a/src/xterm.js +++ b/src/xterm.js @@ -2302,6 +2302,7 @@ Terminal.inherits = inherits; // Expose for Phantom.JS tests Terminal.CharMeasure = CharMeasure; +Terminal.Linkifier = Linkifier; /** * Adds an event listener to the terminal. diff --git a/test-harness.html b/test-harness.html index 8c5f0b30..bad4d36e 100644 --- a/test-harness.html +++ b/test-harness.html @@ -14,6 +14,7 @@ assert = chai.assert + - - - - - - - -