From 4a60f1a712787cfdadeddef399d177b25806017e Mon Sep 17 00:00:00 2001 From: Juuso Valli Date: Thu, 11 Sep 2014 16:34:03 +0300 Subject: [PATCH] Make Vaadin component handling proxy-friendly (#14639) Comparisons with the ==-operator between a proxy and it's underlying instance fail, so we should use a custom equals method instead. Change-Id: Iaa86ae830fecbedfb1f55636e25f5affebf5aba3 --- .../src/com/vaadin/event/ActionManager.java | 7 +++-- .../server/AbstractClientConnector.java | 31 ++++++++++++++++++- .../src/com/vaadin/ui/AbstractComponent.java | 4 +-- .../vaadin/ui/AbstractComponentContainer.java | 2 +- .../com/vaadin/ui/AbstractOrderedLayout.java | 4 +-- .../ui/AbstractSingleComponentContainer.java | 2 +- .../src/com/vaadin/ui/ConnectorTracker.java | 2 +- server/src/com/vaadin/ui/CssLayout.java | 4 +-- server/src/com/vaadin/ui/CustomComponent.java | 2 +- server/src/com/vaadin/ui/DateField.java | 2 +- server/src/com/vaadin/ui/PopupView.java | 2 +- server/src/com/vaadin/ui/TabSheet.java | 2 +- server/src/com/vaadin/ui/Table.java | 4 +-- server/src/com/vaadin/ui/UI.java | 6 ++-- 14 files changed, 53 insertions(+), 21 deletions(-) diff --git a/server/src/com/vaadin/event/ActionManager.java b/server/src/com/vaadin/event/ActionManager.java index b42a5d8dde..6eb698d08a 100644 --- a/server/src/com/vaadin/event/ActionManager.java +++ b/server/src/com/vaadin/event/ActionManager.java @@ -78,7 +78,10 @@ public class ActionManager implements Action.Container, Action.Handler, public void setViewer( T viewer) { - if (viewer == this.viewer) { + // This somewhat complicated check exists to make sure that proxies are + // handled correctly + if (this.viewer == viewer + || (this.viewer != null && this.viewer.equals(viewer))) { return; } if (this.viewer != null) { @@ -113,7 +116,7 @@ public class ActionManager implements Action.Container, Action.Handler, @Override public void addActionHandler(Handler actionHandler) { - if (actionHandler == this) { + if (equals(actionHandler)) { // don't add the actionHandler to itself return; } diff --git a/server/src/com/vaadin/server/AbstractClientConnector.java b/server/src/com/vaadin/server/AbstractClientConnector.java index 03300b20e2..d8d0d5cbd9 100644 --- a/server/src/com/vaadin/server/AbstractClientConnector.java +++ b/server/src/com/vaadin/server/AbstractClientConnector.java @@ -47,6 +47,7 @@ import com.vaadin.ui.HasComponents; import com.vaadin.ui.LegacyComponent; import com.vaadin.ui.UI; + /** * An abstract base class for ClientConnector implementations. This class * provides all the basic functionality required for connectors. @@ -549,7 +550,7 @@ public abstract class AbstractClientConnector implements ClientConnector, */ protected void addExtension(Extension extension) { ClientConnector previousParent = extension.getParent(); - if (previousParent == this) { + if (equals(previousParent)) { // Nothing to do, already attached return; } else if (previousParent != null) { @@ -999,4 +1000,32 @@ public abstract class AbstractClientConnector implements ClientConnector, public void setErrorHandler(ErrorHandler errorHandler) { this.errorHandler = errorHandler; } + + private AbstractClientConnector getInstance() { + // returns the underlying instance regardless of proxies + return this; + } + + /* + * (non-Javadoc) + * + * @see java.lang.Object#equals(java.lang.Object) + */ + @Override + public boolean equals(Object obj) { + if (obj instanceof AbstractClientConnector) { + return super.equals(((AbstractClientConnector) obj).getInstance()); + } + return false; + } + + /* + * (non-Javadoc) + * + * @see java.lang.Object#hashCode() + */ + @Override + public int hashCode() { + return super.hashCode(); + } } diff --git a/server/src/com/vaadin/ui/AbstractComponent.java b/server/src/com/vaadin/ui/AbstractComponent.java index c9639813ca..5c4fba739d 100644 --- a/server/src/com/vaadin/ui/AbstractComponent.java +++ b/server/src/com/vaadin/ui/AbstractComponent.java @@ -462,7 +462,7 @@ public abstract class AbstractComponent extends AbstractClientConnector @Override public void setParent(HasComponents parent) { // If the parent is not changed, don't do anything - if (parent == this.parent) { + if (parent == null ? this.parent == null : parent.equals(this.parent)) { return; } @@ -1005,7 +1005,7 @@ public abstract class AbstractComponent extends AbstractClientConnector if (content instanceof HasComponents) { for (Component parent = this; parent != null; parent = parent .getParent()) { - if (parent == content) { + if (parent.equals(content)) { return true; } } diff --git a/server/src/com/vaadin/ui/AbstractComponentContainer.java b/server/src/com/vaadin/ui/AbstractComponentContainer.java index b1e69ba76b..e70b0fa0ce 100644 --- a/server/src/com/vaadin/ui/AbstractComponentContainer.java +++ b/server/src/com/vaadin/ui/AbstractComponentContainer.java @@ -220,7 +220,7 @@ public abstract class AbstractComponentContainer extends AbstractComponent */ @Override public void removeComponent(Component c) { - if (c.getParent() == this) { + if (equals(c.getParent())) { c.setParent(null); fireComponentDetachEvent(c); } diff --git a/server/src/com/vaadin/ui/AbstractOrderedLayout.java b/server/src/com/vaadin/ui/AbstractOrderedLayout.java index 27880db75f..638f6bc3f9 100644 --- a/server/src/com/vaadin/ui/AbstractOrderedLayout.java +++ b/server/src/com/vaadin/ui/AbstractOrderedLayout.java @@ -105,7 +105,7 @@ public abstract class AbstractOrderedLayout extends AbstractLayout implements public void addComponentAsFirst(Component c) { // If c is already in this, we must remove it before proceeding // see ticket #7668 - if (c.getParent() == this) { + if (equals(c.getParent())) { removeComponent(c); } components.addFirst(c); @@ -131,7 +131,7 @@ public abstract class AbstractOrderedLayout extends AbstractLayout implements public void addComponent(Component c, int index) { // If c is already in this, we must remove it before proceeding // see ticket #7668 - if (c.getParent() == this) { + if (equals(c.getParent())) { // When c is removed, all components after it are shifted down if (index > getComponentIndex(c)) { index--; diff --git a/server/src/com/vaadin/ui/AbstractSingleComponentContainer.java b/server/src/com/vaadin/ui/AbstractSingleComponentContainer.java index ba108fc302..e7b5205f2d 100644 --- a/server/src/com/vaadin/ui/AbstractSingleComponentContainer.java +++ b/server/src/com/vaadin/ui/AbstractSingleComponentContainer.java @@ -134,7 +134,7 @@ public abstract class AbstractSingleComponentContainer extends // do not set the same content twice return; } - if (oldContent != null && oldContent.getParent() == this) { + if (oldContent != null && equals(oldContent.getParent())) { oldContent.setParent(null); fireComponentDetachEvent(oldContent); } diff --git a/server/src/com/vaadin/ui/ConnectorTracker.java b/server/src/com/vaadin/ui/ConnectorTracker.java index 9b8729f779..02d34582af 100644 --- a/server/src/com/vaadin/ui/ConnectorTracker.java +++ b/server/src/com/vaadin/ui/ConnectorTracker.java @@ -371,7 +371,7 @@ public class ConnectorTracker implements Serializable { for (ClientConnector child : children) { stack.add(child); - if (child.getParent() != connector) { + if (!connector.equals(child.getParent())) { noErrors = false; getLogger() .log(Level.WARNING, diff --git a/server/src/com/vaadin/ui/CssLayout.java b/server/src/com/vaadin/ui/CssLayout.java index 7fdae32bd1..e7b63cc87a 100644 --- a/server/src/com/vaadin/ui/CssLayout.java +++ b/server/src/com/vaadin/ui/CssLayout.java @@ -135,7 +135,7 @@ public class CssLayout extends AbstractLayout implements LayoutClickNotifier { public void addComponentAsFirst(Component c) { // If c is already in this, we must remove it before proceeding // see ticket #7668 - if (c.getParent() == this) { + if (equals(c.getParent())) { removeComponent(c); } components.addFirst(c); @@ -160,7 +160,7 @@ public class CssLayout extends AbstractLayout implements LayoutClickNotifier { public void addComponent(Component c, int index) { // If c is already in this, we must remove it before proceeding // see ticket #7668 - if (c.getParent() == this) { + if (equals(c.getParent())) { // When c is removed, all components after it are shifted down if (index > getComponentIndex(c)) { index--; diff --git a/server/src/com/vaadin/ui/CustomComponent.java b/server/src/com/vaadin/ui/CustomComponent.java index cfd109d175..7f20086bbf 100644 --- a/server/src/com/vaadin/ui/CustomComponent.java +++ b/server/src/com/vaadin/ui/CustomComponent.java @@ -107,7 +107,7 @@ public class CustomComponent extends AbstractComponent implements HasComponents */ protected void setCompositionRoot(Component compositionRoot) { if (compositionRoot != root) { - if (root != null && root.getParent() == this) { + if (root != null && equals(root.getParent())) { // remove old component root.setParent(null); } diff --git a/server/src/com/vaadin/ui/DateField.java b/server/src/com/vaadin/ui/DateField.java index ba1178548f..e88d767bc9 100644 --- a/server/src/com/vaadin/ui/DateField.java +++ b/server/src/com/vaadin/ui/DateField.java @@ -723,7 +723,7 @@ public class DateField extends AbstractField implements Collection visibleItemProperties = f.getItemPropertyIds(); for (Object fieldId : visibleItemProperties) { Field field = f.getField(fieldId); - if (field == this) { + if (equals(field)) { /* * this datefield is logically in a form. Do the same * thing as form does in its value change listener that diff --git a/server/src/com/vaadin/ui/PopupView.java b/server/src/com/vaadin/ui/PopupView.java index 90c60edc6e..2a2da26b62 100644 --- a/server/src/com/vaadin/ui/PopupView.java +++ b/server/src/com/vaadin/ui/PopupView.java @@ -152,7 +152,7 @@ public class PopupView extends AbstractComponent implements HasComponents { } visibleComponent.setParent(this); } else { - if (visibleComponent.getParent() == this) { + if (equals(visibleComponent.getParent())) { visibleComponent.setParent(null); } visibleComponent = null; diff --git a/server/src/com/vaadin/ui/TabSheet.java b/server/src/com/vaadin/ui/TabSheet.java index 8b13ecf1a4..313d060f1d 100644 --- a/server/src/com/vaadin/ui/TabSheet.java +++ b/server/src/com/vaadin/ui/TabSheet.java @@ -1174,7 +1174,7 @@ public class TabSheet extends AbstractComponentContainer implements Focusable, @Override public Component getComponent() { for (Map.Entry entry : tabs.entrySet()) { - if (entry.getValue() == this) { + if (equals(entry.getValue())) { return entry.getKey(); } } diff --git a/server/src/com/vaadin/ui/Table.java b/server/src/com/vaadin/ui/Table.java index 84e4eaed13..d574ed5ba8 100644 --- a/server/src/com/vaadin/ui/Table.java +++ b/server/src/com/vaadin/ui/Table.java @@ -2386,7 +2386,7 @@ public class Table extends AbstractSelect implements Action.Container, "Registered {0}: {1}", new Object[] { component.getClass().getSimpleName(), component.getCaption() }); - if (component.getParent() != this) { + if (!equals(component.getParent())) { component.setParent(this); } visibleComponents.add(component); @@ -4199,7 +4199,7 @@ public class Table extends AbstractSelect implements Action.Container, @Override public void valueChange(Property.ValueChangeEvent event) { - if (event.getProperty() == this + if (equals(event.getProperty()) || event.getProperty() == getPropertyDataSource()) { super.valueChange(event); } else { diff --git a/server/src/com/vaadin/ui/UI.java b/server/src/com/vaadin/ui/UI.java index 5abeea9480..d67e08828a 100644 --- a/server/src/com/vaadin/ui/UI.java +++ b/server/src/com/vaadin/ui/UI.java @@ -319,9 +319,9 @@ public abstract class UI extends AbstractSingleComponentContainer implements if (pendingFocus != null) { // ensure focused component is still attached to this main window - if (pendingFocus.getUI() == this - || (pendingFocus.getUI() != null && pendingFocus.getUI() - .getParent() == this)) { + if (equals(pendingFocus.getUI()) + || (pendingFocus.getUI() != null && equals(pendingFocus + .getUI().getParent()))) { target.addAttribute("focused", pendingFocus); } pendingFocus = null; -- 2.39.5