Merge pull request #497 from NativeScript/feature/css-apply-crashes

Handle exceptions when setting CSS vlaues

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,33 @@ 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<viewModule.View>) {
+        TKUnit.assertEqual(30, testButton.style.fontSize);
+    }, invalidCSS);
+}
+
 // <snippet module="ui/styling" title="styling">
 // For information and example how to use style properties please refer to special [**Styling**](../../../styling.md) topic. 
 // </snippet>

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}$)|(^#[0-9A-F]{8}$)/i;
 
 export class Color implements definition.Color {
     private _a: number;
@@ -107,6 +108,22 @@ export class Color implements definition.Color {
         return value1.equals(value2);
     }
 
+    public static isValid(value: any): boolean {
+        if (types.isNullOrUndefined(value) || value instanceof Color) {
+            return true;
+        }
+
+        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);
     }

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 value can be converted to color.
+         * @param value Input string.
+         */
+        public static isValid(value: any): boolean;
     }
 }

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) => {
+            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);
 }

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",