From baf2c6aa52724941e7d8d1c45713c7881170926b Mon Sep 17 00:00:00 2001 From: jahorton Date: Thu, 5 Sep 2019 11:06:27 +0700 Subject: [PATCH 1/4] Implements legacy check for default keycap display --- web/source/osk/defaultLayouts.ts | 6 ++- web/source/osk/visualKeyboard.ts | 9 +++- web/source/utils/version.ts | 77 ++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 4 deletions(-) create mode 100644 web/source/utils/version.ts diff --git a/web/source/osk/defaultLayouts.ts b/web/source/osk/defaultLayouts.ts index f8624b3f79..a05e48be64 100644 --- a/web/source/osk/defaultLayouts.ts +++ b/web/source/osk/defaultLayouts.ts @@ -3,6 +3,8 @@ Copyright 2017 SIL International ***/ +/// + namespace com.keyman.osk { let Codes = com.keyman.text.Codes; @@ -114,7 +116,7 @@ namespace com.keyman.osk { * @param {string} formFactor * @return {Object} */ - static buildDefaultLayout(PVK, kbdBitmask: number, formFactor: string): LayoutFormFactor { + static buildDefaultLayout(PVK, kbdDevVersion: utils.Version, kbdBitmask: number, formFactor: string): LayoutFormFactor { let keyman = com.keyman.singleton; let util = keyman.util; @@ -250,7 +252,7 @@ namespace com.keyman.osk { // Fall back to US English keycap text as default for the base two layers if not otherwise defined. // (Any 'ghost' keys must be explicitly defined in layout for these layers.) - if(isDefault) { + if(isDefault && kbdDevVersion.precedes(utils.Version.NO_DEFAULT_KEYCAPS)) { if(key['id'] != 'K_SPACE' && kx+65 * isShift < Layouts.dfltText.length && key['text'] !== null) { key['text'] = key['text'] || Layouts.dfltText[kx+65*isShift]; } diff --git a/web/source/osk/visualKeyboard.ts b/web/source/osk/visualKeyboard.ts index ab629036d6..a0d0801705 100644 --- a/web/source/osk/visualKeyboard.ts +++ b/web/source/osk/visualKeyboard.ts @@ -1,4 +1,5 @@ /// +/// namespace com.keyman.osk { let Codes = com.keyman.text.Codes; @@ -591,7 +592,9 @@ namespace com.keyman.osk { // Build a layout using the default for the device if(typeof layout != 'object' || layout == null) { - layout=Layouts.buildDefaultLayout(PVK,kbdBitmask, formFactor); + // Using 9.0.0 as a fallback Developer version for keyboards. + let kbdDevVersion = utils.Version.parseWithDefault(activeKeyboard['KVER'], "9.0.0"); + layout=Layouts.buildDefaultLayout(PVK, kbdDevVersion, kbdBitmask, formFactor); } // Create the collection of HTML elements from the device-dependent layout object @@ -2093,7 +2096,9 @@ namespace com.keyman.osk { // Else get a default layout for the device for this keyboard if(layout == null && PVK != null) { - layout=Layouts.buildDefaultLayout(PVK,keymanweb.keyboardManager.getKeyboardModifierBitmask(PKbd),formFactor); + // Using 9.0.0 as a fallback Developer version for keyboards. + let kbdDevVersion = utils.Version.parseWithDefault(PKbd['KVER'], "9.0.0"); + layout=Layouts.buildDefaultLayout(PVK, kbdDevVersion, keymanweb.keyboardManager.getKeyboardModifierBitmask(PKbd),formFactor); } // Cannot create an OSK if no layout defined, just return empty DIV diff --git a/web/source/utils/version.ts b/web/source/utils/version.ts new file mode 100644 index 0000000000..ed314f6cb4 --- /dev/null +++ b/web/source/utils/version.ts @@ -0,0 +1,77 @@ +namespace com.keyman.utils { + // Dotted-decimal version + export class Version { + public static readonly NO_DEFAULT_KEYCAPS = new Version("12.0"); + + private readonly components: number[] + + constructor(text: String) { + let parts = text.split('.'); + let componentArray: number[] = []; + + if(parts.length < 2) { + throw new Error("Version string must have at least a major and minor component!"); + } + + for(let i=0; i < parts.length; i++) { + let value = parseInt(parts[i], 10); + if(isNaN(value)) { + throw new Error("Version string components must be numerical!"); + } + + componentArray.push(value); + } + + this.components = componentArray; + } + + get major(): number { + return this.components[0]; + } + + get minor(): number { + return this.components[1]; + } + + toString(): string { + return this.components.join('.'); + } + + equals(other: Version): boolean { + if(this.components.length != other.components.length) { + return false; + } else { + for(let i=0; i < this.components.length; i++) { + if(this.components[i] != other.components[i]) { + return false; + } + } + + return true; + } + } + + precedes(other: Version): boolean { + // If the version info depth differs, we need a flag to indicate whether or not 'this' instance wins ties. + // 12.0 is considered to precede 12.0.0. + var tieBreaker: boolean = this.components.length < other.components.length; + var maxDepth: number = tieBreaker ? this.components.length : other.components.length; + + for(let i=0; i < maxDepth; i++) { + if(this.components[i] < other.components[i]) { + return true; + } + } + + return tieBreaker; + } + + static parseWithDefault(text: string, fallback: string): Version { + if(!text) { + return new Version(fallback); + } else { + return new Version(text); + } + } + } +} \ No newline at end of file From a88bed2a2b0a852a51d212b78524d8849ac80269 Mon Sep 17 00:00:00 2001 From: jahorton Date: Thu, 5 Sep 2019 11:45:06 +0700 Subject: [PATCH 2/4] Adds a few simple test cases for version logic. --- web/unit_tests/cases/versions.js | 39 ++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 web/unit_tests/cases/versions.js diff --git a/web/unit_tests/cases/versions.js b/web/unit_tests/cases/versions.js new file mode 100644 index 0000000000..097b4625c6 --- /dev/null +++ b/web/unit_tests/cases/versions.js @@ -0,0 +1,39 @@ +var assert = chai.assert; + +describe('Version Logic', function() { + this.timeout(kmwconfig.timeouts.standard); + + before(function(done) { + this.timeout(kmwconfig.timeouts.scriptLoad); + setupKMW(null, done, kmwconfig.timeouts.scriptLoad); + }); + + after(function() { + teardownKMW(); + }); + + + it('Should properly process a simple major.minor version string.', function() { + var version = new com.keyman.utils.Version("1.2"); + assert.equal(version.major, 1); + assert.equal(version.minor, 2); + }); + + it('Should handle long/deep version specifications.', function() { + var version = new com.keyman.utils.Version("1.2.3.4.5.6"); + assert.equal(version.components.length, 6); + assert.equal(version.major, 1); + assert.equal(version.minor, 2); + }); + + it('Should properly compare two versions.', function() { + var v9_0_1 = new com.keyman.utils.Version("9.0.1"); + var v9_1_0 = new com.keyman.utils.Version("9.1.0"); + var v10_0_0 = new com.keyman.utils.Version("10.0.0"); + + assert.isTrue(v9_0_1.precedes(v9_1_0)); + assert.isTrue(v9_1_0.precedes(v10_0_0)); + assert.isTrue(v9_0_1.precedes(v10_0_0)); + }); + +}); \ No newline at end of file From 74a2ad49dfa6fd2bb51540d7310e0fc1e48eb127 Mon Sep 17 00:00:00 2001 From: jahorton Date: Fri, 6 Sep 2019 12:01:17 +0700 Subject: [PATCH 3/4] Adjustments based on PR reviews --- web/source/osk/defaultLayouts.ts | 9 ++-- web/source/osk/visualKeyboard.ts | 6 +-- web/source/utils/version.ts | 77 +++++++++++++++++++++----------- web/unit_tests/cases/versions.js | 20 +++++++-- 4 files changed, 75 insertions(+), 37 deletions(-) diff --git a/web/source/osk/defaultLayouts.ts b/web/source/osk/defaultLayouts.ts index a05e48be64..f6088746ff 100644 --- a/web/source/osk/defaultLayouts.ts +++ b/web/source/osk/defaultLayouts.ts @@ -111,8 +111,9 @@ namespace com.keyman.osk { /** * Build a default layout for keyboards with no explicit layout * - * @param {Object} PVK keyboard object (as loaded) - * @param {number} kbdBitmask keyboard modifier bitmask + * @param {Object} PVK keyboard object (as loaded) + * @param {Object} kbdDevVersion object representing the version of Developer that compiled the keyboard + * @param {number} kbdBitmask keyboard modifier bitmask * @param {string} formFactor * @return {Object} */ @@ -250,8 +251,8 @@ namespace com.keyman.osk { if(kx >= 0 && kx < layerSpec.length) key['text']=layerSpec[kx]; } - // Fall back to US English keycap text as default for the base two layers if not otherwise defined. - // (Any 'ghost' keys must be explicitly defined in layout for these layers.) + // Legacy (pre 12.0) behavior: fall back to US English keycap text as default for the base two layers + // if a key cap is not otherwise defined. (Any intentional 'ghost' keys must be explicitly defined.) if(isDefault && kbdDevVersion.precedes(utils.Version.NO_DEFAULT_KEYCAPS)) { if(key['id'] != 'K_SPACE' && kx+65 * isShift < Layouts.dfltText.length && key['text'] !== null) { key['text'] = key['text'] || Layouts.dfltText[kx+65*isShift]; diff --git a/web/source/osk/visualKeyboard.ts b/web/source/osk/visualKeyboard.ts index a0d0801705..beffc245cc 100644 --- a/web/source/osk/visualKeyboard.ts +++ b/web/source/osk/visualKeyboard.ts @@ -592,8 +592,7 @@ namespace com.keyman.osk { // Build a layout using the default for the device if(typeof layout != 'object' || layout == null) { - // Using 9.0.0 as a fallback Developer version for keyboards. - let kbdDevVersion = utils.Version.parseWithDefault(activeKeyboard['KVER'], "9.0.0"); + let kbdDevVersion = new utils.Version(activeKeyboard['KVER']); layout=Layouts.buildDefaultLayout(PVK, kbdDevVersion, kbdBitmask, formFactor); } @@ -2096,8 +2095,7 @@ namespace com.keyman.osk { // Else get a default layout for the device for this keyboard if(layout == null && PVK != null) { - // Using 9.0.0 as a fallback Developer version for keyboards. - let kbdDevVersion = utils.Version.parseWithDefault(PKbd['KVER'], "9.0.0"); + let kbdDevVersion = new utils.Version(PKbd['KVER']); layout=Layouts.buildDefaultLayout(PVK, kbdDevVersion, keymanweb.keyboardManager.getKeyboardModifierBitmask(PKbd),formFactor); } diff --git a/web/source/utils/version.ts b/web/source/utils/version.ts index ed314f6cb4..f67453df64 100644 --- a/web/source/utils/version.ts +++ b/web/source/utils/version.ts @@ -1,11 +1,39 @@ namespace com.keyman.utils { // Dotted-decimal version export class Version { - public static readonly NO_DEFAULT_KEYCAPS = new Version("12.0"); + // Represents a default version value for keyboards compiled before this was compiled into keyboards. + // The exact version is unknown at this point, but the value is "good enough" for what we need. + public static readonly DEVELOPER_VERSION_FALLBACK = new Version([9, 0, 0]); + + // For 12.0, the old default behavior of adding missing keycaps to the default layers was removed, + // as it results in unexpected, bug-like behavior for keyboard designers when it is unwanted. + public static readonly NO_DEFAULT_KEYCAPS = new Version([12, 0]); private readonly components: number[] - constructor(text: String) { + /** + * Parses version information, preparing it for use in comparisons. + * @param text Either a string representing a version number (ex: "9.0.0") or an array representing + * its components (ex: [9, 0, 0]). + */ + constructor(text: String | number[]) { + // If a keyboard doesn't specify a version, use the DEVELOPER_VERSION_FALLBACK values. + if(text === undefined || text === null) { + this.components = [].concat(Version.DEVELOPER_VERSION_FALLBACK.components); + return; + } + + if(Array.isArray(text)) { + let components = text as number[]; + if(components.length < 2) { + throw new Error("Version string must have at least a major and minor component!"); + } else { + this.components = [].concat(components); + return; + } + } + + // else, standard constructor path. let parts = text.split('.'); let componentArray: number[] = []; @@ -38,40 +66,37 @@ namespace com.keyman.utils { } equals(other: Version): boolean { - if(this.components.length != other.components.length) { - return false; - } else { - for(let i=0; i < this.components.length; i++) { - if(this.components[i] != other.components[i]) { - return false; - } - } - - return true; - } + return this.compareTo(other) == 0; } precedes(other: Version): boolean { + return this.compareTo(other) < 0; + } + + compareTo(other: Version): number { // If the version info depth differs, we need a flag to indicate whether or not 'this' instance wins ties. // 12.0 is considered to precede 12.0.0. - var tieBreaker: boolean = this.components.length < other.components.length; - var maxDepth: number = tieBreaker ? this.components.length : other.components.length; + var isShorter: boolean = this.components.length < other.components.length; + var maxDepth: number = (this.components.length < other.components.length) ? this.components.length : other.components.length; - for(let i=0; i < maxDepth; i++) { - if(this.components[i] < other.components[i]) { - return true; + var i: number; + for(i = 0; i < maxDepth; i++) { + let delta = this.components[i] - other.components[i]; + if(delta != 0) { + return delta; } } - return tieBreaker; - } + var longList = isShorter ? other.components : this.components; + do { + if(longList[i] > 0) { + return isShorter ? -1 : 1; + } + i++; + } while (i < longList.length); - static parseWithDefault(text: string, fallback: string): Version { - if(!text) { - return new Version(fallback); - } else { - return new Version(text); - } + // Equal. + return 0; } } } \ No newline at end of file diff --git a/web/unit_tests/cases/versions.js b/web/unit_tests/cases/versions.js index 097b4625c6..bd77a9039d 100644 --- a/web/unit_tests/cases/versions.js +++ b/web/unit_tests/cases/versions.js @@ -12,6 +12,10 @@ describe('Version Logic', function() { teardownKMW(); }); + it('Should provide a default, fallback value when nothing is specified', function() { + var fallback = new com.keyman.utils.Version(undefined); + assert.isTrue(fallback.equals(com.keyman.utils.Version.DEVELOPER_VERSION_FALLBACK)); + }); it('Should properly process a simple major.minor version string.', function() { var version = new com.keyman.utils.Version("1.2"); @@ -29,11 +33,21 @@ describe('Version Logic', function() { it('Should properly compare two versions.', function() { var v9_0_1 = new com.keyman.utils.Version("9.0.1"); var v9_1_0 = new com.keyman.utils.Version("9.1.0"); + var v10_0 = new com.keyman.utils.Version("10.0"); var v10_0_0 = new com.keyman.utils.Version("10.0.0"); - assert.isTrue(v9_0_1.precedes(v9_1_0)); - assert.isTrue(v9_1_0.precedes(v10_0_0)); - assert.isTrue(v9_0_1.precedes(v10_0_0)); + // "Precede" checks + assert.equal(v9_0_1.compareTo(v9_1_0), -1); + assert.equal(v9_1_0.compareTo(v10_0_0), -1); + assert.equal(v9_0_1.compareTo(v10_0_0), -1); + + // Equality checks + assert.equal(v9_0_1.compareTo(v9_0_1), 0); + // Tests equal versions where one omits the build number. + assert.equal( v10_0.compareTo(v10_0_0), 0); + + // Ensures the first "precede" check's return is flipped when the order's flipped. + assert.equal(v9_1_0.compareTo(v9_0_1), 1); }); }); \ No newline at end of file From e6f906bd3bf42acd7da5c7afae7d906d92e93534 Mon Sep 17 00:00:00 2001 From: jahorton Date: Fri, 6 Sep 2019 12:14:03 +0700 Subject: [PATCH 4/4] Missed comment update, history.md --- web/history.md | 3 +++ web/source/utils/version.ts | 3 +-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/web/history.md b/web/history.md index dd9e80727b..3b1fcca013 100644 --- a/web/history.md +++ b/web/history.md @@ -3,6 +3,9 @@ ## 13.0 alpha * Start version 13.0 +## 2019-09-06 12.0.81 beta +* For keyboards compiled with Developer 12.0 onward, default key caps will no longer be provided. (#2045) + ## 2019-09-04 12.0.80 beta * Fixed issue with 'pt' and unitless font size specifications for keyboard layouts (#2033) * Slightly enhanced error logging for internal events (#2037) diff --git a/web/source/utils/version.ts b/web/source/utils/version.ts index f67453df64..3d43ee7ea8 100644 --- a/web/source/utils/version.ts +++ b/web/source/utils/version.ts @@ -74,8 +74,7 @@ namespace com.keyman.utils { } compareTo(other: Version): number { - // If the version info depth differs, we need a flag to indicate whether or not 'this' instance wins ties. - // 12.0 is considered to precede 12.0.0. + // If the version info depth differs, we need a flag to indicate which instance is shorter. var isShorter: boolean = this.components.length < other.components.length; var maxDepth: number = (this.components.length < other.components.length) ? this.components.length : other.components.length;