More friendly Css parser error messages

CrossPlatformModules.csproj

@@ -514,6 +514,7 @@
     <Content Include="apps\gallery-app\layouts\dock-layout.xml" />
     <Content Include="apps\template-settings\app.css" />
     <Content Include="apps\tests\app\binding_tests.xml" />
+    <Content Include="apps\tests\ui\label\label-tests-wrong.css" />
     <Content Include="apps\ui-tests-app\pages\i86.xml" />
     <Content Include="apps\template-blank\app.css" />
     <Content Include="apps\template-hello-world\app.css" />
@@ -1474,7 +1475,7 @@
           <SaveServerSettingsInUserFile>False</SaveServerSettingsInUserFile>
         </WebProjectProperties>
       </FlavorProperties>
-      <UserProperties ui_2scroll-view_2package_1json__JSONSchema="http://json.schemastore.org/package" apps_2editable-text-demo_2package_1json__JSONSchema="http://json.schemastore.org/package" apps_2absolute-layout-demo_2package_1json__JSONSchema="http://json.schemastore.org/package" apps_2gallery-app_2package_1json__JSONSchema="http://json.schemastore.org/package" ui_2content-view_2package_1json__JSONSchema="http://json.schemastore.org/package" ui_2web-view_2package_1json__JSONSchema="http://json.schemastore.org/package" ui_2layouts_2linear-layout_2package_1json__JSONSchema="http://json.schemastore.org/package" ui_2layouts_2absolute-layout_2package_1json__JSONSchema="http://json.schemastore.org/package" ui_2layouts_2dock-layout_2package_1json__JSONSchema="" ui_2layouts_2grid-layout_2package_1json__JSONSchema="" ui_2layouts_2wrap-layout_2package_1json__JSONSchema="http://json.schemastore.org/package" />
+      <UserProperties ui_2layouts_2wrap-layout_2package_1json__JSONSchema="http://json.schemastore.org/package" ui_2layouts_2grid-layout_2package_1json__JSONSchema="" ui_2layouts_2dock-layout_2package_1json__JSONSchema="" ui_2layouts_2absolute-layout_2package_1json__JSONSchema="http://json.schemastore.org/package" ui_2layouts_2linear-layout_2package_1json__JSONSchema="http://json.schemastore.org/package" ui_2web-view_2package_1json__JSONSchema="http://json.schemastore.org/package" ui_2content-view_2package_1json__JSONSchema="http://json.schemastore.org/package" apps_2gallery-app_2package_1json__JSONSchema="http://json.schemastore.org/package" apps_2absolute-layout-demo_2package_1json__JSONSchema="http://json.schemastore.org/package" apps_2editable-text-demo_2package_1json__JSONSchema="http://json.schemastore.org/package" ui_2scroll-view_2package_1json__JSONSchema="http://json.schemastore.org/package" />
     </VisualStudio>
   </ProjectExtensions>
 </Project>

application/application-common.ts

@@ -1,6 +1,5 @@
 require("globals");
 import definition = require("application");
-import cssParser = require("js-libs/reworkcss");
 import fs = require("file-system");
 import fileSystemAccess = require("file-system/file-system-access");
 import styleScope = require("ui/styling/style-scope");
@@ -28,10 +27,8 @@ export function loadCss() {
         var cssFileName = fs.path.join(fs.knownFolders.currentApp().path, definition.cssFile);
         var applicationCss;
         if (fs.File.exists(cssFileName)) {
-            // Read the CSS file.
             new fileSystemAccess.FileSystemAccess().readText(cssFileName, r => { applicationCss = r; });
-            var applicationCssSyntaxTree = cssParser.parse(applicationCss, undefined);
+            definition.cssSelectorsCache = styleScope.StyleScope.createSelectorsFromCss(applicationCss, cssFileName);
-            definition.cssSelectorsCache = styleScope.StyleScope.createSelectorsFromSyntaxTree(applicationCssSyntaxTree);
         }
     }
 }

apps/tests/app/mainPage.ts

@@ -20,7 +20,7 @@ class MyTraceWriter implements trace.TraceWriter {
 
 trace.addWriter(new MyTraceWriter());
 trace.enable();
-trace.setCategories(trace.categories.Test);
+trace.setCategories(trace.categories.Test + "," + trace.categories.Error);
 
 var textView = new textViewModule.TextView();
 textView.editable = false;

apps/tests/ui/label/label-tests-wrong.css

@@ -0,0 +1,4 @@
+label {
+<!--Test wrong comment-->
+    background-color: red;
+}

apps/tests/ui/label/label-tests.ts

@@ -25,6 +25,25 @@ import page = require("ui/page");
 import textBase = require("ui/text-base");
 import enums = require("ui/enums");
 import labelTestsNative = require("./label-tests-native");
+import trace = require("trace");
+
+var errorMessage;
+var errorTraceWriter = {
+    write: function (message, category, messageType) {
+        if (category === trace.categories.Error) {
+            errorMessage = message;
+        }
+    }
+}
+
+export var setUp = function () {
+    trace.addWriter(errorTraceWriter);
+}
+
+export var tearDown = function () {
+    trace.removeWriter(errorTraceWriter);
+    errorMessage = undefined;
+}
 
 export var test_Label_Members = function () {
     var label = new LabelModule.Label();
@@ -487,3 +506,28 @@ export var testNativeTextAlignmentFromLocal = function () {
         TKUnit.assert(actualResult === expectedTextAlignment, "Actual: " + actualResult + "; Expected: " + expectedTextAlignment);
     });
 }
+
+export var testErrorMessageWhenWrongCssIsAddedWithFile = function () {
+    helper.buildUIAndRunTest(_createLabelFunc(), function (views: Array<view.View>) {
+        var view = <LabelModule.Label>views[0];
+        view.id = "testLabel";
+        var page = <page.Page>views[1];
+        errorMessage = undefined;   
+        page.addCssFile("/app/tests/ui/label/label-tests-wrong.css");
+
+        TKUnit.assertNotEqual(errorMessage, undefined);
+    });
+    
+}
+
+export var testErrorMessageWhenWrongCssIsAdded = function () {
+    helper.buildUIAndRunTest(_createLabelFunc(), function (views: Array<view.View>) {
+        var view = <LabelModule.Label>views[0];
+        view.id = "testLabel";
+        var page = <page.Page>views[1];
+        errorMessage = undefined;
+        page.addCss("label { < !--Test wrong comment-- > background-color: red; }");
+
+        TKUnit.assertNotEqual(errorMessage, undefined);
+    });
+}

js-libs/reworkcss/reworkcss.js

@@ -57,17 +57,25 @@ module.exports.parse = function(css, options){
    */
 
   function error(msg) {
-    if (options.silent === true) {
+      if (options.silent === true) {
-      return false;
+          return false;
-    }
+      };
 
-    var err = new Error(options.source + ':' + lineno + ':' + column + ': ' + msg);
+      var errorMessage;
-    err.reason = msg;
+      if (options.source) {
-    err.filename = options.source;
+          errorMessage = options.source + ':' + lineno + ':' + column + ': ' + msg;
-    err.line = lineno;
+      }
-    err.column = column;
+      else {
-    err.source = css;
+          errorMessage = "Parsing '" + css + "' issue an error: " + msg;
-    throw err;
+      }
+
+      var err = new Error(errorMessage);
+      err.reason = msg;
+      err.filename = options.source;
+      err.line = lineno;
+      err.column = column;
+      err.source = css;
+      throw err;
   }
 
   /**

trace/trace.d.ts

@@ -68,6 +68,7 @@ declare module "trace" {
         export var Navigation: string;
         export var Test: string;
         export var Binding: string;
+        export var Error: string;
 
         export var All: string;
 

trace/trace.ts

@@ -107,7 +107,8 @@ export module categories {
     export var Navigation = "Navigation";
     export var Test = "Test";
     export var Binding = "Binding";
-    export var All = VisualTreeEvents + "," + Layout + "," + Style + "," + ViewHierarchy + "," + NativeLifecycle + "," + Debug + "," + Navigation + "," + Test + "," + Binding;
+    export var Error = "Error";
+    export var All = VisualTreeEvents + "," + Layout + "," + Style + "," + ViewHierarchy + "," + NativeLifecycle + "," + Debug + "," + Navigation + "," + Test + "," + Binding + "," + Error;
 
     export var separator = ",";
 

ui/page/page-common.ts

@@ -5,7 +5,6 @@ import frame = require("ui/frame");
 import styleScope = require("ui/styling/style-scope");
 import fs = require("file-system");
 import fileSystemAccess = require("file-system/file-system-access");
-import trace = require("trace");
 
 export module knownEvents {
     export var navigatedTo = "navigatedTo";
@@ -53,7 +52,11 @@ export class Page extends contentView.ContentView implements dts.Page {
     }
 
     public addCss(cssString: string): void {
-        this._styleScope.addCss(cssString);
+        this._addCssInternal(cssString, undefined);
+    }
+
+    private _addCssInternal(cssString: string, cssFileName: string): void {
+        this._styleScope.addCss(cssString, cssFileName);
         this._refreshCss();
     }
 
@@ -62,7 +65,7 @@ export class Page extends contentView.ContentView implements dts.Page {
         var realCssFileName = fs.path.join(fs.knownFolders.currentApp().path, cssFileName);
         if (fs.File.exists(realCssFileName)) {
             new fileSystemAccess.FileSystemAccess().readText(realCssFileName, r => { cssString = r; });
-            this.addCss(cssString);
+            this._addCssInternal(cssString, cssFileName);
         }
     }
 
@@ -101,22 +104,18 @@ export class Page extends contentView.ContentView implements dts.Page {
             return;
         }
 
-        try {
+        this._styleScope.ensureSelectors();
-            this._styleScope.ensureSelectors();
 
-            var scope = this._styleScope;
+        var scope = this._styleScope;
-            var checkSelectors = (view: view.View): boolean => {
+        var checkSelectors = (view: view.View): boolean => {
-                scope.applySelectors(view);
+            scope.applySelectors(view);
-                return true;
+            return true;
-            }
-
-            checkSelectors(this);
-            view.eachDescendant(this, checkSelectors);
-
-            this._cssApplied = true;
-        } catch (e) {
-            trace.write("Css styling failed: " + e, trace.categories.Style);
         }
+
+        checkSelectors(this);
+        view.eachDescendant(this, checkSelectors);
+
+        this._cssApplied = true;
     }
 
     private _resetCssValues() {

ui/styling/style-scope.ts

@@ -12,6 +12,7 @@ export class StyleScope {
     private _viewIdToKey = {};
 
     private _css: string;
+    private _cssFileName: string;
     private _cssSelectors: Array<cssSelector.CssSelector>;
 
     get css(): string {
@@ -19,33 +20,44 @@ export class StyleScope {
     }
     set css(value: string) {
         this._css = value;
+        this._cssFileName = undefined;
         this._cssSelectors = undefined;
         this._reset();
     }
 
-    public addCss(cssString: string): void {
+    public addCss(cssString: string, cssFileName: string): void {
         if (this._css === undefined) {
             this._css = cssString;
         }
         else {
             this._css += cssString;
         }
+        this._cssFileName = cssFileName;
         this._reset();
         if (this._cssSelectors) {
-            var addedCssTree = cssParser.parse(cssString, undefined);
+            var addedSelectors = StyleScope.createSelectorsFromCss(cssString, cssFileName);
-            var addedSelectors = StyleScope.createSelectorsFromSyntaxTree(addedCssTree);
             this._cssSelectors = this._joinCssSelectorsArrays([this._cssSelectors, addedSelectors]);
         }
     }
 
+    public static createSelectorsFromCss(css: string, cssFileName: string): cssSelector.CssSelector[] {
+        try {
+            var pageCssSyntaxTree = css ? cssParser.parse(css, { source: cssFileName }) : null;
+            var pageCssSelectors;
+            if (pageCssSyntaxTree) {
+                pageCssSelectors = StyleScope.createSelectorsFromSyntaxTree(pageCssSyntaxTree);
+            }
+            return pageCssSelectors;
+        }
+        catch (e) {
+            trace.write("Css styling failed: " + e, trace.categories.Error, trace.messageType.error);
+        }
+    }
+
     public ensureSelectors() {
         if (!this._cssSelectors && (this._css || application.cssSelectorsCache)) {
             var applicationCssSelectors = application.cssSelectorsCache ? application.cssSelectorsCache : null;
-            var pageCssSyntaxTree = this._css ? cssParser.parse(this._css, undefined) : null;
+            var pageCssSelectors = StyleScope.createSelectorsFromCss(this._css, this._cssFileName);
-            var pageCssSelectors;
-            if (pageCssSyntaxTree) {
-                pageCssSelectors = StyleScope.createSelectorsFromSyntaxTree(pageCssSyntaxTree);
-            }
             this._cssSelectors = this._joinCssSelectorsArrays([applicationCssSelectors, pageCssSelectors]);
         }
     }
@@ -131,7 +143,7 @@ export class StyleScope {
         }
     }
 
-    public static createSelectorsFromSyntaxTree(ast: cssParser.SyntaxTree): Array<cssSelector.CssSelector> {
+    private static createSelectorsFromSyntaxTree(ast: cssParser.SyntaxTree): Array<cssSelector.CssSelector> {
         var result: Array<cssSelector.CssSelector> = [];
 
         var rules = ast.stylesheet.rules;
@@ -168,6 +180,6 @@ export function applyInlineSyle(view: view.View, style: string) {
         var filteredDeclarations = syntaxTree.stylesheet.rules[0].declarations.filter((val, i, arr) => { return val.type === "declaration" });
         cssSelector.applyInlineSyle(view, filteredDeclarations);
     } catch (ex) {
-        trace.write("Applying local style failed: " + ex, trace.categories.Style);
+        trace.write("Applying local style failed: " + ex, trace.categories.Error, trace.messageType.error);
     }
 }