Merge pull request #13270 from adobe/swmitra/StyleAttributeHints

zaggino · web-flow · commit 387731bf16fc · 2017-04-13T14:44:00.000+10:00
CSS code hints in 'style' attribute value context
diff --git a/src/editor/Editor.js b/src/editor/Editor.js
@@ -75,6 +75,7 @@ define(function (require, exports, module) {
         TextRange          = require("document/TextRange").TextRange,
         TokenUtils         = require("utils/TokenUtils"),
         ValidationUtils    = require("utils/ValidationUtils"),
+        HTMLUtils          = require("language/HTMLUtils"),
         ViewUtils          = require("utils/ViewUtils"),
         MainViewManager    = require("view/MainViewManager"),
         _                  = require("thirdparty/lodash");
@@ -2209,6 +2210,19 @@ define(function (require, exports, module) {
             isMixed     = (outerMode.name !== startMode.name);
 
         if (isMixed) {
+            // This is the magic code to let the code view know that we are in 'css' context
+            // if the CodeMirror outermode is 'htmlmixed' and we are in 'style' attributes
+            // value context. This has to be done as CodeMirror doesn't yet think this as 'css'
+            // This magic is executed only when user is having a cursor and not selection
+            // We will enable selection handling one we figure a way out to handle mixed scope selection
+            if (outerMode.name === 'htmlmixed' && primarySel.start.line === primarySel.end.line && primarySel.start.ch === primarySel.end.ch) {
+                var tagInfo = HTMLUtils.getTagInfo(this, primarySel.start, true),
+                    tokenType = tagInfo.position.tokenType;
+ 
+                if (tokenType === HTMLUtils.ATTR_VALUE && tagInfo.attr.name.toLowerCase() === 'style') {
+                    return 'css';
+                }
+            }
             // Shortcut the first check to avoid getModeAt(), which can be expensive
             if (primarySel.start.line !== primarySel.end.line || primarySel.start.ch !== primarySel.end.ch) {
                 var endMode = TokenUtils.getModeAt(this._codeMirror, primarySel.end);
diff --git a/src/extensions/default/CSSCodeHints/unittests.js b/src/extensions/default/CSSCodeHints/unittests.js
@@ -51,6 +51,21 @@ define(function (require, exports, module) {
                              " bordborder: \n" +
                              " color\n" +
                              "} \n";
+                             
+        var defaultHTMLContent = "<html> \n" +
+                                 "<head> \n" +
+                                 "</head> \n" +
+                                 "<body> \n" +
+                                 "<div style=' \n" + // line 4
+                                 " \n" +
+                                 " b\n" +
+                                 " bord\n" +
+                                 " border-\n" +
+                                 " border-colo\n" +
+                                 " border-color: red;'>\n" + // line 10
+                                 "</div> \n" +
+                                 "</body> \n" +
+                                 "</html> \n";
 
         var testDocument, testEditor;
 
@@ -437,6 +452,92 @@ define(function (require, exports, module) {
             });
 
         });
+        
+        describe("CSS Hint provider in style attribute value context for html mode", function () {
+
+            beforeEach(function () {
+                // create Editor instance (containing a CodeMirror instance)
+                var mock = SpecRunnerUtils.createMockEditor(defaultHTMLContent, "html");
+                testEditor = mock.editor;
+                testDocument = mock.doc;
+            });
+
+            afterEach(function () {
+                SpecRunnerUtils.destroyMockEditor(testDocument);
+                testEditor = null;
+                testDocument = null;
+            });
+            
+            it("should list all prop-name hints right after the open quote for style value context", function () {
+                testEditor.setCursorPos({ line: 4, ch: 12 });    // after "='"
+                var hintList = expectHints(CSSCodeHints.cssPropHintProvider);
+                verifyAttrHints(hintList, "align-content");  // filtered on "empty string"
+            });
+
+            it("should list all prop-name hints in new line for style value context", function () {
+                testEditor.setCursorPos({ line: 5, ch: 0 });
+
+                var hintList = expectHints(CSSCodeHints.cssPropHintProvider);
+                verifyAttrHints(hintList, "align-content");  // filtered on "empty string"
+            });
+
+            it("should list all prop-name hints starting with 'b' in new line for style value context", function () {
+                testEditor.setCursorPos({ line: 6, ch: 2 });
+
+                var hintList = expectHints(CSSCodeHints.cssPropHintProvider);
+                verifyAttrHints(hintList, "backface-visibility");  // filtered on "b"
+            });
+
+            it("should list all prop-name hints starting with 'bord' for style value context", function () {
+                // insert semicolon after previous rule to avoid incorrect tokenizing
+                testDocument.replaceRange(";", { line: 6, ch: 2 });
+
+                testEditor.setCursorPos({ line: 7, ch: 5 });
+                var hintList = expectHints(CSSCodeHints.cssPropHintProvider);
+                verifyAttrHints(hintList, "border");  // filtered on "bord"
+            });
+
+            it("should list all prop-name hints starting with 'border-' for style value context", function () {
+                // insert semicolon after previous rule to avoid incorrect tokenizing
+                testDocument.replaceRange(";", { line: 7, ch: 5 });
+
+                testEditor.setCursorPos({ line: 8, ch: 8 });
+                var hintList = expectHints(CSSCodeHints.cssPropHintProvider);
+                verifyAttrHints(hintList, "border-bottom");  // filtered on "border-"
+            });
+
+            it("should list only prop-name hint border-color for style value context", function () {
+                // insert semicolon after previous rule to avoid incorrect tokenizing
+                testDocument.replaceRange(";", { line: 8, ch: 8 });
+
+                testEditor.setCursorPos({ line: 9, ch: 12 });
+                var hintList = expectHints(CSSCodeHints.cssPropHintProvider);
+                verifyAttrHints(hintList, "border-color");  // filtered on "border-color"
+                verifyListsAreIdentical(hintList, ["border-color",
+                                                   "border-left-color",
+                                                   "border-top-color",
+                                                   "border-bottom-color",
+                                                   "border-right-color"]);
+            });
+
+            it("should list prop-name hints at end of property-value finished by ; for style value context", function () {
+                testEditor.setCursorPos({ line: 10, ch: 19 });    // after ;
+                var hintList = expectHints(CSSCodeHints.cssPropHintProvider);
+                verifyAttrHints(hintList, "align-content");  // filtered on "empty string"
+            });
+
+            it("should NOT list prop-name hints right before style value context", function () {
+                testEditor.setCursorPos({ line: 4, ch: 11 });    // after =
+                expectNoHints(CSSCodeHints.cssPropHintProvider);
+            });
+
+            it("should NOT list prop-name hints after style value context", function () {
+                testEditor.setCursorPos({ line: 10, ch: 20 });    // after "'"
+                expectNoHints(CSSCodeHints.cssPropHintProvider);
+            });
+            
+        });
+
 
         describe("CSS hint provider in other filecontext (e.g. javascript)", function () {
             var defaultContent = "function foobar (args) { \n " +
diff --git a/src/language/CSSUtils.js b/src/language/CSSUtils.js
@@ -90,8 +90,13 @@ define(function (require, exports, module) {
             return null;
         }
         var state = ctx.token.state.localState || ctx.token.state;
-        if (!state.context && ctx.token.state.html && ctx.token.state.html.localState) {
-            state = ctx.token.state.html.localState;
+        // if state contains a valid html inner state use that first
+        if (state.htmlState) {
+            state = ctx.token.state.htmlState;
+        } else {
+            if (!state.context && ctx.token.state.html && ctx.token.state.html.localState) {
+                state = ctx.token.state.html.localState;
+            }
         }
         return state;
     }
@@ -435,11 +440,13 @@ define(function (require, exports, module) {
      *           range: {start: {line: number, ch: number},
      *                   end: {line: number, ch: number}}}} A CSS context info object.
      */
-    function _getRuleInfoStartingFromPropValue(ctx, editor) {
-        var propNamePos = $.extend({}, ctx.pos),
+    function _getRuleInfoStartingFromPropValue(ctx, editorParam) {
+        var editor      = editorParam._codeMirror || editorParam,
+            contextDoc  = editor.document || editor.doc,
+            propNamePos = $.extend({}, ctx.pos),
             backwardPos = $.extend({}, ctx.pos),
             forwardPos  = $.extend({}, ctx.pos),
-            propNameCtx = TokenUtils.getInitialContext(editor._codeMirror, propNamePos),
+            propNameCtx = TokenUtils.getInitialContext(editor, propNamePos),
             backwardCtx,
             forwardCtx,
             lastValue = "",
@@ -448,7 +455,7 @@ define(function (require, exports, module) {
             offset = TokenUtils.offsetInToken(ctx),
             canAddNewOne = false,
             testPos = {ch: ctx.pos.ch + 1, line: ctx.pos.line},
-            testToken = editor._codeMirror.getTokenAt(testPos, true),
+            testToken = editor.getTokenAt(testPos, true),
             propName,
             range;
 
@@ -460,7 +467,7 @@ define(function (require, exports, module) {
         }
 
         // Scan backward to collect all preceding property values
-        backwardCtx = TokenUtils.getInitialContext(editor._codeMirror, backwardPos);
+        backwardCtx = TokenUtils.getInitialContext(editor, backwardPos);
         propValues = _getPrecedingPropValues(backwardCtx);
 
         lastValue = "";
@@ -492,14 +499,14 @@ define(function (require, exports, module) {
             offset = 0;
 
             // If pos is at EOL, then there's implied whitespace (newline).
-            if (editor.document.getLine(ctx.pos.line).length > ctx.pos.ch  &&
+            if (contextDoc.getLine(ctx.pos.line).length > ctx.pos.ch  &&
                     (testToken.string.length === 0 || testToken.string.match(/\S/))) {
                 canAddNewOne = false;
             }
         }
 
         // Scan forward to collect all succeeding property values and append to all propValues.
-        forwardCtx = TokenUtils.getInitialContext(editor._codeMirror, forwardPos);
+        forwardCtx = TokenUtils.getInitialContext(editor, forwardPos);
         propValues = propValues.concat(_getSucceedingPropValues(forwardCtx, lastValue));
 
         if (propValues.length) {
@@ -613,16 +620,34 @@ define(function (require, exports, module) {
             return createInfo();
         }
 
+        // Context from the current editor will have htmlState if we are in css mode
+        // and in attribute value state of a tag with attribute name style
+        if (ctx.token.state.htmlState && (!ctx.token.state.localMode || ctx.token.state.localMode.name !== "css")) {
+
+            // tagInfo is required to aquire the style attr value
+            var tagInfo = HTMLUtils.getTagInfo(editor, pos, true),
+                // To be used as relative character position
+                offset = tagInfo.position.offset;
+
+            /**
+             * We will use this CM to cook css context in case of style attribute value
+             * as CM in htmlmixed mode doesn't yet identify this as css context. We provide
+             * a no-op display function to run CM without a DOM head.
+             */
+            var _contextCM = new CodeMirror(function () {}, {
+                value: "{" + tagInfo.attr.value.replace(/(^")|("$)/g, ""),
+                mode:  "css"
+            });
+
+            ctx = TokenUtils.getInitialContext(_contextCM, {line: 0, ch: offset + 1});
+        }
+        
         if (_isInPropName(ctx)) {
-            return _getPropNameInfo(ctx, editor);
+            return _getPropNameInfo(ctx, ctx.editor);
         }
-
+        
         if (_isInPropValue(ctx)) {
-            return _getRuleInfoStartingFromPropValue(ctx, editor);
-        }
-
-        if (_isInAtRule(ctx)) {
-            return _getImportUrlInfo(ctx, editor);
+            return _getRuleInfoStartingFromPropValue(ctx, ctx.editor);
         }
 
         return createInfo();
diff --git a/src/language/HTMLUtils.js b/src/language/HTMLUtils.js
@@ -226,6 +226,18 @@ define(function (require, exports, module) {
                 return createTagInfo();
             }
         }
+        
+        //Skip all the 'string' tokens backwards. Required to reach to the first line 
+        //of multiline HTML attribute value.
+        while (TokenUtils.moveSkippingWhitespace(TokenUtils.movePrevToken, ctx)) {
+            if (ctx.token.type !== "string") {
+                break;
+            }
+        }
+
+        //As we have skipped all the string tokens, make a forward navigation to move to the
+        //first 'string token so that in next backward navigation we can find '='.
+        TokenUtils.moveSkippingWhitespace(TokenUtils.moveNextToken, ctx);
 
         //Move to the prev token, and check if it's "="
         if (!TokenUtils.moveSkippingWhitespace(TokenUtils.movePrevToken, ctx) || ctx.token.string !== "=") {
@@ -299,13 +311,14 @@ define(function (require, exports, module) {
      *      className:tag       string:"></span>"
      * @param {Editor} editor An instance of a Brackets editor
      * @param {{ch: number, line: number}} constPos  A CM pos (likely from editor.getCursorPos())
+     * @param {isHtmlMode:boolean} let the module know we are in html mode
      * @return {{tagName:string,
      *           attr:{name:string, value:string, valueAssigned:boolean, quoteChar:string, hasEndQuote:boolean},
      *           position:{tokenType:string, offset:number}
      *         }}
      *         A tagInfo object with some context about the current tag hint.
      */
-    function getTagInfo(editor, constPos) {
+    function getTagInfo(editor, constPos, isHtmlMode) {
         // We're going to be changing pos a lot, but we don't want to mess up
         // the pos the caller passed in so we use extend to make a safe copy of it.
         var pos = $.extend({}, constPos),
@@ -314,8 +327,8 @@ define(function (require, exports, module) {
             tagInfo,
             tokenType;
 
-        // Check if this is inside a style block.
-        if (editor.getModeForSelection() !== "html") {
+        // Check if this is not known to be in html mode and inside a style block.
+        if (!isHtmlMode && editor.getModeForSelection() !== "html") {
             return createTagInfo();
         }
 
