From 4a6c841f0e358f17e98beac6ea5df8acf0808c75 Mon Sep 17 00:00:00 2001 From: vakrilov Date: Mon, 27 Jul 2015 16:29:24 +0300 Subject: [PATCH 1/5] Fix: crashes when applying css --- color/color-common.ts | 13 +++++++++++++ color/color.d.ts | 6 ++++++ ui/styling/css-selector.ts | 10 ++++++++-- ui/styling/style.ts | 6 +++--- 4 files changed, 30 insertions(+), 5 deletions(-) diff --git a/color/color-common.ts b/color/color-common.ts index 2772a5515..837af9b46 100644 --- a/color/color-common.ts +++ b/color/color-common.ts @@ -3,6 +3,7 @@ import types = require("utils/types"); import knownColors = require("color/known-colors"); var AMP = "#"; +var HEX_REGEX = /(^#[0-9A-F]{6}$)|(^#[0-9A-F]{3}$)/i; export class Color implements definition.Color { private _a: number; @@ -107,6 +108,18 @@ export class Color implements definition.Color { return value1.equals(value2); } + public static isValid(value: string): boolean { + if (!types.isString(value)) { + return false; + } + + if (knownColors.isKnownName(value)) { + return true; + } + + return HEX_REGEX.test(value); + } + private _buildHex(): string { return AMP + this._componentToHex(this._a) + this._componentToHex(this._r) + this._componentToHex(this._g) + this._componentToHex(this._b); } diff --git a/color/color.d.ts b/color/color.d.ts index 6e31ddd03..f97088317 100644 --- a/color/color.d.ts +++ b/color/color.d.ts @@ -68,5 +68,11 @@ declare module "color" { * @param value2 A Color to compare. */ public static equals(value1: Color, value2: Color): boolean; + + /** + * Validates if a string value can be converted to color. + * @param value Input string. + */ + public static isValid(value: string): boolean; } } \ No newline at end of file diff --git a/ui/styling/css-selector.ts b/ui/styling/css-selector.ts index 6027e6e68..c2dc05e63 100644 --- a/ui/styling/css-selector.ts +++ b/ui/styling/css-selector.ts @@ -2,6 +2,7 @@ import observable = require("ui/core/dependency-observable"); import cssParser = require("js-libs/reworkcss"); import styleProperty = require("ui/styling/style-property"); +import trace = require("trace"); var ID_SPECIFICITY = 10000; var CLASS_SPECIFICITY = 100; @@ -34,7 +35,12 @@ export class CssSelector { public apply(view: view.View) { this.eachSetter((property, value) => { - view.style._setValue(property, value, observable.ValueSource.Css); + try { + view.style._setValue(property, value, observable.ValueSource.Css); + } + catch (ex) { + trace.write("Error setting property: " + property.name + " view: " + view + " value: " + value + " " + ex, trace.categories.Style, trace.messageType.error); + } }); } @@ -177,7 +183,7 @@ class InlineStyleSelector extends CssSelector { } } -export function applyInlineSyle(view: view.View, declarations : cssParser.Declaration[]) { +export function applyInlineSyle(view: view.View, declarations: cssParser.Declaration[]) { var localStyleSelector = new InlineStyleSelector(declarations); localStyleSelector.apply(view); } \ No newline at end of file diff --git a/ui/styling/style.ts b/ui/styling/style.ts index e52f2d263..6b5b08375 100644 --- a/ui/styling/style.ts +++ b/ui/styling/style.ts @@ -455,14 +455,14 @@ function getHandlerInternal(propertyId: number, classInfo: types.ClassInfo): sty // Property registration export var colorProperty = new styleProperty.Property("color", "color", - new observable.PropertyMetadata(undefined, observable.PropertyMetadataSettings.Inheritable, undefined, undefined, color.Color.equals), + new observable.PropertyMetadata(undefined, observable.PropertyMetadataSettings.Inheritable, undefined, color.Color.isValid, color.Color.equals), converters.colorConverter); export var backgroundImageProperty = new styleProperty.Property("backgroundImage", "background-image", new observable.PropertyMetadata(undefined, observable.PropertyMetadataSettings.None, onBackgroundImagePropertyChanged)); export var backgroundColorProperty = new styleProperty.Property("backgroundColor", "background-color", - new observable.PropertyMetadata(undefined, observable.PropertyMetadataSettings.None, onBackgroundColorPropertyChanged, undefined, color.Color.equals), + new observable.PropertyMetadata(undefined, observable.PropertyMetadataSettings.None, onBackgroundColorPropertyChanged, color.Color.isValid, color.Color.equals), converters.colorConverter); export var backgroundRepeatProperty = new styleProperty.Property("backgroundRepeat", "background-repeat", @@ -475,7 +475,7 @@ export var backgroundPositionProperty = new styleProperty.Property("backgroundPo new observable.PropertyMetadata(undefined, observable.PropertyMetadataSettings.None, onBackgroundPositionPropertyChanged)); export var borderColorProperty = new styleProperty.Property("borderColor", "border-color", - new observable.PropertyMetadata(undefined, observable.PropertyMetadataSettings.None, undefined, undefined, color.Color.equals), + new observable.PropertyMetadata(undefined, observable.PropertyMetadataSettings.None, undefined, color.Color.isValid, color.Color.equals), converters.colorConverter); export var borderWidthProperty = new styleProperty.Property("borderWidth", "border-width", From 5902ab4981634091164d21c6888495f958048d21 Mon Sep 17 00:00:00 2001 From: vakrilov Date: Mon, 27 Jul 2015 16:52:35 +0300 Subject: [PATCH 2/5] Invalid CSS style tests --- apps/tests/ui/style/style-tests.ts | 34 +++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/apps/tests/ui/style/style-tests.ts b/apps/tests/ui/style/style-tests.ts index 9bc6ba6a9..29361141d 100644 --- a/apps/tests/ui/style/style-tests.ts +++ b/apps/tests/ui/style/style-tests.ts @@ -756,7 +756,7 @@ export function test_setStyle_throws() { }, "View.style property is read-only."); } -export var test_CSS_isAppliedOnPage = function () { +export function test_CSS_isAppliedOnPage() { var testButton = new buttonModule.Button(); testButton.text = "Test"; @@ -767,7 +767,7 @@ export var test_CSS_isAppliedOnPage = function () { }); } -export var test_CSS_isAppliedOnPage_From_Import = function () { +export function test_CSS_isAppliedOnPage_From_Import() { var testButton = new buttonModule.Button(); testButton.text = "Test"; @@ -778,7 +778,7 @@ export var test_CSS_isAppliedOnPage_From_Import = function () { }); } -export var test_CSS_isAppliedOnPage_From_addCssFile = function () { +export function test_CSS_isAppliedOnPage_From_addCssFile() { var testButton = new buttonModule.Button(); testButton.text = "Test"; @@ -789,6 +789,34 @@ export var test_CSS_isAppliedOnPage_From_addCssFile = function () { }); } +var invalidCSS = ".invalid { " + + "color: invalidValue; " + + "background-color: invalidValue; " + + "border-color: invalidValue; " + + "border-width: invalidValue; " + + "border-radius: invalidValue; " + + "font: invalidValue; " + + "font-style: invalidValue; " + + "font-weight: invalidValue; " + + "text-align: invalidValue; " + + "min-width: invalidValue; " + + "min-height: invalidValue; " + + "visibility: invalidValue; " + + "opacity: invalidValue; " + + "font-size: 30;" + // set one valid value to test it is applied +"}"; + +export function test_set_invalid_CSS_values_dont_cause_crash() { + var testButton = new buttonModule.Button(); + testButton.text = "Test"; + testButton.cssClass = "invalid"; + + helper.buildUIAndRunTest(testButton, function (views: Array) { + var page: pageModule.Page = views[1]; + TKUnit.assertEqual(30, testButton.style.fontSize); + }, invalidCSS); +} + // // For information and example how to use style properties please refer to special [**Styling**](../../../styling.md) topic. // From 312d11305925665181e9d3e64e0bb7ac1a52af72 Mon Sep 17 00:00:00 2001 From: vakrilov Date: Mon, 27 Jul 2015 17:09:51 +0300 Subject: [PATCH 3/5] Color validation extended --- color/color-common.ts | 8 ++++++-- color/color.d.ts | 4 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/color/color-common.ts b/color/color-common.ts index 837af9b46..fa7d56492 100644 --- a/color/color-common.ts +++ b/color/color-common.ts @@ -3,7 +3,7 @@ import types = require("utils/types"); import knownColors = require("color/known-colors"); var AMP = "#"; -var HEX_REGEX = /(^#[0-9A-F]{6}$)|(^#[0-9A-F]{3}$)/i; +var HEX_REGEX = /(^#[0-9A-F]{6}$)|(^#[0-9A-F]{3}$|(^#[0-9A-F]{8}$)/i; export class Color implements definition.Color { private _a: number; @@ -108,7 +108,11 @@ export class Color implements definition.Color { return value1.equals(value2); } - public static isValid(value: string): boolean { + public static isValid(value: any): boolean { + if (types.isNullOrUndefined(value) || value instanceof Color) { + return true; + } + if (!types.isString(value)) { return false; } diff --git a/color/color.d.ts b/color/color.d.ts index f97088317..5b97b5bfc 100644 --- a/color/color.d.ts +++ b/color/color.d.ts @@ -70,9 +70,9 @@ declare module "color" { public static equals(value1: Color, value2: Color): boolean; /** - * Validates if a string value can be converted to color. + * Validates if a value can be converted to color. * @param value Input string. */ - public static isValid(value: string): boolean; + public static isValid(value: any): boolean; } } \ No newline at end of file From d0ea1f30af94ca75344d70dcc4a4ddc16ce17ee3 Mon Sep 17 00:00:00 2001 From: vakrilov Date: Mon, 27 Jul 2015 18:02:43 +0300 Subject: [PATCH 4/5] Ts lint --- apps/tests/ui/style/style-tests.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/tests/ui/style/style-tests.ts b/apps/tests/ui/style/style-tests.ts index 29361141d..f389725f3 100644 --- a/apps/tests/ui/style/style-tests.ts +++ b/apps/tests/ui/style/style-tests.ts @@ -812,7 +812,6 @@ export function test_set_invalid_CSS_values_dont_cause_crash() { testButton.cssClass = "invalid"; helper.buildUIAndRunTest(testButton, function (views: Array) { - var page: pageModule.Page = views[1]; TKUnit.assertEqual(30, testButton.style.fontSize); }, invalidCSS); } From 2be36f7f6c73653285673b11a2f48cbaa0aca206 Mon Sep 17 00:00:00 2001 From: vakrilov Date: Mon, 27 Jul 2015 18:38:49 +0300 Subject: [PATCH 5/5] Typo fixed --- color/color-common.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/color/color-common.ts b/color/color-common.ts index fa7d56492..a7270eac5 100644 --- a/color/color-common.ts +++ b/color/color-common.ts @@ -3,7 +3,7 @@ import types = require("utils/types"); import knownColors = require("color/known-colors"); var AMP = "#"; -var HEX_REGEX = /(^#[0-9A-F]{6}$)|(^#[0-9A-F]{3}$|(^#[0-9A-F]{8}$)/i; +var HEX_REGEX = /(^#[0-9A-F]{6}$)|(^#[0-9A-F]{3}$)|(^#[0-9A-F]{8}$)/i; export class Color implements definition.Color { private _a: number;