From dc5b2a4ea0849f38ec5eeb37be38fc9e3b1bac34 Mon Sep 17 00:00:00 2001 From: Frederic Junod Date: Fri, 12 Jan 2018 12:14:55 +0100 Subject: [PATCH 1/5] Fix for loop in color test --- test/spec/ol/color.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/spec/ol/color.test.js b/test/spec/ol/color.test.js index ad0993f5c1..83b45acc07 100644 --- a/test/spec/ol/color.test.js +++ b/test/spec/ol/color.test.js @@ -123,7 +123,7 @@ describe('ol.color', function() { it('throws an error on invalid colors', function() { const invalidColors = ['tuesday', '#12345', '#1234567', 'rgb(255.0,0,0)']; let i, ii; - for (i = 0, ii < invalidColors.length; i < ii; ++i) { + for (i = 0, ii = invalidColors.length; i < ii; ++i) { expect(function() { fromString(invalidColors[i]); }).to.throwException(); From bb0904f20d1332d5e81332a16178c640b6a0c41d Mon Sep 17 00:00:00 2001 From: Frederic Junod Date: Fri, 12 Jan 2018 13:34:03 +0100 Subject: [PATCH 2/5] Fix invalid named color detection If the named color is invalid, the value is not stored into the property. --- src/ol/color.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/ol/color.js b/src/ol/color.js index 2a5cf1f6f6..1f1b1bc713 100644 --- a/src/ol/color.js +++ b/src/ol/color.js @@ -45,10 +45,14 @@ export function asString(color) { function fromNamed(color) { const el = document.createElement('div'); el.style.color = color; - document.body.appendChild(el); - const rgb = getComputedStyle(el).color; - document.body.removeChild(el); - return rgb; + if (el.style.color !== '') { + document.body.appendChild(el); + const rgb = getComputedStyle(el).color; + document.body.removeChild(el); + return rgb; + } else { + return ''; + } } From 5ade60218335ae5375786d7dba321f6659e13ee8 Mon Sep 17 00:00:00 2001 From: Frederic Junod Date: Fri, 12 Jan 2018 14:55:48 +0100 Subject: [PATCH 3/5] Remove 'rgb(255.0,0,0)' from the invalid color list `rgb(255.0,0,0)` is a valid color value --- test/spec/ol/color.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/spec/ol/color.test.js b/test/spec/ol/color.test.js index 83b45acc07..b230e289ad 100644 --- a/test/spec/ol/color.test.js +++ b/test/spec/ol/color.test.js @@ -121,7 +121,7 @@ describe('ol.color', function() { }); it('throws an error on invalid colors', function() { - const invalidColors = ['tuesday', '#12345', '#1234567', 'rgb(255.0,0,0)']; + const invalidColors = ['tuesday', '#12345', '#1234567']; let i, ii; for (i = 0, ii = invalidColors.length; i < ii; ++i) { expect(function() { From 974f9391f222919aa47f41ca815146466dbc89ab Mon Sep 17 00:00:00 2001 From: Frederic Junod Date: Fri, 12 Jan 2018 15:21:25 +0100 Subject: [PATCH 4/5] Normalize color in place to reduce garbage generation --- src/ol/color.js | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/ol/color.js b/src/ol/color.js index 1f1b1bc713..a4c4ef64fd 100644 --- a/src/ol/color.js +++ b/src/ol/color.js @@ -134,7 +134,7 @@ export function asArray(color) { * @return {ol.Color} Color. */ function fromStringInternal_(s) { - let r, g, b, a, color, parts; + let r, g, b, a, color; if (NAMED_COLOR_RE_.exec(s)) { s = fromNamed(s); @@ -167,12 +167,12 @@ function fromStringInternal_(s) { } color = [r, g, b, a / 255]; } else if (s.indexOf('rgba(') == 0) { // rgba() - parts = s.slice(5, -1).split(',').map(Number); - color = normalize(parts); + color = s.slice(5, -1).split(',').map(Number); + normalize(color); } else if (s.indexOf('rgb(') == 0) { // rgb() - parts = s.slice(4, -1).split(',').map(Number); - parts.push(1); - color = normalize(parts); + color = s.slice(4, -1).split(',').map(Number); + color.push(1); + normalize(color); } else { assert(false, 14); // Invalid color } @@ -183,16 +183,14 @@ function fromStringInternal_(s) { /** * TODO this function is only used in the test, we probably shouldn't export it * @param {ol.Color} color Color. - * @param {ol.Color=} opt_color Color. * @return {ol.Color} Clamped color. */ -export function normalize(color, opt_color) { - const result = opt_color || []; - result[0] = clamp((color[0] + 0.5) | 0, 0, 255); - result[1] = clamp((color[1] + 0.5) | 0, 0, 255); - result[2] = clamp((color[2] + 0.5) | 0, 0, 255); - result[3] = clamp(color[3], 0, 1); - return result; +export function normalize(color) { + color[0] = clamp((color[0] + 0.5) | 0, 0, 255); + color[1] = clamp((color[1] + 0.5) | 0, 0, 255); + color[2] = clamp((color[2] + 0.5) | 0, 0, 255); + color[3] = clamp(color[3], 0, 1); + return color; } From 66d9ef872c6381e680a93bec1ea20166ec3f56a4 Mon Sep 17 00:00:00 2001 From: Frederic Junod Date: Tue, 16 Jan 2018 09:08:16 +0100 Subject: [PATCH 5/5] Fix color regexp to not match 7 hex digit --- src/ol/color.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ol/color.js b/src/ol/color.js index a4c4ef64fd..a6112d11b7 100644 --- a/src/ol/color.js +++ b/src/ol/color.js @@ -11,7 +11,7 @@ import {clamp} from './math.js'; * @type {RegExp} * @private */ -const HEX_COLOR_RE_ = /^#(?:[0-9a-f]{3,4}){1,2}$/i; +const HEX_COLOR_RE_ = /^#([a-f0-9]{3}|[a-f0-9]{4}(?:[a-f0-9]{2}){0,2})$/i; /**