Fix the order of keys on more keys keyboard

This CL fixes the behavior of !autoColumnOrder!<n> flag in a more keys
specification. Formerly the <n> value specifies the maximum number of
column of more keys. With this CL, the <n> value specifies the exact
number of column of more keys.

Bug: 16603887
Change-Id: I2120620fa512ee30043bb146ad675a942a375129

java/src/com/android/inputmethod/keyboard/Key.java

@@ -108,11 +108,23 @@ public class Key implements Comparable<Key> {
     private final MoreKeySpec[] mMoreKeys;
     /** More keys column number and flags */
     private final int mMoreKeysColumnAndFlags;
-    private static final int MORE_KEYS_COLUMN_MASK = 0x000000ff;
+    private static final int MORE_KEYS_COLUMN_NUMBER_MASK = 0x000000ff;
-    private static final int MORE_KEYS_FLAGS_FIXED_COLUMN_ORDER = 0x80000000;
+    // If this flag is specified, more keys keyboard should have the specified number of columns.
+    // Otherwise more keys keyboard should have less than or equal to the specified maximum number
+    // of columns.
+    private static final int MORE_KEYS_FLAGS_FIXED_COLUMN = 0x00000100;
+    // If this flag is specified, the order of more keys is determined by the order in the more
+    // keys' specification. Otherwise the order of more keys is automatically determined.
+    private static final int MORE_KEYS_FLAGS_FIXED_ORDER = 0x00000200;
+    private static final int MORE_KEYS_MODE_MAX_COLUMN_WITH_AUTO_ORDER = 0;
+    private static final int MORE_KEYS_MODE_FIXED_COLUMN_WITH_AUTO_ORDER =
+            MORE_KEYS_FLAGS_FIXED_COLUMN;
+    private static final int MORE_KEYS_MODE_FIXED_COLUMN_WITH_FIXED_ORDER =
+            (MORE_KEYS_FLAGS_FIXED_COLUMN | MORE_KEYS_FLAGS_FIXED_ORDER);
     private static final int MORE_KEYS_FLAGS_HAS_LABELS = 0x40000000;
     private static final int MORE_KEYS_FLAGS_NEEDS_DIVIDERS = 0x20000000;
     private static final int MORE_KEYS_FLAGS_NO_PANEL_AUTO_MORE_KEY = 0x10000000;
+    // TODO: Rename these specifiers to !autoOrder! and !fixedOrder! respectively.
     private static final String MORE_KEYS_AUTO_COLUMN_ORDER = "!autoColumnOrder!";
     private static final String MORE_KEYS_FIXED_COLUMN_ORDER = "!fixedColumnOrder!";
     private static final String MORE_KEYS_HAS_LABELS = "!hasLabels!";
@@ -255,25 +267,31 @@ public class Key implements Comparable<Key> {
         int actionFlags = style.getFlags(keyAttr, R.styleable.Keyboard_Key_keyActionFlags);
         String[] moreKeys = style.getStringArray(keyAttr, R.styleable.Keyboard_Key_moreKeys);
 
-        int moreKeysColumn = style.getInt(keyAttr,
+        // Get maximum column order number and set a relevant mode value.
-                R.styleable.Keyboard_Key_maxMoreKeysColumn, params.mMaxMoreKeysKeyboardColumn);
+        int moreKeysColumnAndFlags = MORE_KEYS_MODE_MAX_COLUMN_WITH_AUTO_ORDER
+                | style.getInt(keyAttr, R.styleable.Keyboard_Key_maxMoreKeysColumn,
+                        params.mMaxMoreKeysKeyboardColumn);
         int value;
         if ((value = MoreKeySpec.getIntValue(moreKeys, MORE_KEYS_AUTO_COLUMN_ORDER, -1)) > 0) {
-            moreKeysColumn = value & MORE_KEYS_COLUMN_MASK;
+            // Override with fixed column order number and set a relevant mode value.
+            moreKeysColumnAndFlags = MORE_KEYS_MODE_FIXED_COLUMN_WITH_AUTO_ORDER
+                    | (value & MORE_KEYS_COLUMN_NUMBER_MASK);
         }
         if ((value = MoreKeySpec.getIntValue(moreKeys, MORE_KEYS_FIXED_COLUMN_ORDER, -1)) > 0) {
-            moreKeysColumn = MORE_KEYS_FLAGS_FIXED_COLUMN_ORDER | (value & MORE_KEYS_COLUMN_MASK);
+            // Override with fixed column order number and set a relevant mode value.
+            moreKeysColumnAndFlags = MORE_KEYS_MODE_FIXED_COLUMN_WITH_FIXED_ORDER
+                    | (value & MORE_KEYS_COLUMN_NUMBER_MASK);
         }
         if (MoreKeySpec.getBooleanValue(moreKeys, MORE_KEYS_HAS_LABELS)) {
-            moreKeysColumn |= MORE_KEYS_FLAGS_HAS_LABELS;
+            moreKeysColumnAndFlags |= MORE_KEYS_FLAGS_HAS_LABELS;
         }
         if (MoreKeySpec.getBooleanValue(moreKeys, MORE_KEYS_NEEDS_DIVIDERS)) {
-            moreKeysColumn |= MORE_KEYS_FLAGS_NEEDS_DIVIDERS;
+            moreKeysColumnAndFlags |= MORE_KEYS_FLAGS_NEEDS_DIVIDERS;
         }
         if (MoreKeySpec.getBooleanValue(moreKeys, MORE_KEYS_NO_PANEL_AUTO_MORE_KEY)) {
-            moreKeysColumn |= MORE_KEYS_FLAGS_NO_PANEL_AUTO_MORE_KEY;
+            moreKeysColumnAndFlags |= MORE_KEYS_FLAGS_NO_PANEL_AUTO_MORE_KEY;
         }
-        mMoreKeysColumnAndFlags = moreKeysColumn;
+        mMoreKeysColumnAndFlags = moreKeysColumnAndFlags;
 
         final String[] additionalMoreKeys;
         if ((mLabelFlags & LABEL_FLAGS_DISABLE_ADDITIONAL_MORE_KEYS) != 0) {
@@ -680,12 +698,16 @@ public class Key implements Comparable<Key> {
                 && !TextUtils.isEmpty(mHintLabel);
     }
 
-    public final int getMoreKeysColumn() {
+    public final int getMoreKeysColumnNumber() {
-        return mMoreKeysColumnAndFlags & MORE_KEYS_COLUMN_MASK;
+        return mMoreKeysColumnAndFlags & MORE_KEYS_COLUMN_NUMBER_MASK;
     }
 
-    public final boolean isFixedColumnOrderMoreKeys() {
+    public final boolean isMoreKeysFixedColumn() {
-        return (mMoreKeysColumnAndFlags & MORE_KEYS_FLAGS_FIXED_COLUMN_ORDER) != 0;
+        return (mMoreKeysColumnAndFlags & MORE_KEYS_FLAGS_FIXED_COLUMN) != 0;
+    }
+
+    public final boolean isMoreKeysFixedOrder() {
+        return (mMoreKeysColumnAndFlags & MORE_KEYS_FLAGS_FIXED_ORDER) != 0;
     }
 
     public final boolean hasLabelsInMoreKeys() {

java/src/com/android/inputmethod/keyboard/MoreKeysKeyboard.java

@@ -43,7 +43,7 @@ public final class MoreKeysKeyboard extends Keyboard {
 
     @UsedForTesting
     static class MoreKeysKeyboardParams extends KeyboardParams {
-        public boolean mIsFixedOrder;
+        public boolean mIsMoreKeysFixedOrder;
         /* package */int mTopRowAdjustment;
         public int mNumRows;
         public int mNumColumns;
@@ -61,29 +61,35 @@ public final class MoreKeysKeyboard extends Keyboard {
          * Set keyboard parameters of more keys keyboard.
          *
          * @param numKeys number of keys in this more keys keyboard.
-         * @param maxColumns number of maximum columns of this more keys keyboard.
+         * @param numColumn number of columns of this more keys keyboard.
          * @param keyWidth more keys keyboard key width in pixel, including horizontal gap.
          * @param rowHeight more keys keyboard row height in pixel, including vertical gap.
          * @param coordXInParent coordinate x of the key preview in parent keyboard.
          * @param parentKeyboardWidth parent keyboard width in pixel.
-         * @param isFixedColumnOrder if true, more keys should be laid out in fixed order.
+         * @param isMoreKeysFixedColumn true if more keys keyboard should have
+         *   <code>numColumn</code> columns. Otherwise more keys keyboard should have
+         *   <code>numColumn</code> columns at most.
+         * @param isMoreKeysFixedOrder true if the order of more keys is determined by the order in
+         *   the more keys' specification. Otherwise the order of more keys is automatically
+         *   determined.
          * @param dividerWidth width of divider, zero for no dividers.
          */
-        public void setParameters(final int numKeys, final int maxColumns, final int keyWidth,
+        public void setParameters(final int numKeys, final int numColumn, final int keyWidth,
                 final int rowHeight, final int coordXInParent, final int parentKeyboardWidth,
-                final boolean isFixedColumnOrder, final int dividerWidth) {
+                final boolean isMoreKeysFixedColumn, final boolean isMoreKeysFixedOrder,
-            mIsFixedOrder = isFixedColumnOrder;
+                final int dividerWidth) {
-            if (parentKeyboardWidth / keyWidth < Math.min(numKeys, maxColumns)) {
+            mIsMoreKeysFixedOrder = isMoreKeysFixedOrder;
+            if (parentKeyboardWidth / keyWidth < Math.min(numKeys, numColumn)) {
                 throw new IllegalArgumentException("Keyboard is too small to hold more keys: "
-                        + parentKeyboardWidth + " " + keyWidth + " " + numKeys + " " + maxColumns);
+                        + parentKeyboardWidth + " " + keyWidth + " " + numKeys + " " + numColumn);
             }
             mDefaultKeyWidth = keyWidth;
             mDefaultRowHeight = rowHeight;
 
-            final int numRows = (numKeys + maxColumns - 1) / maxColumns;
+            final int numRows = (numKeys + numColumn - 1) / numColumn;
             mNumRows = numRows;
-            final int numColumns = mIsFixedOrder ? Math.min(numKeys, maxColumns)
+            final int numColumns = isMoreKeysFixedColumn ? Math.min(numKeys, numColumn)
-                    : getOptimizedColumns(numKeys, maxColumns);
+                    : getOptimizedColumns(numKeys, numColumn);
             mNumColumns = numColumns;
             final int topKeys = numKeys % numColumns;
             mTopKeys = topKeys == 0 ? numColumns : topKeys;
@@ -120,7 +126,7 @@ public final class MoreKeysKeyboard extends Keyboard {
             mRightKeys = rightKeys;
 
             // Adjustment of the top row.
-            mTopRowAdjustment = mIsFixedOrder ? getFixedOrderTopRowAdjustment()
+            mTopRowAdjustment = isMoreKeysFixedOrder ? getFixedOrderTopRowAdjustment()
                     : getAutoOrderTopRowAdjustment();
             mDividerWidth = dividerWidth;
             mColumnWidth = mDefaultKeyWidth + mDividerWidth;
@@ -148,7 +154,7 @@ public final class MoreKeysKeyboard extends Keyboard {
 
         // Return key position according to column count (0 is default).
         /* package */int getColumnPos(final int n) {
-            return mIsFixedOrder ? getFixedOrderColumnPos(n) : getAutomaticColumnPos(n);
+            return mIsMoreKeysFixedOrder ? getFixedOrderColumnPos(n) : getAutomaticColumnPos(n);
         }
 
         private int getFixedOrderColumnPos(final int n) {
@@ -263,7 +269,8 @@ public final class MoreKeysKeyboard extends Keyboard {
          * @param keyboard the {@link Keyboard} that contains the parentKey.
          * @param isSingleMoreKeyWithPreview true if the <code>key</code> has just a single
          *        "more key" and its key popup preview is enabled.
-         * @param keyPreviewDrawParams the parameter to place key preview.
+         * @param keyPreviewVisibleWidth the width of visible part of key popup preview.
+         * @param keyPreviewVisibleHeight the height of visible part of key popup preview
          * @param paintToMeasure the {@link Paint} object to measure a "more key" width
          */
         public Builder(final Context context, final Key key, final Keyboard keyboard,
@@ -306,9 +313,9 @@ public final class MoreKeysKeyboard extends Keyboard {
                 dividerWidth = 0;
             }
             final MoreKeySpec[] moreKeys = key.getMoreKeys();
-            mParams.setParameters(moreKeys.length, key.getMoreKeysColumn(), keyWidth, rowHeight,
+            mParams.setParameters(moreKeys.length, key.getMoreKeysColumnNumber(), keyWidth, rowHeight,
                     key.getX() + key.getWidth() / 2, keyboard.mId.mWidth,
-                    key.isFixedColumnOrderMoreKeys(), dividerWidth);
+                    key.isMoreKeysFixedColumn(), key.isMoreKeysFixedOrder(), dividerWidth);
         }
 
         private static int getMaxKeyWidth(final Key parentKey, final int minKeyWidth,

tests/src/com/android/inputmethod/keyboard/MoreKeysKeyboardBuilderFixedOrderTests.java

@@ -47,7 +47,8 @@ public class MoreKeysKeyboardBuilderFixedOrderTests extends AndroidTestCase {
             final int coordXInParent) {
         final MoreKeysKeyboardParams params = new MoreKeysKeyboardParams();
         params.setParameters(numKeys, columnNum, WIDTH, HEIGHT, coordXInParent, KEYBOARD_WIDTH,
-                true /* isFixedOrderColumn */, 0 /* dividerWidth */);
+                true /* isMoreKeysFixedColumn */, true /* isMoreKeysFixedOrder */,
+                0 /* dividerWidth */);
         return params;
     }
 

tests/src/com/android/inputmethod/keyboard/MoreKeysKeyboardBuilderMaxOrderTests.java

@@ -22,7 +22,7 @@ import android.test.suitebuilder.annotation.MediumTest;
 import com.android.inputmethod.keyboard.MoreKeysKeyboard.MoreKeysKeyboardParams;
 
 @MediumTest
-public class MoreKeysKeyboardBuilderTests extends AndroidTestCase {
+public class MoreKeysKeyboardBuilderMaxOrderTests extends AndroidTestCase {
     private static final int WIDTH = 10;
     private static final int HEIGHT = 10;
 
@@ -47,7 +47,8 @@ public class MoreKeysKeyboardBuilderTests extends AndroidTestCase {
             final int coordXInParent) {
         final MoreKeysKeyboardParams params = new MoreKeysKeyboardParams();
         params.setParameters(numKeys, maxColumns, WIDTH, HEIGHT, coordXInParent, KEYBOARD_WIDTH,
-                false /* isFixedOrderColumn */, 0 /* dividerWidth */);
+                false /* isMoreKeysFixedColumn */, false /* isMoreKeysFixedOrder */,
+                0 /* dividerWidth */);
         return params;
     }