#8500 Properly repaint parent (hierarchy) when the visibility of a child

changes

3 files changed, 18 insertions, 68 deletions

diff --git a/src/com/vaadin/ui/AbstractComponent.java b/src/com/vaadin/ui/AbstractComponent.java
index 500b5afaaf..db6be1e4b8 100644
--- a/src/com/vaadin/ui/AbstractComponent.java
+++ b/src/com/vaadin/ui/AbstractComponent.java
@@ -440,11 +440,12 @@ public abstract class AbstractComponent implements Component, MethodEventSource
     public void setVisible(boolean visible) {
         if (getState().isVisible() != visible) {
             getState().setVisible(visible);
-            // Instead of requesting repaint normally we
-            // fire the event directly to assure that the
-            // event goes through event in the component might
-            // now be invisible
-            fireRequestRepaintEvent(null);
+            requestRepaint();
+            if (getParent() != null) {
+                // Must always repaint the parent (at least the hierarchy) when
+                // visibility of a child component changes.
+                getParent().requestRepaint();
+            }
         }
     }
 
@@ -641,14 +642,6 @@ public abstract class AbstractComponent implements Component, MethodEventSource
     public void attach() {
         getRoot().componentAttached(this);
         requestRepaint();
-        if (!getState().isVisible()) {
-            /*
-             * Bypass the repaint optimization in childRequestedRepaint method
-             * when attaching. When reattaching (possibly moving) -> must
-             * repaint
-             */
-            fireRequestRepaintEvent(null);
-        }
         if (delayedFocus) {
             focus();
         }
@@ -889,22 +882,13 @@ public abstract class AbstractComponent implements Component, MethodEventSource
 
     /* Documentation copied from interface */
     public void requestRepaint() {
-
-        // The effect of the repaint request is identical to case where a
-        // child requests repaint
-        childRequestedRepaint(null);
-    }
-
-    /* Documentation copied from interface */
-    public void childRequestedRepaint(
-            Collection<RepaintRequestListener> alreadyNotified) {
         // Invisible components (by flag in this particular component) do not
         // need repaints
         if (!getState().isVisible()) {
             return;
         }
 
-        fireRequestRepaintEvent(alreadyNotified);
+        fireRequestRepaintEvent();
     }
 
     /**
@@ -912,31 +896,17 @@ public abstract class AbstractComponent implements Component, MethodEventSource
      * 
      * @param alreadyNotified
      */
-    private void fireRequestRepaintEvent(
-            Collection<RepaintRequestListener> alreadyNotified) {
-        // Notify listeners only once
+    // Notify listeners only once
+    private void fireRequestRepaintEvent() {
         // Notify the listeners
         if (repaintRequestListeners != null
                 && !repaintRequestListeners.isEmpty()) {
             final Object[] listeners = repaintRequestListeners.toArray();
             final RepaintRequestEvent event = new RepaintRequestEvent(this);
             for (int i = 0; i < listeners.length; i++) {
-                if (alreadyNotified == null) {
-                    alreadyNotified = new LinkedList<RepaintRequestListener>();
-                }
-                if (!alreadyNotified.contains(listeners[i])) {
-                    ((RepaintRequestListener) listeners[i])
-                            .repaintRequested(event);
-                    alreadyNotified.add((RepaintRequestListener) listeners[i]);
-                }
+                ((RepaintRequestListener) listeners[i]).repaintRequested(event);
             }
         }
-
-        // Notify the parent
-        final Component parent = getParent();
-        if (parent != null) {
-            parent.childRequestedRepaint(alreadyNotified);
-        }
     }
 
     /* Documentation copied from interface */
diff --git a/src/com/vaadin/ui/Component.java b/src/com/vaadin/ui/Component.java
index c0c43fcec3..3fe71dfea1 100644
--- a/src/com/vaadin/ui/Component.java
+++ b/src/com/vaadin/ui/Component.java
@@ -5,7 +5,6 @@
 package com.vaadin.ui;
 
 import java.io.Serializable;
-import java.util.Collection;
 import java.util.EventListener;
 import java.util.EventObject;
 import java.util.Locale;
@@ -707,33 +706,6 @@ public interface Component extends ClientConnector, Paintable, VariableOwner,
      */
     public ComponentState getState();
 
-    /**
-     * The child components of the component must call this method when they
-     * need repainting. The call must be made even in the case in which the
-     * children sent the repaint request themselves.
-     * 
-     * <p>
-     * A repaint request is ignored if the component is invisible.
-     * </p>
-     * 
-     * <p>
-     * This method is called automatically by {@link AbstractComponent}, which
-     * also provides a default implementation of it. As this is a somewhat
-     * internal feature, it is rarely necessary to reimplement this or call it
-     * explicitly.
-     * </p>
-     * 
-     * @param alreadyNotified
-     *            the collection of repaint request listeners that have been
-     *            already notified by the child. This component should not
-     *            re-notify the listed listeners again. The container given as
-     *            parameter must be modifiable as the component might modify it
-     *            and pass it forward. A {@code null} parameter is interpreted
-     *            as an empty collection.
-     */
-    public void childRequestedRepaint(
-            Collection<RepaintRequestListener> alreadyNotified);
-
     /* Component event framework */
 
     /**
diff --git a/src/com/vaadin/ui/Form.java b/src/com/vaadin/ui/Form.java
index c39f043b21..d66fae76ca 100644
--- a/src/com/vaadin/ui/Form.java
+++ b/src/com/vaadin/ui/Form.java
@@ -851,6 +851,9 @@ public class Form extends AbstractField<Object> implements Item.Editor,
         newLayout.setParent(this);
         layout = newLayout;
 
+        // Hierarchy has changed so we need to repaint (this could be a
+        // hierarchy repaint only)
+        requestRepaint();
     }
 
     /**
@@ -1293,6 +1296,11 @@ public class Form extends AbstractField<Object> implements Item.Editor,
         }
         formFooter = newFormFooter;
         formFooter.setParent(this);
+
+        // Hierarchy has changed so we need to repaint (this could be a
+        // hierarchy repaint only)
+        requestRepaint();
+
     }
 
     @Override