Hierarchy change markAsDirty to AbstractComponentContainer (#14227)

Change-Id: I7a05ad72dfa3d16d85aa4d0cdd235572ec01e31a

9 files changed, 59 insertions, 21 deletions

diff --git a/server/src/com/vaadin/ui/AbsoluteLayout.java b/server/src/com/vaadin/ui/AbsoluteLayout.java
index afc73f5ecc..af47981db6 100644
--- a/server/src/com/vaadin/ui/AbsoluteLayout.java
+++ b/server/src/com/vaadin/ui/AbsoluteLayout.java
@@ -153,7 +153,6 @@ public class AbsoluteLayout extends AbstractLayout implements
             internalRemoveComponent(c);
             throw e;
         }
-        markAsDirty();
     }
 
     /**
@@ -197,7 +196,6 @@ public class AbsoluteLayout extends AbstractLayout implements
     public void removeComponent(Component c) {
         internalRemoveComponent(c);
         super.removeComponent(c);
-        markAsDirty();
     }
 
     /**
diff --git a/server/src/com/vaadin/ui/AbstractComponentContainer.java b/server/src/com/vaadin/ui/AbstractComponentContainer.java
index e70b0fa0ce..1095331602 100644
--- a/server/src/com/vaadin/ui/AbstractComponentContainer.java
+++ b/server/src/com/vaadin/ui/AbstractComponentContainer.java
@@ -209,6 +209,7 @@ public abstract class AbstractComponentContainer extends AbstractComponent
 
         c.setParent(this);
         fireComponentAttachEvent(c);
+        markAsDirty();
     }
 
     /**
@@ -223,6 +224,7 @@ public abstract class AbstractComponentContainer extends AbstractComponent
         if (equals(c.getParent())) {
             c.setParent(null);
             fireComponentDetachEvent(c);
+            markAsDirty();
         }
     }
 
diff --git a/server/src/com/vaadin/ui/AbstractSplitPanel.java b/server/src/com/vaadin/ui/AbstractSplitPanel.java
index e9b37f8cff..a78f192fa2 100644
--- a/server/src/com/vaadin/ui/AbstractSplitPanel.java
+++ b/server/src/com/vaadin/ui/AbstractSplitPanel.java
@@ -214,7 +214,6 @@ public abstract class AbstractSplitPanel extends AbstractComponentContainer {
         } else if (c == getSecondComponent()) {
             getState().secondChild = null;
         }
-        markAsDirty();
     }
 
     /*
@@ -256,7 +255,6 @@ public abstract class AbstractSplitPanel extends AbstractComponentContainer {
         } else if (oldComponent == getSecondComponent()) {
             setSecondComponent(newComponent);
         }
-        markAsDirty();
     }
 
     /**
diff --git a/server/src/com/vaadin/ui/CssLayout.java b/server/src/com/vaadin/ui/CssLayout.java
index e7b63cc87a..350423576f 100644
--- a/server/src/com/vaadin/ui/CssLayout.java
+++ b/server/src/com/vaadin/ui/CssLayout.java
@@ -118,7 +118,6 @@ public class CssLayout extends AbstractLayout implements LayoutClickNotifier {
         components.add(c);
         try {
             super.addComponent(c);
-            markAsDirty();
         } catch (IllegalArgumentException e) {
             components.remove(c);
             throw e;
@@ -141,7 +140,6 @@ public class CssLayout extends AbstractLayout implements LayoutClickNotifier {
         components.addFirst(c);
         try {
             super.addComponent(c);
-            markAsDirty();
         } catch (IllegalArgumentException e) {
             components.remove(c);
             throw e;
@@ -170,7 +168,6 @@ public class CssLayout extends AbstractLayout implements LayoutClickNotifier {
         components.add(index, c);
         try {
             super.addComponent(c);
-            markAsDirty();
         } catch (IllegalArgumentException e) {
             components.remove(c);
             throw e;
@@ -187,7 +184,6 @@ public class CssLayout extends AbstractLayout implements LayoutClickNotifier {
     public void removeComponent(Component c) {
         components.remove(c);
         super.removeComponent(c);
-        markAsDirty();
     }
 
     /**
diff --git a/server/src/com/vaadin/ui/CustomLayout.java b/server/src/com/vaadin/ui/CustomLayout.java
index f4fe7fa66c..a9c266b0b9 100644
--- a/server/src/com/vaadin/ui/CustomLayout.java
+++ b/server/src/com/vaadin/ui/CustomLayout.java
@@ -144,8 +144,8 @@ public class CustomLayout extends AbstractLayout implements LegacyComponent {
         }
         slots.put(location, c);
         getState().childLocations.put(c, location);
-        c.setParent(this);
-        fireComponentAttachEvent(c);
+
+        super.addComponent(c);
     }
 
     /**
diff --git a/server/src/com/vaadin/ui/GridLayout.java b/server/src/com/vaadin/ui/GridLayout.java
index 0dd16a03e7..96854c5b1b 100644
--- a/server/src/com/vaadin/ui/GridLayout.java
+++ b/server/src/com/vaadin/ui/GridLayout.java
@@ -255,8 +255,6 @@ public class GridLayout extends AbstractLayout implements
                 cursorY = row1;
             }
         }
-
-        markAsDirty();
     }
 
     /**
@@ -390,7 +388,6 @@ public class GridLayout extends AbstractLayout implements
 
         getState().childData.remove(component);
         components.remove(component);
-
         super.removeComponent(component);
     }
 
diff --git a/server/src/com/vaadin/ui/TabSheet.java b/server/src/com/vaadin/ui/TabSheet.java
index d3410464a2..88002104b1 100644
--- a/server/src/com/vaadin/ui/TabSheet.java
+++ b/server/src/com/vaadin/ui/TabSheet.java
@@ -195,7 +195,6 @@ public class TabSheet extends AbstractComponentContainer implements Focusable,
         if (component != null && components.contains(component)) {
 
             int componentIndex = components.indexOf(component);
-
             super.removeComponent(component);
             keyMapper.remove(component);
             components.remove(component);
@@ -232,7 +231,6 @@ public class TabSheet extends AbstractComponentContainer implements Focusable,
                     fireSelectedTabChange();
                 }
             }
-            markAsDirty();
         }
     }
 
@@ -394,8 +392,9 @@ public class TabSheet extends AbstractComponentContainer implements Focusable,
                 setSelected(tabComponent);
                 fireSelectedTabChange();
             }
+
             super.addComponent(tabComponent);
-            markAsDirty();
+
             return tab;
         }
     }
@@ -967,16 +966,16 @@ public class TabSheet extends AbstractComponentContainer implements Focusable,
 
         /**
          * Gets the icon alt text for the tab.
-         *
+         * 
          * @since 7.2
          */
         public String getIconAlternateText();
 
         /**
          * Sets the icon alt text for the tab.
-         *
+         * 
          * @since 7.2
-         *
+         * 
          * @param iconAltText
          *            the icon to set
          */
diff --git a/server/tests/src/com/vaadin/tests/server/components/ComponentAttachDetachListenerTest.java b/server/tests/src/com/vaadin/tests/server/components/ComponentAttachDetachListenerTest.java
index df515795eb..d8b366ffbc 100644
--- a/server/tests/src/com/vaadin/tests/server/components/ComponentAttachDetachListenerTest.java
+++ b/server/tests/src/com/vaadin/tests/server/components/ComponentAttachDetachListenerTest.java
@@ -9,6 +9,7 @@ import com.vaadin.ui.AbsoluteLayout.ComponentPosition;
 import com.vaadin.ui.AbstractOrderedLayout;
 import com.vaadin.ui.Component;
 import com.vaadin.ui.CssLayout;
+import com.vaadin.ui.CustomLayout;
 import com.vaadin.ui.GridLayout;
 import com.vaadin.ui.GridLayout.Area;
 import com.vaadin.ui.HasComponents;
@@ -25,6 +26,7 @@ public class ComponentAttachDetachListenerTest extends TestCase {
     private GridLayout gridlayout;
     private AbsoluteLayout absolutelayout;
     private CssLayout csslayout;
+    private CustomLayout customlayout;
 
     // General variables
     private int attachCounter = 0;
@@ -143,6 +145,10 @@ public class ComponentAttachDetachListenerTest extends TestCase {
         csslayout = new CssLayout();
         csslayout.addComponentAttachListener(new MyAttachListener());
         csslayout.addComponentDetachListener(new MyDetachListener());
+
+        customlayout = new CustomLayout("<div location='loc'/>");
+        customlayout.addComponentAttachListener(new MyAttachListener());
+        customlayout.addComponentDetachListener(new MyDetachListener());
     }
 
     public void testOrderedLayoutAttachListener() {
@@ -342,4 +348,48 @@ public class ComponentAttachDetachListenerTest extends TestCase {
         // The detached component should not be found in the container
         assertFalse(foundInContainer);
     }
+
+    public void testCustomLayoutAttachListener() {
+        // Reset state variables
+        resetVariables();
+
+        // Add component -> Should trigger attach listener
+        Component comp = new Label();
+        customlayout.addComponent(comp, "loc");
+
+        assertEquals("Attach counter should get incremented", 1, attachCounter);
+
+        assertSame("The attached component should be the label", comp,
+                attachedComponent);
+
+        assertSame("The attached target should be the layout", customlayout,
+                attachTarget);
+
+        assertTrue("The attached component should be found in the container",
+                foundInContainer);
+    }
+
+    public void testCustomLayoutDetachListener() {
+        // Add a component to detach
+        Component comp = new Label();
+        customlayout.addComponent(comp);
+
+        // Reset state variables (since they are set by the attach listener)
+        resetVariables();
+
+        // Detach the component -> triggers the detach listener
+        customlayout.removeComponent(comp);
+
+        assertEquals("Detach counter should get incremented", 1, detachCounter);
+
+        assertSame("The detached component should be the label", comp,
+                detachedComponent);
+
+        assertSame("The detached target should be the layout", customlayout,
+                detachedTarget);
+
+        assertFalse(
+                "The detached component should not be found in the container",
+                foundInContainer);
+    }
 }
diff --git a/uitest/src/com/vaadin/tests/minitutorials/v7a2/WidgetContainer.java b/uitest/src/com/vaadin/tests/minitutorials/v7a2/WidgetContainer.java
index 8c14ba8bd7..850fa1044f 100644
--- a/uitest/src/com/vaadin/tests/minitutorials/v7a2/WidgetContainer.java
+++ b/uitest/src/com/vaadin/tests/minitutorials/v7a2/WidgetContainer.java
@@ -15,14 +15,12 @@ public class WidgetContainer extends AbstractComponentContainer {
     public void addComponent(Component c) {
         children.add(c);
         super.addComponent(c);
-        markAsDirty();
     }
 
     @Override
     public void removeComponent(Component c) {
         children.remove(c);
         super.removeComponent(c);
-        markAsDirty();
     }
 
     @Override