Merge pull request #358 from adobe/pflynn/inline-editor

More CSS parsing unit tests (and beef up EditorManager docs)

src/EditorManager.js

@@ -319,10 +319,12 @@ define(function (require, exports, module) {
      * Creates a new inline CodeMirror editor instance containing the given text. The editor's mode
      * is set based on the given filename's extension (the actual file on disk is never examined).
      * The editor is not yet visible.
-     * @param {CodeMirror} hostEditor
-     * @param {string} text
-     * @param {?{startLine:Number, endLine:Number}} range
-     * @param {string} fileNameToSelectMode
+     * @param {!CodeMirror} hostEditor  Outer CodeMirror instance that inline editor will sit within.
+     * @param {!string} text  The text content of the editor.
+     * @param {?{startLine:Number, endLine:Number}} range  If specified, all lines outside the given
+     *      range are hidden from the editor. Range is inclusive. Line numbers start at 0.
+     * @param {!string} fileNameToSelectMode  A filename (optionally including path) from which to
+     *      infer the editor's mode.
      */
     function createInlineEditorFromText(hostEditor, text, range, fileNameToSelectMode) {
         // Container to hold editor & render its stylized frame

test/spec/CSSManager-test.js

@@ -352,7 +352,8 @@ define(function (require, exports, module) {
     describe("CSS Parsing: ", function () {
 
         var manager,
-            match;
+            match,
+            expectParseError;
 
         function findMatchingRules(tagInfo) {
             if (tagInfo) {
@@ -393,10 +394,32 @@ define(function (require, exports, module) {
             return findMatchingRules(tagInfo);
         }
 
+
+        /**
+         * Test helper function: expects CSS parsing to fail at the given 0-based offset within the
+         * cssCode string, with the given error message.
+         */
+        var _expectParseError = function (cssCode, expectedCodeOffset, expectedErrorMessage) {
+            try {
+                manager = new CSSManager._CSSManager();
+                manager._loadString(cssCode);
+
+                // shouldn't get here since _loadString() is expected to throw
+                this.fail("Expected parse error: " + cssCode);
+
+            } catch (error) {
+                expect(error.index).toBe(expectedCodeOffset);
+                expect(error.message).toBe(expectedErrorMessage);
+            }
+        };
+
+        /** To call fail(), these helpers need access to the value of 'this' inside each it() */
         beforeEach(function () {
             match = _match.bind(this);
+            expectParseError = _expectParseError.bind(this);
         });
 
+
         describe("Simple selectors: ", function () {
 
             it("should match a lone type selector given a type", function () {
@@ -692,6 +715,15 @@ define(function (require, exports, module) {
                 result = matchAgain({ tag: "h4" });
                 expect(result.length).toBe(0);
 
+                // Quotes AND braces nested inside string
+                // TODO (issue #343): these should get parsed correctly
+                expectParseError("div::after { content: \"\\\"}\\\"\"; }", 13, "Syntax Error on line 1");
+                expectParseError("div::after { content: \"\\\"{\"; }", undefined, "Missing closing `}`");
+
+                css = "@import \"null?\\\"{\"; \n" +   // this is a real-world CSS hack that is a case of the above
+                      "div { color: red }";
+                expectParseError(css, undefined, "Missing closing `}`");
+
                 // Newline inside string (escaped)
                 result = match("li::before { content: 'foo\\nbar'; } \n div { color:red }", { tag: "div" });
                 expect(result.length).toBe(1);
@@ -950,19 +982,41 @@ define(function (require, exports, module) {
         // The following tests expect "failures" in order to pass. They
         // will be updated once the associated issues are fixed.
         describe("Known Issues", function () {
+
             // TODO (issue #332): ParseError for double semi-colon
             it("should handle an empty declaration (extra semi-colon)", function () {
-                try {
-                    manager = new CSSManager._CSSManager();
-                    manager._loadString("h4 { color:red;; }");
-                } catch (error) {
-                    expect(error.index).toBe(15);
-                    expect(error.message).toBe("Syntax Error on line 1");
-                    return;
-                }
-
-                this.fail("Known issue #332");
+                expectParseError("h4 { color:red;; }", 15, "Syntax Error on line 1");
+                expectParseError("div{; color:red}", 4, "Syntax Error on line 1");
+            });
+
+            // TODO (issue #338): ParseError for various IE filter syntaxes
+            it("should handle IE filter syntaxes", function () {
+                expectParseError("div{opacity:0; filter:alpha(opacity = 0)}", 15, "Syntax Error on line 1");
+                expectParseError("div { filter:alpha(opacity = 0) }", 6, "Syntax Error on line 1");
+                expectParseError("div { filter:progid:DXImageTransform.Microsoft.Gradient(GradientType=0,StartColorStr='#92CBE0',EndColorStr='#6B9EBC'); }",
+                    6, "Syntax Error on line 1");
+            });
+
+            // TODO (issue #343): Inline editor has trouble with CSS Hacks
+            it("should handle unnecessary escape codes", function () {
+                expectParseError("div { f\\loat: left; }", 6, "Syntax Error on line 1");
             });
+
+            // TODO (issue #343): Inline editor has trouble with CSS Hacks
+            it("should handle comments within properties", function () {
+                expectParseError("div { display/**/: block; }", 6, "Syntax Error on line 1");
+
+                // Related cases that DO work already:
+                match("div/**/ { display: block; }");
+                match("div /**/{ display: block; }");
+                match("div {/**/ display: block; }");
+                match("div { /**/display: block; }");
+                match("div { display:/**/ block; }");
+                match("div { display: /**/block; }");
+                match("div { display: block/**/; }");
+                match("div { display: block /**/; }");
+            });
+
         }); // describe("Known Issues")