When removing columns from CellTable, also remove the col element.

This fixes a bug in Firefox. Firefox would remove the
column, yet keep white space the size of the removed column instead of
having the table fill the remaining space.

Change-Id: I5651120d2c8bdd36d5c1787a9dbf6a0935750c2f
Review-Link: https://gwt-review.googlesource.com/#/c/12641/

user/src/com/google/gwt/user/cellview/client/CellTable.java

diff --git a/user/src/com/google/gwt/user/cellview/client/CellTable.java b/user/src/com/google/gwt/user/cellview/client/CellTable.java
index 419a49f..8253cd6 100644
--- a/user/src/com/google/gwt/user/cellview/client/CellTable.java
+++ b/user/src/com/google/gwt/user/cellview/client/CellTable.java
@@ -502,6 +502,7 @@
   private TableSectionElement tfoot;
   private TableSectionElement thead;
   private boolean colGroupEnabled = true;
+  private boolean removeColumnsOnHide = true;
 
   /**
    * Constructs a table with a default page size of 15.
@@ -581,11 +582,11 @@
       Widget loadingIndicator) {
     this(pageSize, resources, keyProvider, loadingIndicator, true, true);
   }
-  
+
   /**
    * Constructs a table with the specified page size, {@link Resources}, key
    * provider, and loading indicator.
-   * 
+   *
    * @param pageSize the page size
    * @param resources the resources to use for this widget
    * @param keyProvider an instance of ProvidesKey<T>, or null if the record
@@ -595,7 +596,7 @@
    * @param enableColGroup enable colgroup element. This is used when the table is using fixed
    *          layout and when column style is added. Ignoring this element will boost rendering
    *          performance. Note that when colgroup is disabled, {@link #setColumnWidth},
-   *          {@link setTableLayoutFixed} and {@link addColumnStyleName} are no longe supported
+   *          {@link setTableLayoutFixed} and {@link addColumnStyleName} are no longer supported
    * @param attachLoadingPanel attaching the table section that contains the empty table widget and
    *          the loading indicator. Attaching this to the table significantly improve the rendering
    *          performance in webkit based browsers but also introduces significantly larger latency
@@ -818,6 +819,10 @@
     setTableLayoutFixed(isFixedLayout);
   }
 
+  public void setRemoveColumnsOnHide(boolean removeColumnsOnHide) {
+    this.removeColumnsOnHide = removeColumnsOnHide;
+  }
+
   @Override
   protected void doSetColumnWidth(int column, String width) {
     // This is invoked when column width is set (which will throw an exception if colgroup is not
@@ -889,21 +894,22 @@
     super.refreshColumnWidths();
 
     /*
-     * Set the width to zero and the display to none for all col elements that 
-     * appear after the last column. Clearing the width would cause it to take 
-     * up the remaining width in a fixed layout table.
-     * 
-     * Clear the display for all columns that appear in the table. 
+     * Clear the display for all columns that appear in the table. And removes any cols from the
+     * colgroup that are no longer in the table.
      */
     if (colGroupEnabled) {
       int colCount = colgroup.getChildCount();
-      int lastColumn = getRealColumnCount(); 
+      int lastColumn = getRealColumnCount();
       for (int i = 0; i < lastColumn; i++) {
         ensureTableColElement(i).getStyle().clearDisplay();
       }
-      for (int i = lastColumn; i < colCount; i++) {
-        doSetColumnWidth(i, "0px");
-        ensureTableColElement(i).getStyle().setDisplay(Display.NONE);
+      for (int i = colCount - 1; i >= lastColumn; i--) {
+        if (removeColumnsOnHide) {
+          doSetColumnWidth(i, "0px");
+          ensureTableColElement(i).getStyle().setDisplay(Display.NONE);
+        } else {
+          colgroup.removeChild(colgroup.getChild(i));
+        }
       }
     }
   }

user/test/com/google/gwt/user/cellview/client/CellTableTest.java

diff --git a/user/test/com/google/gwt/user/cellview/client/CellTableTest.java b/user/test/com/google/gwt/user/cellview/client/CellTableTest.java
index 9bc9aaf..7dae8ee 100644
--- a/user/test/com/google/gwt/user/cellview/client/CellTableTest.java
+++ b/user/test/com/google/gwt/user/cellview/client/CellTableTest.java
@@ -291,17 +291,38 @@
   /**
    * Test that removing a column sets its width to zero and the display to none.
    */
-  public void testRemoveColumnWithWidth() {
+  public void testRemoveColumnWithWidth_withRemoveColumnsOnHideDisabled() {
     CellTable<String> table = createAbstractHasData(new TextCell());
+    table.setRemoveColumnsOnHide(false);
     Column<String, ?> column1 = table.getColumn(1);
     table.setColumnWidth(column1, "100px");
     Element col0 = table.colgroup.getFirstChildElement();
     Element col1 = col0.getNextSiblingElement();
     assertEquals("100px", col1.getStyle().getWidth().toLowerCase(Locale.ROOT));
+    assertEquals(2, table.colgroup.getChildCount());
 
     // Remove column 1.
     table.removeColumn(column1);
     table.getPresenter().flush();
+
+    assertNull(col0.getNextSiblingElement());
+    assertEquals(1, table.colgroup.getChildCount());
+  }
+
+  public void testRemoveColumnWithWidth_withRemoveColumnsOnHideEnabled() {
+    CellTable<String> table = createAbstractHasData(new TextCell());
+    table.setRemoveColumnsOnHide(true);
+    Column<String, ?> column1 = table.getColumn(1);
+    table.setColumnWidth(column1, "100px");
+    Element col0 = table.colgroup.getFirstChildElement();
+    Element col1 = col0.getNextSiblingElement();
+    assertEquals("100px", col1.getStyle().getWidth().toLowerCase(Locale.ROOT));
+    assertEquals(2, table.colgroup.getChildCount());
+
+    // Remove column 1.
+    table.removeColumn(column1);
+    table.getPresenter().flush();
+
     assertEquals("0px", col1.getStyle().getWidth());
     assertEquals("none", col1.getStyle().getDisplay().toLowerCase(Locale.ROOT));
   }