Merge commit 'ecd026'

Conflicts:
	src/com/vaadin/ui/TreeTable.java

6 files changed, 224 insertions, 7 deletions

diff --git a/src/com/vaadin/terminal/gwt/client/ui/dd/DDUtil.java b/src/com/vaadin/terminal/gwt/client/ui/dd/DDUtil.java
index 02c1fe5061..97f5eb86fd 100644
--- a/src/com/vaadin/terminal/gwt/client/ui/dd/DDUtil.java
+++ b/src/com/vaadin/terminal/gwt/client/ui/dd/DDUtil.java
@@ -5,6 +5,7 @@ package com.vaadin.terminal.gwt.client.ui.dd;
 
 import com.google.gwt.dom.client.NativeEvent;
 import com.google.gwt.user.client.Element;
+import com.google.gwt.user.client.Window;
 import com.vaadin.terminal.gwt.client.Util;
 
 public class DDUtil {
@@ -43,8 +44,11 @@ public class DDUtil {
     public static VerticalDropLocation getVerticalDropLocation(Element element,
             int offsetHeight, int clientY, double topBottomRatio) {
 
-        int absoluteTop = element.getAbsoluteTop();
-        int fromTop = clientY - absoluteTop;
+        // Event coordinates are relative to the viewport, element absolute
+        // position is relative to the document. Make element position relative
+        // to viewport by adjusting for viewport scrolling. See #6021
+        int elementTop = element.getAbsoluteTop() - Window.getScrollTop();
+        int fromTop = clientY - elementTop;
 
         float percentageFromTop = (fromTop / (float) offsetHeight);
         if (percentageFromTop < topBottomRatio) {
@@ -74,14 +78,17 @@ public class DDUtil {
     public static HorizontalDropLocation getHorizontalDropLocation(
             Element element, int clientX, double leftRightRatio) {
 
-        int absoluteLeft = element.getAbsoluteLeft();
+        // Event coordinates are relative to the viewport, element absolute
+        // position is relative to the document. Make element position relative
+        // to viewport by adjusting for viewport scrolling. See #6021
+        int elementLeft = element.getAbsoluteLeft() - Window.getScrollLeft();
         int offsetWidth = element.getOffsetWidth();
-        int fromTop = clientX - absoluteLeft;
+        int fromLeft = clientX - elementLeft;
 
-        float percentageFromTop = (fromTop / (float) offsetWidth);
-        if (percentageFromTop < leftRightRatio) {
+        float percentageFromLeft = (fromLeft / (float) offsetWidth);
+        if (percentageFromLeft < leftRightRatio) {
             return HorizontalDropLocation.LEFT;
-        } else if (percentageFromTop > 1 - leftRightRatio) {
+        } else if (percentageFromLeft > 1 - leftRightRatio) {
             return HorizontalDropLocation.RIGHT;
         } else {
             return HorizontalDropLocation.CENTER;
diff --git a/src/com/vaadin/ui/TreeTable.java b/src/com/vaadin/ui/TreeTable.java
index 9607add2c9..74faac4781 100644
--- a/src/com/vaadin/ui/TreeTable.java
+++ b/src/com/vaadin/ui/TreeTable.java
@@ -84,6 +84,7 @@ public class TreeTable extends Table implements Hierarchical {
          * Consider adding getDepth to {@link Collapsible}, might help
          * scalability with some container implementations.
          */
+        @Override
         public int getDepth(Object itemId) {
             int depth = 0;
             Hierarchical hierarchicalContainer = getContainerDataSource();
@@ -94,6 +95,7 @@ public class TreeTable extends Table implements Hierarchical {
             return depth;
         }
 
+        @Override
         public void containerItemSetChange(ItemSetChangeEvent event) {
         }
 
@@ -112,44 +114,54 @@ public class TreeTable extends Table implements Hierarchical {
             return (Collapsible) getContainerDataSource();
         }
 
+        @Override
         public void toggleChildVisibility(Object itemId) {
             c().setCollapsed(itemId, !c().isCollapsed(itemId));
         }
 
+        @Override
         public boolean isNodeOpen(Object itemId) {
             return !c().isCollapsed(itemId);
         }
 
+        @Override
         public int size() {
             return TreeTable.super.size();
         }
 
+        @Override
         public Object getIdByIndex(int index) {
             return TreeTable.super.getIdByIndex(index);
         }
 
+        @Override
         public int indexOfId(Object id) {
             return TreeTable.super.indexOfId(id);
         }
 
+        @Override
         public boolean isLastId(Object itemId) {
             // using the default impl
             return TreeTable.super.isLastId(itemId);
         }
 
+        @Override
         public Object lastItemId() {
             // using the default impl
             return TreeTable.super.lastItemId();
         }
 
+        @Override
         public Object nextItemId(Object itemId) {
             return TreeTable.super.nextItemId(itemId);
         }
 
+        @Override
         public Object prevItemId(Object itemId) {
             return TreeTable.super.prevItemId(itemId);
         }
 
+        @Override
         public Collection<?> getItemIds() {
             return TreeTable.super.getItemIds();
         }
@@ -167,18 +179,22 @@ public class TreeTable extends Table implements Hierarchical {
 
         private final HashSet<Object> openItems = new HashSet<Object>();
 
+        @Override
         public boolean isNodeOpen(Object itemId) {
             return openItems.contains(itemId);
         }
 
+        @Override
         public int size() {
             return getPreOrder().size();
         }
 
+        @Override
         public Collection<Object> getItemIds() {
             return Collections.unmodifiableCollection(getPreOrder());
         }
 
+        @Override
         public boolean isLastId(Object itemId) {
             if (itemId == null) {
                 return false;
@@ -187,6 +203,7 @@ public class TreeTable extends Table implements Hierarchical {
             return itemId.equals(lastItemId());
         }
 
+        @Override
         public Object lastItemId() {
             if (getPreOrder().size() > 0) {
                 return getPreOrder().get(getPreOrder().size() - 1);
@@ -195,6 +212,7 @@ public class TreeTable extends Table implements Hierarchical {
             }
         }
 
+        @Override
         public Object nextItemId(Object itemId) {
             int indexOf = getPreOrder().indexOf(itemId);
             if (indexOf == -1) {
@@ -208,6 +226,7 @@ public class TreeTable extends Table implements Hierarchical {
             }
         }
 
+        @Override
         public Object prevItemId(Object itemId) {
             int indexOf = getPreOrder().indexOf(itemId);
             indexOf--;
@@ -218,6 +237,7 @@ public class TreeTable extends Table implements Hierarchical {
             }
         }
 
+        @Override
         public void toggleChildVisibility(Object itemId) {
             boolean removed = openItems.remove(itemId);
             if (!removed) {
@@ -267,10 +287,12 @@ public class TreeTable extends Table implements Hierarchical {
 
         }
 
+        @Override
         public int indexOfId(Object id) {
             return getPreOrder().indexOf(id);
         }
 
+        @Override
         public Object getIdByIndex(int index) {
             return getPreOrder().get(index);
         }
@@ -612,36 +634,44 @@ public class TreeTable extends Table implements Hierarchical {
         return getContainerStrategy().getItemIds();
     }
 
+    @Override
     public boolean areChildrenAllowed(Object itemId) {
         return getContainerDataSource().areChildrenAllowed(itemId);
     }
 
+    @Override
     public Collection<?> getChildren(Object itemId) {
         return getContainerDataSource().getChildren(itemId);
     }
 
+    @Override
     public Object getParent(Object itemId) {
         return getContainerDataSource().getParent(itemId);
     }
 
+    @Override
     public boolean hasChildren(Object itemId) {
         return getContainerDataSource().hasChildren(itemId);
     }
 
+    @Override
     public boolean isRoot(Object itemId) {
         return getContainerDataSource().isRoot(itemId);
     }
 
+    @Override
     public Collection<?> rootItemIds() {
         return getContainerDataSource().rootItemIds();
     }
 
+    @Override
     public boolean setChildrenAllowed(Object itemId, boolean areChildrenAllowed)
             throws UnsupportedOperationException {
         return getContainerDataSource().setChildrenAllowed(itemId,
                 areChildrenAllowed);
     }
 
+    @Override
     public boolean setParent(Object itemId, Object newParentId)
             throws UnsupportedOperationException {
         return getContainerDataSource().setParent(itemId, newParentId);
diff --git a/tests/server-side/com/vaadin/tests/server/component/tabsheet/TestTabSheet.java b/tests/server-side/com/vaadin/tests/server/component/tabsheet/TestTabSheet.java
index 40d0ffd17d..0ef8ae5a76 100644
--- a/tests/server-side/com/vaadin/tests/server/component/tabsheet/TestTabSheet.java
+++ b/tests/server-side/com/vaadin/tests/server/component/tabsheet/TestTabSheet.java
@@ -2,6 +2,8 @@ package com.vaadin.tests.server.component.tabsheet;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
 
 import java.util.Iterator;
 
@@ -158,4 +160,65 @@ public class TestTabSheet {
         tabSheet.setSelectedTab(123);
         assertEquals(tab1.getComponent(), tabSheet.getSelectedTab());
     }
+
+    @Test
+    public void replaceComponent() {
+        TabSheet tabSheet = new TabSheet();
+        Label lbl1 = new Label("aaa");
+        Label lbl2 = new Label("bbb");
+        Label lbl3 = new Label("ccc");
+        Label lbl4 = new Label("ddd");
+
+        Tab tab1 = tabSheet.addTab(lbl1);
+        tab1.setCaption("tab1");
+        tab1.setClosable(true);
+        Tab tab2 = tabSheet.addTab(lbl2);
+        tab2.setDescription("description");
+        tab2.setEnabled(false);
+
+        // Replace component not in tabsheet with one already in tabsheet -
+        // should be no-op
+        tabSheet.replaceComponent(lbl3, lbl2);
+        assertEquals(2, tabSheet.getComponentCount());
+        assertSame(tab1, tabSheet.getTab(lbl1));
+        assertSame(tab2, tabSheet.getTab(lbl2));
+        assertNull(tabSheet.getTab(lbl3));
+
+        // Replace component not in tabsheet with one not in tabsheet either
+        // should add lbl4 as last tab
+        tabSheet.replaceComponent(lbl3, lbl4);
+        assertEquals(3, tabSheet.getComponentCount());
+        assertSame(tab1, tabSheet.getTab(lbl1));
+        assertSame(tab2, tabSheet.getTab(lbl2));
+        assertEquals(2, tabSheet.getTabPosition(tabSheet.getTab(lbl4)));
+
+        // Replace component in tabsheet with another
+        // should swap places, tab association should stay the same but tabs
+        // should swap metadata
+        tabSheet.replaceComponent(lbl1, lbl2);
+        assertSame(tab1, tabSheet.getTab(lbl1));
+        assertSame(tab2, tabSheet.getTab(lbl2));
+        assertEquals(false, tab1.isClosable());
+        assertEquals(true, tab2.isClosable());
+        assertEquals(false, tab1.isEnabled());
+        assertEquals(true, tab2.isEnabled());
+        assertEquals("description", tab1.getDescription());
+        assertEquals(null, tab2.getDescription());
+        assertEquals(3, tabSheet.getComponentCount());
+        assertEquals(1, tabSheet.getTabPosition(tabSheet.getTab(lbl1)));
+        assertEquals(0, tabSheet.getTabPosition(tabSheet.getTab(lbl2)));
+
+        // Replace component in tabsheet with one not in tabsheet
+        // should create a new tab instance for the new component, old tab
+        // instance should become unattached
+        // tab metadata should be copied from old to new
+        tabSheet.replaceComponent(lbl1, lbl3);
+        assertEquals(3, tabSheet.getComponentCount());
+        assertNull(tabSheet.getTab(lbl1));
+        assertNull(tab1.getComponent());
+        assertNotNull(tabSheet.getTab(lbl3));
+        assertEquals(false, tabSheet.getTab(lbl3).isEnabled());
+        assertEquals("description", tab1.getDescription());
+        assertEquals(1, tabSheet.getTabPosition(tabSheet.getTab(lbl3)));
+    }
 }
diff --git a/tests/testbench/com/vaadin/tests/components/treetable/ComponentsInTreeTable.java b/tests/testbench/com/vaadin/tests/components/treetable/ComponentsInTreeTable.java
new file mode 100644
index 0000000000..5b7700bc6e
--- /dev/null
+++ b/tests/testbench/com/vaadin/tests/components/treetable/ComponentsInTreeTable.java
@@ -0,0 +1,50 @@
+package com.vaadin.tests.components.treetable;
+
+import com.vaadin.tests.components.TestBase;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Component;
+import com.vaadin.ui.HorizontalLayout;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.Layout;
+import com.vaadin.ui.TreeTable;
+
+public class ComponentsInTreeTable extends TestBase {
+
+    @Override
+    protected void setup() {
+        TreeTable tt = new TreeTable();
+        tt.setWidth("300px");
+        addComponent(tt);
+
+        Object id, id2;
+
+        tt.addContainerProperty("foo", Component.class, "");
+        tt.addContainerProperty("bar", String.class, "bar");
+        tt.addContainerProperty("baz", String.class, "baz");
+
+        id = tt.addItem();
+        Layout l = new HorizontalLayout();
+        l.addComponent(new Label("bar"));
+        l.addComponent(new Label("bar"));
+        tt.getContainerProperty(id, "foo").setValue(l);
+
+        id = tt.addItem();
+        Label lbl = new Label("<b>foo</b><br/><i>bar</i>");
+        lbl.setContentMode(Label.CONTENT_XHTML);
+        tt.getContainerProperty(id, "foo").setValue(lbl);
+
+        id2 = tt.addItem();
+        tt.setParent(id2, id);
+        tt.getContainerProperty(id2, "foo").setValue(new Button("Test"));
+    }
+
+    @Override
+    protected String getDescription() {
+        return "Components in TreeTable cells should be rendered inline with the expand/collapse arrow";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 7387;
+    }
+}
diff --git a/tests/testbench/com/vaadin/tests/dd/ScrolledDropTarget.html.disabled b/tests/testbench/com/vaadin/tests/dd/ScrolledDropTarget.html.disabled
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/tests/testbench/com/vaadin/tests/dd/ScrolledDropTarget.html.disabled
diff --git a/tests/testbench/com/vaadin/tests/dd/ScrolledDropTarget.java b/tests/testbench/com/vaadin/tests/dd/ScrolledDropTarget.java
new file mode 100644
index 0000000000..093e12f84a
--- /dev/null
+++ b/tests/testbench/com/vaadin/tests/dd/ScrolledDropTarget.java
@@ -0,0 +1,67 @@
+package com.vaadin.tests.dd;
+
+import com.vaadin.event.dd.DragAndDropEvent;
+import com.vaadin.event.dd.DropHandler;
+import com.vaadin.event.dd.acceptcriteria.AcceptCriterion;
+import com.vaadin.terminal.gwt.client.ui.dd.VerticalDropLocation;
+import com.vaadin.tests.components.TestBase;
+import com.vaadin.tests.util.Log;
+import com.vaadin.ui.AbstractSelect.AbstractSelectTargetDetails;
+import com.vaadin.ui.AbstractSelect.VerticalLocationIs;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.Table;
+import com.vaadin.ui.Table.TableDragMode;
+
+public class ScrolledDropTarget extends TestBase {
+    private final Log log = new Log(5);
+
+    @Override
+    protected void setup() {
+
+        Table table = new Table();
+        table.addContainerProperty("A", String.class, "");
+        for (int i = 0; i < 100; i++) {
+            table.addItem(new Object[] { Integer.toString(i) },
+                    Integer.valueOf(i));
+        }
+
+        table.setDragMode(TableDragMode.ROW);
+        table.setDropHandler(new DropHandler() {
+            public AcceptCriterion getAcceptCriterion() {
+                return VerticalLocationIs.MIDDLE;
+            }
+
+            public void drop(DragAndDropEvent event) {
+                AbstractSelectTargetDetails targetDetails = (AbstractSelectTargetDetails) event
+                        .getTargetDetails();
+                VerticalDropLocation dropLocation = targetDetails
+                        .getDropLocation();
+                log.log("Drop at " + dropLocation + " relative to "
+                        + targetDetails.getItemIdOver());
+            }
+        });
+
+        addComponent(table);
+        addComponent(new Button("Scroll body", new Button.ClickListener() {
+            public void buttonClick(ClickEvent event) {
+                getMainWindow().executeJavaScript(
+                        "document.body.style.overflow = 'auto';"
+                                + "document.body.style.height = '200%';"
+                                + "window.scrollTo(0,18)");
+            }
+        }));
+        addComponent(log);
+    }
+
+    @Override
+    protected String getDescription() {
+        return "Vertical location for drags should work even when the browser window is scrolled";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return Integer.valueOf(6021);
+    }
+
+}