Fixes for #6720

- Fixes based on review
- Fix for possible NPE
- Fixed bug where resizing columns would throw off the header/column width sync
- Fixed bug where column reordering would fail due to element not found exception

svn changeset:20544/svn branch:6.7

4 files changed, 168 insertions, 57 deletions

diff --git a/src/com/vaadin/terminal/gwt/client/ComputedStyle.java b/src/com/vaadin/terminal/gwt/client/ComputedStyle.java
index 5aac2caf47..1dd53fa98f 100644
--- a/src/com/vaadin/terminal/gwt/client/ComputedStyle.java
+++ b/src/com/vaadin/terminal/gwt/client/ComputedStyle.java
@@ -115,7 +115,11 @@ public class ComputedStyle {
     }-*/;
 
     public final int getIntProperty(String name) {
-        return parseInt(getProperty(name)).intValue();
+        Integer parsed = parseInt(getProperty(name));
+        if (parsed != null) {
+            return parsed.intValue();
+        }
+        return 0;
     }
 
     /**
diff --git a/src/com/vaadin/terminal/gwt/client/ui/VScrollTable.java b/src/com/vaadin/terminal/gwt/client/ui/VScrollTable.java
index 3e57378a22..5c55a432b7 100644
--- a/src/com/vaadin/terminal/gwt/client/ui/VScrollTable.java
+++ b/src/com/vaadin/terminal/gwt/client/ui/VScrollTable.java
@@ -282,7 +282,7 @@ public class VScrollTable extends FlowPanel implements Table, ScrollHandler,
      */
     private boolean headerChangedDuringUpdate = false;
 
-    private final TableHead tHead = new TableHead();
+    protected final TableHead tHead = new TableHead();
 
     private final TableFooter tFoot = new TableFooter();
 
@@ -4218,21 +4218,7 @@ public class VScrollTable extends FlowPanel implements Table, ScrollHandler,
          */
         public void setColWidth(int colIndex, int w) {
             for (Widget row : renderedRows) {
-                TableRowElement tr = row.getElement().cast();
-                TableCellElement cell = tr.getCells().getItem(colIndex);
-                boolean spanned = false;
-                if (row instanceof VScrollTableGeneratedRow) {
-                    spanned = ((VScrollTableGeneratedRow) row).isSpanColumns();
-                }
-                if (!spanned) {
-                    cell.getFirstChildElement().getStyle()
-                            .setPropertyPx("width", w);
-                    cell.getStyle().setPropertyPx("width", w);
-                } else if (colIndex == 0) {
-                    cell.getFirstChildElement().getStyle()
-                            .clearProperty("width");
-                    cell.getStyle().clearProperty("width");
-                }
+                ((VScrollTableRow) row).setCellWidth(colIndex, w);
             }
         }
 
@@ -4304,10 +4290,11 @@ public class VScrollTable extends FlowPanel implements Table, ScrollHandler,
                 final VScrollTableRow row = (VScrollTableRow) rows.next();
 
                 final Element td = DOM.getChild(row.getElement(), oldIndex);
-                DOM.removeChild(row.getElement(), td);
-
-                DOM.insertChild(row.getElement(), td, newIndex);
+                if (td != null) {
+                    DOM.removeChild(row.getElement(), td);
 
+                    DOM.insertChild(row.getElement(), td, newIndex);
+                }
             }
 
         }
@@ -4332,8 +4319,8 @@ public class VScrollTable extends FlowPanel implements Table, ScrollHandler,
             protected final int rowKey;
             private List<UIDL> pendingComponentPaints;
 
-            protected String[] actionKeys = null;
-            protected final TableRowElement rowElement;
+            private String[] actionKeys = null;
+            private final TableRowElement rowElement;
             private boolean mDown;
             private int index;
             private Event touchStart;
@@ -4410,19 +4397,23 @@ public class VScrollTable extends FlowPanel implements Table, ScrollHandler,
             }
 
             protected void initCellWidths() {
-                final int cells = DOM.getChildCount(getElement());
+                final int cells = tHead.getVisibleCellCount();
                 for (int i = 0; i < cells; i++) {
-                    final Element cell = DOM.getChild(getElement(), i);
                     int w = VScrollTable.this.getColWidth(getColKeyByIndex(i));
                     if (w < 0) {
                         w = 0;
                     }
-                    cell.getFirstChildElement().getStyle()
-                            .setPropertyPx("width", w);
-                    cell.getStyle().setPropertyPx("width", w);
+                    setCellWidth(i, w);
                 }
             }
 
+            protected void setCellWidth(int cellIx, int width) {
+                final Element cell = DOM.getChild(getElement(), cellIx);
+                cell.getFirstChildElement().getStyle()
+                        .setPropertyPx("width", width);
+                cell.getStyle().setPropertyPx("width", width);
+            }
+
             protected void addCellsFromUIDL(UIDL uidl, char[] aligns, int col,
                     int visibleColumnIndex) {
                 final Iterator<?> cells = uidl.getChildIterator();
@@ -4446,7 +4437,7 @@ public class VScrollTable extends FlowPanel implements Table, ScrollHandler,
                     boolean sorted = tHead.getHeaderCell(col).isSorted();
                     if (cell instanceof String) {
                         addCell(uidl, cell.toString(), aligns[col++], style,
-                                isRenderCellsAsHtml(), sorted, description);
+                                isRenderHtmlInCells(), sorted, description);
                     } else {
                         final Paintable cellContent = client
                                 .getPaintable((UIDL) cell);
@@ -4464,7 +4455,7 @@ public class VScrollTable extends FlowPanel implements Table, ScrollHandler,
              * 
              * @return always returns false in the default implementation
              */
-            protected boolean isRenderCellsAsHtml() {
+            protected boolean isRenderHtmlInCells() {
                 return false;
             }
 
@@ -5274,7 +5265,7 @@ public class VScrollTable extends FlowPanel implements Table, ScrollHandler,
                 };
             }
 
-            protected int getColIndexOf(Widget child) {
+            private int getColIndexOf(Widget child) {
                 com.google.gwt.dom.client.Element widgetCell = child
                         .getElement().getParentElement().getParentElement();
                 NodeList<TableCellElement> cells = rowElement.getCells();
@@ -5323,33 +5314,52 @@ public class VScrollTable extends FlowPanel implements Table, ScrollHandler,
         protected class VScrollTableGeneratedRow extends VScrollTableRow {
 
             private boolean spanColumns;
-            private boolean renderAsHtml;
+            private boolean htmlContentAllowed;
 
             public VScrollTableGeneratedRow(UIDL uidl, char[] aligns) {
                 super(uidl, aligns);
                 addStyleName("v-table-generated-row");
             }
 
+            public boolean isSpanColumns() {
+                return spanColumns;
+            }
+
             @Override
             protected void initCellWidths() {
-                if (!spanColumns) {
+                if (spanColumns) {
+                    setSpannedColumnWidthAfterDOMFullyInited();
+                } else {
                     super.initCellWidths();
                 }
             }
 
-            public boolean isSpanColumns() {
-                return spanColumns;
+            private void setSpannedColumnWidthAfterDOMFullyInited() {
+                // Defer setting width on spanned columns to make sure that
+                // they are added to the DOM before trying to calculate
+                // widths.
+                Scheduler.get().scheduleDeferred(new ScheduledCommand() {
+
+                    public void execute() {
+                        if (showRowHeaders) {
+                            setCellWidth(0, tHead.getHeaderCell(0).getWidth());
+                            calcAndSetSpanWidthOnCell(1);
+                        } else {
+                            calcAndSetSpanWidthOnCell(0);
+                        }
+                    }
+                });
             }
 
             @Override
-            protected boolean isRenderCellsAsHtml() {
-                return renderAsHtml;
+            protected boolean isRenderHtmlInCells() {
+                return htmlContentAllowed;
             }
 
             @Override
             protected void addCellsFromUIDL(UIDL uidl, char[] aligns, int col,
                     int visibleColumnIndex) {
-                renderAsHtml = uidl.getBooleanAttribute("gen_html");
+                htmlContentAllowed = uidl.getBooleanAttribute("gen_html");
                 spanColumns = uidl.getBooleanAttribute("gen_span");
 
                 final Iterator<?> cells = uidl.getChildIterator();
@@ -5359,7 +5369,8 @@ public class VScrollTable extends FlowPanel implements Table, ScrollHandler,
                         final Object cell = cells.next();
                         if (cell instanceof String) {
                             addSpannedCell(uidl, cell.toString(), aligns[0],
-                                    "", renderAsHtml, false, null, colCount);
+                                    "", htmlContentAllowed, false, null,
+                                    colCount);
                         } else {
                             addSpannedCell(uidl, (Widget) cell, aligns[0], "",
                                     false, colCount);
@@ -5376,6 +5387,7 @@ public class VScrollTable extends FlowPanel implements Table, ScrollHandler,
                 TableCellElement td = DOM.createTD().cast();
                 td.setColSpan(colCount);
                 initCellWithWidget(w, align, style, sorted, td);
+                td.getStyle().setHeight(getRowHeight(), Unit.PX);
             }
 
             private void addSpannedCell(UIDL rowUidl, String text, char align,
@@ -5386,6 +5398,37 @@ public class VScrollTable extends FlowPanel implements Table, ScrollHandler,
                 td.setColSpan(colCount);
                 initCellWithText(text, align, style, textIsHTML, sorted,
                         description, td);
+                td.getStyle().setHeight(getRowHeight(), Unit.PX);
+            }
+
+            @Override
+            protected void setCellWidth(int cellIx, int width) {
+                if (isSpanColumns()) {
+                    if (showRowHeaders) {
+                        if (cellIx == 0) {
+                            super.setCellWidth(0, width);
+                        } else {
+                            // We need to recalculate the spanning TDs width for
+                            // every cellIx in order to support column resizing.
+                            calcAndSetSpanWidthOnCell(1);
+                        }
+                    } else {
+                        // Same as above.
+                        calcAndSetSpanWidthOnCell(0);
+                    }
+                } else {
+                    super.setCellWidth(cellIx, width);
+                }
+            }
+
+            private void calcAndSetSpanWidthOnCell(final int cellIx) {
+                int spanWidth = 0;
+                for (int ix = (showRowHeaders ? 1 : 0); ix < tHead
+                        .getVisibleCellCount(); ix++) {
+                    spanWidth += tHead.getHeaderCell(ix).getOffsetWidth();
+                }
+                Util.setWidthExcludingPaddingAndBorder((Element) getElement()
+                        .getChild(cellIx), spanWidth, 13, false);
             }
         }
 
diff --git a/src/com/vaadin/terminal/gwt/client/ui/VTreeTable.java b/src/com/vaadin/terminal/gwt/client/ui/VTreeTable.java
index aa2ebed23d..ecc065e117 100644
--- a/src/com/vaadin/terminal/gwt/client/ui/VTreeTable.java
+++ b/src/com/vaadin/terminal/gwt/client/ui/VTreeTable.java
@@ -9,6 +9,8 @@ import java.util.Iterator;
 import java.util.List;
 
 import com.google.gwt.animation.client.Animation;
+import com.google.gwt.core.client.Scheduler;
+import com.google.gwt.core.client.Scheduler.ScheduledCommand;
 import com.google.gwt.dom.client.Document;
 import com.google.gwt.dom.client.ImageElement;
 import com.google.gwt.dom.client.SpanElement;
@@ -26,6 +28,7 @@ import com.vaadin.terminal.gwt.client.BrowserInfo;
 import com.vaadin.terminal.gwt.client.ComputedStyle;
 import com.vaadin.terminal.gwt.client.RenderSpace;
 import com.vaadin.terminal.gwt.client.UIDL;
+import com.vaadin.terminal.gwt.client.Util;
 import com.vaadin.terminal.gwt.client.ui.VScrollTable.VScrollTableBody.VScrollTableRow;
 import com.vaadin.terminal.gwt.client.ui.VTreeTable.VTreeTableScrollBody.VTreeTableRow;
 
@@ -281,35 +284,53 @@ public class VTreeTable extends VScrollTable {
         }
 
         protected class VTreeTableGeneratedRow extends VTreeTableRow {
-
             private boolean spanColumns;
-            private boolean renderAsHtml;
+            private boolean htmlContentAllowed;
 
             public VTreeTableGeneratedRow(UIDL uidl, char[] aligns) {
                 super(uidl, aligns);
                 addStyleName("v-table-generated-row");
             }
 
+            public boolean isSpanColumns() {
+                return spanColumns;
+            }
+
             @Override
             protected void initCellWidths() {
-                if (!spanColumns) {
+                if (spanColumns) {
+                    setSpannedColumnWidthAfterDOMFullyInited();
+                } else {
                     super.initCellWidths();
                 }
             }
 
-            public boolean isSpanColumns() {
-                return spanColumns;
+            private void setSpannedColumnWidthAfterDOMFullyInited() {
+                // Defer setting width on spanned columns to make sure that
+                // they are added to the DOM before trying to calculate
+                // widths.
+                Scheduler.get().scheduleDeferred(new ScheduledCommand() {
+
+                    public void execute() {
+                        if (showRowHeaders) {
+                            setCellWidth(0, tHead.getHeaderCell(0).getWidth());
+                            calcAndSetSpanWidthOnCell(1);
+                        } else {
+                            calcAndSetSpanWidthOnCell(0);
+                        }
+                    }
+                });
             }
 
             @Override
-            protected boolean isRenderCellsAsHtml() {
-                return renderAsHtml;
+            protected boolean isRenderHtmlInCells() {
+                return htmlContentAllowed;
             }
 
             @Override
             protected void addCellsFromUIDL(UIDL uidl, char[] aligns, int col,
                     int visibleColumnIndex) {
-                renderAsHtml = uidl.getBooleanAttribute("gen_html");
+                htmlContentAllowed = uidl.getBooleanAttribute("gen_html");
                 spanColumns = uidl.getBooleanAttribute("gen_span");
 
                 final Iterator<?> cells = uidl.getChildIterator();
@@ -319,7 +340,8 @@ public class VTreeTable extends VScrollTable {
                         final Object cell = cells.next();
                         if (cell instanceof String) {
                             addSpannedCell(uidl, cell.toString(), aligns[0],
-                                    "", renderAsHtml, false, null, colCount);
+                                    "", htmlContentAllowed, false, null,
+                                    colCount);
                         } else {
                             addSpannedCell(uidl, (Widget) cell, aligns[0], "",
                                     false, colCount);
@@ -336,6 +358,7 @@ public class VTreeTable extends VScrollTable {
                 TableCellElement td = DOM.createTD().cast();
                 td.setColSpan(colCount);
                 initCellWithWidget(w, align, style, sorted, td);
+                td.getStyle().setHeight(getRowHeight(), Unit.PX);
                 if (addTreeSpacer(rowUidl)) {
                     widgetInHierarchyColumn = w;
                 }
@@ -349,8 +372,39 @@ public class VTreeTable extends VScrollTable {
                 td.setColSpan(colCount);
                 initCellWithText(text, align, style, textIsHTML, sorted,
                         description, td);
+                td.getStyle().setHeight(getRowHeight(), Unit.PX);
                 addTreeSpacer(rowUidl);
             }
+
+            @Override
+            protected void setCellWidth(int cellIx, int width) {
+                if (isSpanColumns()) {
+                    if (showRowHeaders) {
+                        if (cellIx == 0) {
+                            super.setCellWidth(0, width);
+                        } else {
+                            // We need to recalculate the spanning TDs width for
+                            // every cellIx in order to support column resizing.
+                            calcAndSetSpanWidthOnCell(1);
+                        }
+                    } else {
+                        // Same as above.
+                        calcAndSetSpanWidthOnCell(0);
+                    }
+                } else {
+                    super.setCellWidth(cellIx, width);
+                }
+            }
+
+            private void calcAndSetSpanWidthOnCell(final int cellIx) {
+                int spanWidth = 0;
+                for (int ix = (showRowHeaders ? 1 : 0); ix < tHead
+                        .getVisibleCellCount(); ix++) {
+                    spanWidth += tHead.getHeaderCell(ix).getOffsetWidth();
+                }
+                Util.setWidthExcludingPaddingAndBorder((Element) getElement()
+                        .getChild(cellIx), spanWidth, 13, false);
+            }
         }
 
         private int getIdentWidth() {
@@ -775,6 +829,6 @@ public class VTreeTable extends VScrollTable {
         // Make sure that initializedAndAttached & al are not reset when the
         // totalrows are updated on expand/collapse requests.
         int newTotalRows = uidl.getIntAttribute("totalrows");
-        setTotalRows(newTotalRows);        
+        setTotalRows(newTotalRows);
     }
 }
diff --git a/src/com/vaadin/ui/Table.java b/src/com/vaadin/ui/Table.java
index 09625d88d6..2b13467481 100644
--- a/src/com/vaadin/ui/Table.java
+++ b/src/com/vaadin/ui/Table.java
@@ -2986,7 +2986,7 @@ public class Table extends AbstractSelect implements Action.Container,
             int indexInRowBuffer) throws PaintException {
         GeneratedRow generatedRow = (GeneratedRow) cells[CELL_GENERATED_ROW][indexInRowBuffer];
         if (generatedRow != null) {
-            target.addAttribute("gen_html", generatedRow.isRenderAsHtml());
+            target.addAttribute("gen_html", generatedRow.isHtmlContentAllowed());
             target.addAttribute("gen_span", generatedRow.isSpanColumns());
         }
     }
@@ -4636,6 +4636,13 @@ public class Table extends AbstractSelect implements Action.Container,
         return itemDescriptionGenerator;
     }
 
+    /**
+     * Row generators can be used to replace certain items in a table with a
+     * generated string. The generator is called each time the table is
+     * rendered, which means that new strings can be generated each time.
+     * 
+     * Row generators can be used for e.g. summary rows or grouping of items.
+     */
     public interface RowGenerator {
         /**
          * Called for every row that is painted in the Table. Returning a
@@ -4671,7 +4678,7 @@ public class Table extends AbstractSelect implements Action.Container,
     }
 
     public static class GeneratedRow {
-        private boolean renderAsHtml = false;
+        private boolean htmlContentAllowed = false;
         private boolean spanColumns = false;
         private String[] text = null;
 
@@ -4682,8 +4689,8 @@ public class Table extends AbstractSelect implements Action.Container,
          * @param text
          */
         public GeneratedRow(String... text) {
-            setRenderAsHtml(false);
-            setSpanColumns(text.length == 1);
+            setHtmlContentAllowed(false);
+            setSpanColumns(text == null || text.length == 1);
             setText(text);
         }
 
@@ -4692,6 +4699,9 @@ public class Table extends AbstractSelect implements Action.Container,
          * column otherwise
          */
         public void setText(String... text) {
+            if (text == null || (text.length == 1 && text[0] == null)) {
+                text = new String[] { "" };
+            }
             this.text = text;
         }
 
@@ -4699,18 +4709,18 @@ public class Table extends AbstractSelect implements Action.Container,
             return text;
         }
 
-        protected boolean isRenderAsHtml() {
-            return renderAsHtml;
+        protected boolean isHtmlContentAllowed() {
+            return htmlContentAllowed;
         }
 
         /**
          * If set to true, all strings passed to {@link #setText(String...)}
          * will be rendered as HTML.
          * 
-         * @param renderAsHtml
+         * @param htmlContentAllowed
          */
-        public void setRenderAsHtml(boolean renderAsHtml) {
-            this.renderAsHtml = renderAsHtml;
+        public void setHtmlContentAllowed(boolean htmlContentAllowed) {
+            this.htmlContentAllowed = htmlContentAllowed;
         }
 
         protected boolean isSpanColumns() {