Merge pull request #7185 from adobe/tom/issue-7093

RaymondLim · RaymondLim · commit 8971aedafd32 · 2014-03-14T16:07:17.000-07:00
Set the font size before opening the documents
diff --git a/src/brackets.js b/src/brackets.js
@@ -100,6 +100,7 @@ define(function (require, exports, module) {
         ColorUtils              = require("utils/ColorUtils"),
         CodeInspection          = require("language/CodeInspection"),
         NativeApp               = require("utils/NativeApp"),
+        ViewCommandHandlers     = require("view/ViewCommandHandlers"),
         _                       = require("thirdparty/lodash");
         
     // Load modules that self-register and just need to get included in the main project
@@ -108,7 +109,6 @@ define(function (require, exports, module) {
     require("editor/EditorStatusBar");
     require("editor/EditorCommandHandlers");
     require("editor/EditorOptionHandlers");
-    require("view/ViewCommandHandlers");
     require("help/HelpCommandHandlers");
     require("search/FindInFiles");
     require("search/FindReplace");
@@ -221,6 +221,7 @@ define(function (require, exports, module) {
             // Load the initial project after extensions have loaded
             extensionLoaderPromise.always(function () {
                 // Finish UI initialization
+                ViewCommandHandlers.restoreFontSize();
                 var initialProjectPath = ProjectManager.getInitialProjectPath();
                 ProjectManager.openProject(initialProjectPath).always(function () {
                     _initTest();
diff --git a/src/preferences/PreferenceStorage.js b/src/preferences/PreferenceStorage.js
@@ -218,6 +218,13 @@ define(function (require, exports, module) {
             var rule = rules[key];
             if (!rule && prefCheckCallback) {
                 rule = prefCheckCallback(key);
+            } else if (prefCheckCallback) {
+                // Check whether we have a new preference key-value pair
+                // for an old preference.
+                var newRule = prefCheckCallback(key, prefs[key]);
+                if (newRule) {
+                    rule = _.cloneDeep(newRule);
+                }
             }
             if (!rule) {
                 console.warn("Preferences conversion for ", self._clientID, " has no rule for", key);
@@ -238,6 +245,11 @@ define(function (require, exports, module) {
                     manager.set(newKey, prefs[key], options);
                     convertedKeys.push(key);
                 }
+            } else if (_.isObject(rule)) {
+                Object.keys(rule).forEach(function (ruleKey) {
+                    manager.set(ruleKey, rule[ruleKey]);
+                });
+                convertedKeys.push(key);
             } else {
                 complete = false;
             }
diff --git a/src/styles/brackets_theme_default.less b/src/styles/brackets_theme_default.less
@@ -144,10 +144,8 @@
  */
 .code-font() {
     color: @content-color;
-    // line-height must be specified in px not em because the code font and line number font sizes are different.
-    // Sizing via em will cause the code and line numbers to misalign
-    line-height: 15px;
     font-size: 12px;
+    line-height: 1.25;
     font-family: "SourceCodePro-Medium", "ＭＳ ゴシック", "MS Gothic", monospace;
 }
 
diff --git a/src/view/ViewCommandHandlers.js b/src/view/ViewCommandHandlers.js
@@ -30,8 +30,6 @@
  *          Increase Font Size, Decrease Font Size, or Restore Font Size commands.
  *          The 2nd arg to the listener is the amount of the change. The 3rd arg
  *          is a string containing the new font size after applying the change.
- *          The 4th arg is a string containing the new line height after applying
- *          the change.
  */
 
 define(function (require, exports, module) {
@@ -58,31 +56,25 @@ define(function (require, exports, module) {
      * @const
      * @private
      * The smallest font size in pixels
-     * @type {int}
+     * @type {number}
      */
     var MIN_FONT_SIZE = 1;
     
     /**
      * @const
      * @private
      * The largest font size in pixels
-     * @type {int}
+     * @type {number}
      */
     var MAX_FONT_SIZE = 72;
     
     /**
      * @const
      * @private
-     * The ratio of line-height to font-size when they use the same units
-     * @type {float}
+     * The default font size used only to convert the old fontSizeAdjustment view state to the new fontSizeStyle
+     * @type {number}
      */
-    var LINE_HEIGHT = 1.25;
-    
-    /**
-     * @private
-     * @type {boolean}
-     */
-    var _fontSizePrefsLoaded = false;
+    var DEFAULT_FONT_SIZE = 12;
     
     
     /**
@@ -93,112 +85,92 @@ define(function (require, exports, module) {
         $("#" + DYNAMIC_FONT_STYLE_ID).remove();
     }
     
+    /**
+     * @private
+     * Add the styles used to update the font size
+     * @param {string} fontSizeStyle  A string with the font size and the size unit
+     */
+    function _addDynamicFontSize(fontSizeStyle) {
+        var style = $("<style type='text/css'></style>").attr("id", DYNAMIC_FONT_STYLE_ID);
+        style.html(".CodeMirror { font-size: " + fontSizeStyle   + " !important; }");
+        $("head").append(style);
+    }
+    
     /**
      * @private
      * Sets the font size and restores the scroll position as best as possible.
-     * @param {string} fontSizeStyle A string with the font size and the size unit
-     * @param {string} lineHeightStyle A string with the line height and a the size unit
+     * @param {string=} fontSizeStyle  A string with the font size and the size unit
      */
-    function _setSizeAndRestoreScroll(fontSizeStyle, lineHeightStyle) {
+    function _setSizeAndRestoreScroll(fontSizeStyle) {
         var editor    = EditorManager.getCurrentFullEditor(),
             oldWidth  = editor._codeMirror.defaultCharWidth(),
-            oldHeight = editor.getTextHeight(),
-            scrollPos = editor.getScrollPos();
+            scrollPos = editor.getScrollPos(),
+            line      = editor._codeMirror.lineAtHeight(scrollPos.y, "local");
         
-        // It's necessary to inject a new rule to address all editors.
         _removeDynamicFontSize();
-        var style = $("<style type='text/css'></style>").attr("id", DYNAMIC_FONT_STYLE_ID);
-        style.html(".CodeMirror {" +
-                   "font-size: "   + fontSizeStyle   + " !important;" +
-                   "line-height: " + lineHeightStyle + " !important;}");
-        $("head").append(style);
-        
+        if (fontSizeStyle) {
+            _addDynamicFontSize(fontSizeStyle);
+        }
         editor.refreshAll();
         
-        // Scroll the document back to its original position, but not on the first load since the position
-        // was saved with the new height and already been restored.
-        if (_fontSizePrefsLoaded) {
-            // Calculate the new scroll based on the old font sizes and scroll position
-            var newWidth   = editor._codeMirror.defaultCharWidth(),
-                newHeight  = editor.getTextHeight(),
-                deltaX     = scrollPos.x / oldWidth,
-                deltaY     = scrollPos.y / oldHeight,
-                scrollPosX = scrollPos.x + Math.round(deltaX * (newWidth  - oldWidth)),
-                scrollPosY = scrollPos.y + Math.round(deltaY * (newHeight - oldHeight));
-                
-            editor.setScrollPos(scrollPosX, scrollPosY);
-        }
+        // Calculate the new scroll based on the old font sizes and scroll position
+        var newWidth   = editor._codeMirror.defaultCharWidth(),
+            deltaX     = scrollPos.x / oldWidth,
+            scrollPosX = scrollPos.x + Math.round(deltaX * (newWidth  - oldWidth)),
+            scrollPosY = editor._codeMirror.heightAtLine(line, "local");
+        
+        editor.setScrollPos(scrollPosX, scrollPosY);
     }
     
     /**
      * @private
      * Increases or decreases the editor's font size.
-     * @param {number} adjustment Negative number to make the font smaller; positive number to make it bigger
+     * @param {number} adjustment  Negative number to make the font smaller; positive number to make it bigger
      * @return {boolean} true if adjustment occurred, false if it did not occur 
      */
     function _adjustFontSize(adjustment) {
-        var fsStyle = $(".CodeMirror").css("font-size");
-        var lhStyle = $(".CodeMirror").css("line-height");
-
-        var validFont = /^[\d\.]+(px|em)$/;
+        var fsStyle   = $(".CodeMirror").css("font-size"),
+            validFont = /^[\d\.]+(px|em)$/;
         
-        // Make sure the font size and line height are expressed in terms
-        // we can handle (px or em). If not, simply bail.
-        if (fsStyle.search(validFont) === -1 || lhStyle.search(validFont) === -1) {
+        // Make sure that the font size is expressed in terms we can handle (px or em). If not, simply bail.
+        if (fsStyle.search(validFont) === -1) {
             return false;
         }
         
         // Guaranteed to work by the validation above.
-        var fsUnits = fsStyle.substring(fsStyle.length - 2, fsStyle.length);
-        var lhUnits = lhStyle.substring(lhStyle.length - 2, lhStyle.length);
-        var delta   = (fsUnits === "px") ? 1 : 0.1;
-        
-        var fsOld   = parseFloat(fsStyle.substring(0, fsStyle.length - 2));
-        var lhOld   = parseFloat(lhStyle.substring(0, lhStyle.length - 2));
-        
-        var fsNew   = fsOld + (delta * adjustment);
-        var lhNew   = lhOld;
-        if (fsUnits === lhUnits) {
-            lhNew = fsNew * LINE_HEIGHT;
-            if (lhUnits === "px") {
-                // Use integer px value to avoid rounding differences
-                lhNew = Math.ceil(lhNew);
-            }
-        }
-        
-        var fsStr   = fsNew + fsUnits;
-        var lhStr   = lhNew + lhUnits;
+        var fsUnits = fsStyle.substring(fsStyle.length - 2, fsStyle.length),
+            delta   = fsUnits === "px" ? 1 : 0.1,
+            fsOld   = parseFloat(fsStyle.substring(0, fsStyle.length - 2)),
+            fsNew   = fsOld + (delta * adjustment),
+            fsStr   = fsNew + fsUnits;
 
         // Don't let the font size get too small or too large. The minimum font size is 1px or 0.1em
         // and the maximum font size is 72px or 7.2em depending on the unit used
         if (fsNew < MIN_FONT_SIZE * delta || fsNew > MAX_FONT_SIZE * delta) {
             return false;
         }
         
-        _setSizeAndRestoreScroll(fsStr, lhStr);
+        _setSizeAndRestoreScroll(fsStr);
+        PreferencesManager.setViewState("fontSizeStyle", fsStr);
         
-        $(exports).triggerHandler("fontSizeChange", [adjustment, fsStr, lhStr]);
+        $(exports).triggerHandler("fontSizeChange", [adjustment, fsStr]);
         return true;
     }
     
     /** Increases the font size by 1 */
     function _handleIncreaseFontSize() {
-        if (_adjustFontSize(1)) {
-            PreferencesManager.setViewState("fontSizeAdjustment", PreferencesManager.getViewState("fontSizeAdjustment") + 1);
-        }
+        _adjustFontSize(1);
     }
     
     /** Decreases the font size by 1 */
     function _handleDecreaseFontSize() {
-        if (_adjustFontSize(-1)) {
-            PreferencesManager.setViewState("fontSizeAdjustment", PreferencesManager.getViewState("fontSizeAdjustment") - 1);
-        }
+        _adjustFontSize(-1);
     }
     
     /** Restores the font size to the original size */
     function _handleRestoreFontSize() {
-        _adjustFontSize(-PreferencesManager.getViewState("fontSizeAdjustment"));
-        PreferencesManager.setViewState("fontSizeAdjustment", 0);
+        _setSizeAndRestoreScroll();
+        PreferencesManager.setViewState("fontSizeStyle");
     }
     
     
@@ -215,14 +187,6 @@ define(function (require, exports, module) {
                 CommandManager.get(Commands.VIEW_DECREASE_FONT_SIZE).setEnabled(true);
                 CommandManager.get(Commands.VIEW_RESTORE_FONT_SIZE).setEnabled(true);
             }
-            
-            // Font Size preferences only need to be loaded one time
-            if (!_fontSizePrefsLoaded) {
-                _removeDynamicFontSize();
-                _adjustFontSize(PreferencesManager.getViewState("fontSizeAdjustment"));
-                _fontSizePrefsLoaded = true;
-            }
-            
         } else {
             // No current document so disable all of the Font Size commands
             CommandManager.get(Commands.VIEW_INCREASE_FONT_SIZE).setEnabled(false);
@@ -231,6 +195,31 @@ define(function (require, exports, module) {
         }
     }
     
+    /**
+     * Restores the font size using the saved style and migrates the old fontSizeAdjustment
+     * view state to the new fontSizeStyle, when required
+     */
+    function restoreFontSize() {
+        var fsStyle      = PreferencesManager.getViewState("fontSizeStyle"),
+            fsAdjustment = PreferencesManager.getViewState("fontSizeAdjustment");
+
+        if (fsAdjustment) {
+            // Always remove the old view state even if we also have the new view state.
+            PreferencesManager.setViewState("fontSizeAdjustment");
+
+            if (!fsStyle) {
+                // Migrate the old view state to the new one.
+                fsStyle = (DEFAULT_FONT_SIZE + fsAdjustment) + "px";
+                PreferencesManager.setViewState("fontSizeStyle", fsStyle);
+            }
+        }
+
+        if (fsStyle) {
+            _removeDynamicFontSize();
+            _addDynamicFontSize(fsStyle);
+        }
+    }
+    
     
     
     /**
@@ -319,6 +308,19 @@ define(function (require, exports, module) {
         _scrollLine(1);
     }
     
+    /**
+     * @private
+     * Convert the old "fontSizeAdjustment" preference to the new view state.
+     *
+     * @param {string} key  The key of the preference to be examined for migration
+     *      of old preferences. Not used since we only have one in this module.
+     * @param {string} value  The value of "fontSizeAdjustment" preference
+     * @return {Object} JSON object for the new view state equivalent to
+     *      the old "fontSizeAdjustment" preference.
+     */
+    function _convertToNewViewState(key, value) {
+        return { "fontSizeStyle": (DEFAULT_FONT_SIZE + value) + "px" };
+    }
     
     // Register command handlers
     CommandManager.register(Strings.CMD_INCREASE_FONT_SIZE, Commands.VIEW_INCREASE_FONT_SIZE, _handleIncreaseFontSize);
@@ -327,13 +329,13 @@ define(function (require, exports, module) {
     CommandManager.register(Strings.CMD_SCROLL_LINE_UP,     Commands.VIEW_SCROLL_LINE_UP,     _handleScrollLineUp);
     CommandManager.register(Strings.CMD_SCROLL_LINE_DOWN,   Commands.VIEW_SCROLL_LINE_DOWN,   _handleScrollLineDown);
 
-    // Initialize the default font size
-    PreferencesManager.stateManager.definePreference("fontSizeAdjustment", "number", 0);
-    PreferencesManager.convertPreferences(module, {"fontSizeAdjustment": "user"}, true);
+    PreferencesManager.convertPreferences(module, {"fontSizeAdjustment": "user"}, true, _convertToNewViewState);
 
     // Update UI when opening or closing a document
     $(DocumentManager).on("currentDocumentChange", _updateUI);
 
     // Update UI when Brackets finishes loading
     AppInit.appReady(_updateUI);
+    
+    exports.restoreFontSize = restoreFontSize;
 });
