From f8f135515517f5e68276e7a59a4c4449f9c934ac Mon Sep 17 00:00:00 2001 From: Bruno Ribeito Date: Mon, 19 Feb 2018 22:54:09 +0000 Subject: [PATCH 1/4] Validate colors --- src/renderer/ColorManager.ts | 72 +++++++++++++++++++++++++----------- 1 file changed, 51 insertions(+), 21 deletions(-) diff --git a/src/renderer/ColorManager.ts b/src/renderer/ColorManager.ts index 90ef0431..43a41f56 100644 --- a/src/renderer/ColorManager.ts +++ b/src/renderer/ColorManager.ts @@ -66,6 +66,8 @@ function toPaddedHex(c: number): string { * Manages the source of truth for a terminal's colors. */ export class ColorManager implements IColorManager { + private VALID_NON_NAMED_COLORS = new RegExp('^(#[0-9a-f]{3}|#(?:[0-9a-f]{2}){2,4}|(rgb|hsl)a?\((-?\d+%?[,\s]+){2,3}\s*[\d\.]+%?\))$', 'g'); + public colors: IColorSet; constructor() { @@ -85,26 +87,54 @@ export class ColorManager implements IColorManager { * colors will be used where colors are not defined. */ public setTheme(theme: ITheme): void { - this.colors.foreground = theme.foreground || DEFAULT_FOREGROUND; - this.colors.background = theme.background || DEFAULT_BACKGROUND; - this.colors.cursor = theme.cursor || DEFAULT_CURSOR; - this.colors.cursorAccent = theme.cursorAccent || DEFAULT_CURSOR_ACCENT; - this.colors.selection = theme.selection || DEFAULT_SELECTION; - this.colors.ansi[0] = theme.black || DEFAULT_ANSI_COLORS[0]; - this.colors.ansi[1] = theme.red || DEFAULT_ANSI_COLORS[1]; - this.colors.ansi[2] = theme.green || DEFAULT_ANSI_COLORS[2]; - this.colors.ansi[3] = theme.yellow || DEFAULT_ANSI_COLORS[3]; - this.colors.ansi[4] = theme.blue || DEFAULT_ANSI_COLORS[4]; - this.colors.ansi[5] = theme.magenta || DEFAULT_ANSI_COLORS[5]; - this.colors.ansi[6] = theme.cyan || DEFAULT_ANSI_COLORS[6]; - this.colors.ansi[7] = theme.white || DEFAULT_ANSI_COLORS[7]; - this.colors.ansi[8] = theme.brightBlack || DEFAULT_ANSI_COLORS[8]; - this.colors.ansi[9] = theme.brightRed || DEFAULT_ANSI_COLORS[9]; - this.colors.ansi[10] = theme.brightGreen || DEFAULT_ANSI_COLORS[10]; - this.colors.ansi[11] = theme.brightYellow || DEFAULT_ANSI_COLORS[11]; - this.colors.ansi[12] = theme.brightBlue || DEFAULT_ANSI_COLORS[12]; - this.colors.ansi[13] = theme.brightMagenta || DEFAULT_ANSI_COLORS[13]; - this.colors.ansi[14] = theme.brightCyan || DEFAULT_ANSI_COLORS[14]; - this.colors.ansi[15] = theme.brightWhite || DEFAULT_ANSI_COLORS[15]; + this.colors.foreground = this._validateColor(theme.foreground, DEFAULT_FOREGROUND); + this.colors.background = this._validateColor(theme.background, DEFAULT_BACKGROUND); + this.colors.cursor = this._validateColor(theme.cursor, DEFAULT_CURSOR); + this.colors.cursorAccent = this._validateColor(theme.cursorAccent, DEFAULT_CURSOR_ACCENT); + this.colors.selection = this._validateColor(theme.selection, DEFAULT_SELECTION); + this.colors.ansi[0] = this._validateColor(theme.black, DEFAULT_ANSI_COLORS[0]); + this.colors.ansi[1] = this._validateColor(theme.red, DEFAULT_ANSI_COLORS[1]); + this.colors.ansi[2] = this._validateColor(theme.green, DEFAULT_ANSI_COLORS[2]); + this.colors.ansi[3] = this._validateColor(theme.yellow, DEFAULT_ANSI_COLORS[3]); + this.colors.ansi[4] = this._validateColor(theme.blue, DEFAULT_ANSI_COLORS[4]); + this.colors.ansi[5] = this._validateColor(theme.magenta, DEFAULT_ANSI_COLORS[5]); + this.colors.ansi[6] = this._validateColor(theme.cyan, DEFAULT_ANSI_COLORS[6]); + this.colors.ansi[7] = this._validateColor(theme.white, DEFAULT_ANSI_COLORS[7]); + this.colors.ansi[8] = this._validateColor(theme.brightBlack, DEFAULT_ANSI_COLORS[8]); + this.colors.ansi[9] = this._validateColor(theme.brightRed, DEFAULT_ANSI_COLORS[9]); + this.colors.ansi[10] = this._validateColor(theme.brightGreen, DEFAULT_ANSI_COLORS[10]); + this.colors.ansi[11] = this._validateColor(theme.brightYellow, DEFAULT_ANSI_COLORS[11]); + this.colors.ansi[12] = this._validateColor(theme.brightBlue, DEFAULT_ANSI_COLORS[12]); + this.colors.ansi[13] = this._validateColor(theme.brightMagenta, DEFAULT_ANSI_COLORS[13]); + this.colors.ansi[14] = this._validateColor(theme.brightCyan, DEFAULT_ANSI_COLORS[14]); + this.colors.ansi[15] = this._validateColor(theme.brightWhite, DEFAULT_ANSI_COLORS[15]); + } + + private _validateColor(color: string, fallback: string): string { + if (!color) { + return fallback; + } + + const isColorValid = this.VALID_NON_NAMED_COLORS.exec(color) || this._validateNamedColor(color); + + if (!isColorValid) { + console.warn(`Color: ${color} is invalid using fallback ${fallback}`); + } + + return isColorValid ? color : fallback; + } + + private _validateNamedColor(color: string): boolean { + const litmus = 'red'; + const d = document.createElement('div'); + d.style.color = litmus; + d.style.color = color; + + // Element's style.color will be reverted to litmus or set to '' if an invalid color is given + if (color !== litmus && (d.style.color === litmus || d.style.color === '')) { + return false; + } + + return true; } } From 99aa142cefe8f1da1c04cc07425d26ec50ed3c08 Mon Sep 17 00:00:00 2001 From: Bruno Ribeito Date: Mon, 19 Feb 2018 23:55:25 +0000 Subject: [PATCH 2/4] Fix test -> wrong Regex flag --- src/renderer/ColorManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderer/ColorManager.ts b/src/renderer/ColorManager.ts index 43a41f56..ee9c9eac 100644 --- a/src/renderer/ColorManager.ts +++ b/src/renderer/ColorManager.ts @@ -66,7 +66,7 @@ function toPaddedHex(c: number): string { * Manages the source of truth for a terminal's colors. */ export class ColorManager implements IColorManager { - private VALID_NON_NAMED_COLORS = new RegExp('^(#[0-9a-f]{3}|#(?:[0-9a-f]{2}){2,4}|(rgb|hsl)a?\((-?\d+%?[,\s]+){2,3}\s*[\d\.]+%?\))$', 'g'); + private VALID_NON_NAMED_COLORS = new RegExp('^(#[0-9a-f]{3}|#(?:[0-9a-f]{2}){2,4}|(rgb|hsl)a?\((-?\d+%?[,\s]+){2,3}\s*[\d\.]+%?\))$', 'i'); public colors: IColorSet; From a9d6686e14b4f54350561ec5be37861161953525 Mon Sep 17 00:00:00 2001 From: Bruno Ribeito Date: Tue, 20 Feb 2018 20:38:12 +0000 Subject: [PATCH 3/4] Regex is not needed for color validation --- src/renderer/ColorManager.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/renderer/ColorManager.ts b/src/renderer/ColorManager.ts index ee9c9eac..33d69e26 100644 --- a/src/renderer/ColorManager.ts +++ b/src/renderer/ColorManager.ts @@ -66,8 +66,6 @@ function toPaddedHex(c: number): string { * Manages the source of truth for a terminal's colors. */ export class ColorManager implements IColorManager { - private VALID_NON_NAMED_COLORS = new RegExp('^(#[0-9a-f]{3}|#(?:[0-9a-f]{2}){2,4}|(rgb|hsl)a?\((-?\d+%?[,\s]+){2,3}\s*[\d\.]+%?\))$', 'i'); - public colors: IColorSet; constructor() { @@ -115,7 +113,7 @@ export class ColorManager implements IColorManager { return fallback; } - const isColorValid = this.VALID_NON_NAMED_COLORS.exec(color) || this._validateNamedColor(color); + const isColorValid = this._isColorValid(color); if (!isColorValid) { console.warn(`Color: ${color} is invalid using fallback ${fallback}`); @@ -124,7 +122,7 @@ export class ColorManager implements IColorManager { return isColorValid ? color : fallback; } - private _validateNamedColor(color: string): boolean { + private _isColorValid(color: string): boolean { const litmus = 'red'; const d = document.createElement('div'); d.style.color = litmus; From 1114977a2194db5457b4f9f304defa3d4d7d480e Mon Sep 17 00:00:00 2001 From: Bruno Ribeito Date: Wed, 21 Feb 2018 23:35:37 +0000 Subject: [PATCH 4/4] Fix tests --- src/renderer/ColorManager.test.ts | 9 ++++++++- src/renderer/ColorManager.ts | 6 ++++-- src/renderer/Renderer.ts | 2 +- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/renderer/ColorManager.test.ts b/src/renderer/ColorManager.test.ts index 22407604..2dc60408 100644 --- a/src/renderer/ColorManager.test.ts +++ b/src/renderer/ColorManager.test.ts @@ -3,14 +3,21 @@ * @license MIT */ +import jsdom = require('jsdom'); import { assert } from 'chai'; import { ColorManager } from './ColorManager'; describe('ColorManager', () => { let cm: ColorManager; + let dom: jsdom.JSDOM; + let document: Document; + let window: Window; beforeEach(() => { - cm = new ColorManager(); + dom = new jsdom.JSDOM(''); + window = dom.window; + document = window.document; + cm = new ColorManager(document); }); describe('constructor', () => { diff --git a/src/renderer/ColorManager.ts b/src/renderer/ColorManager.ts index 33d69e26..ddb928a5 100644 --- a/src/renderer/ColorManager.ts +++ b/src/renderer/ColorManager.ts @@ -67,8 +67,10 @@ function toPaddedHex(c: number): string { */ export class ColorManager implements IColorManager { public colors: IColorSet; + private _document: Document; - constructor() { + constructor(document: Document) { + this._document = document; this.colors = { foreground: DEFAULT_FOREGROUND, background: DEFAULT_BACKGROUND, @@ -124,7 +126,7 @@ export class ColorManager implements IColorManager { private _isColorValid(color: string): boolean { const litmus = 'red'; - const d = document.createElement('div'); + const d = this._document.createElement('div'); d.style.color = litmus; d.style.color = color; diff --git a/src/renderer/Renderer.ts b/src/renderer/Renderer.ts index 2408f808..ca5fca6a 100644 --- a/src/renderer/Renderer.ts +++ b/src/renderer/Renderer.ts @@ -31,7 +31,7 @@ export class Renderer extends EventEmitter implements IRenderer { constructor(private _terminal: ITerminal, theme: ITheme) { super(); - this.colorManager = new ColorManager(); + this.colorManager = new ColorManager(document); if (theme) { this.colorManager.setTheme(theme); }