From 5003b33b3f237fc16a3ed221056defc6bf966cf0 Mon Sep 17 00:00:00 2001
From: Anna Koskinen <anna@vaadin.com>
Date: Fri, 7 Dec 2012 15:16:55 +0200
Subject: [PATCH] Default id should get overridden only if id has been
 specifically set (#10179).

Change-Id: I102ca64648a88e70e88d9c546e8ea49de47451fb
---
 .../VAADIN/themes/tests-tickets/styles.css    | 10 +++
 .../client/ui/AbstractComponentConnector.java | 17 +++--
 .../tests/components/ui/IdOverrideTest.html   | 64 ++++++++++++++++
 .../tests/components/ui/IdOverrideTest.java   | 73 +++++++++++++++++++
 .../client/IdTestLabelConnector.java          | 18 +++++
 .../tests/widgetset/client/VIdTestLabel.java  | 15 ++++
 .../tests/widgetset/server/IdTestLabel.java   | 16 ++++
 7 files changed, 208 insertions(+), 5 deletions(-)
 create mode 100644 uitest/src/com/vaadin/tests/components/ui/IdOverrideTest.html
 create mode 100644 uitest/src/com/vaadin/tests/components/ui/IdOverrideTest.java
 create mode 100644 uitest/src/com/vaadin/tests/widgetset/client/IdTestLabelConnector.java
 create mode 100644 uitest/src/com/vaadin/tests/widgetset/client/VIdTestLabel.java
 create mode 100644 uitest/src/com/vaadin/tests/widgetset/server/IdTestLabel.java

diff --git a/WebContent/VAADIN/themes/tests-tickets/styles.css b/WebContent/VAADIN/themes/tests-tickets/styles.css
index 2bd6536e49..c1dd89d6b8 100644
--- a/WebContent/VAADIN/themes/tests-tickets/styles.css
+++ b/WebContent/VAADIN/themes/tests-tickets/styles.css
@@ -316,6 +316,16 @@
 	border-left: 10px solid blue;
 }
 
+/*****************************************************************************/
+/* Ticket 10179
+/*****************************************************************************/
+#default10179 {
+	color:red;
+}
+
+#set10179 {
+	color:blue;
+}
 
 /*****************************************************************************/
 /* The reference screenshots all have a white background					 */
diff --git a/client/src/com/vaadin/client/ui/AbstractComponentConnector.java b/client/src/com/vaadin/client/ui/AbstractComponentConnector.java
index 16b5adef81..d11be76a80 100644
--- a/client/src/com/vaadin/client/ui/AbstractComponentConnector.java
+++ b/client/src/com/vaadin/client/ui/AbstractComponentConnector.java
@@ -17,6 +17,7 @@ package com.vaadin.client.ui;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Set;
 
 import com.google.gwt.dom.client.Element;
 import com.google.gwt.user.client.ui.Focusable;
@@ -53,6 +54,8 @@ public abstract class AbstractComponentConnector extends AbstractConnector
     private String lastKnownWidth = "";
     private String lastKnownHeight = "";
 
+    private boolean initialStateEvent = true;
+
     /**
      * The style names from getState().getStyles() which are currently applied
      * to the widget.
@@ -123,11 +126,13 @@ public abstract class AbstractComponentConnector extends AbstractConnector
     public void onStateChanged(StateChangeEvent stateChangeEvent) {
         ConnectorMap paintableMap = ConnectorMap.get(getConnection());
 
-        if (getState().id != null) {
-            getWidget().getElement().setId(getState().id);
-        } else {
-            getWidget().getElement().removeAttribute("id");
-
+        Set<String> changedProperties = stateChangeEvent.getChangedProperties();
+        if (changedProperties.contains("id")) {
+            if (getState().id != null) {
+                getWidget().getElement().setId(getState().id);
+            } else if (!initialStateEvent) {
+                getWidget().getElement().removeAttribute("id");
+            }
         }
 
         /*
@@ -164,6 +169,8 @@ public abstract class AbstractComponentConnector extends AbstractConnector
          */
 
         updateComponentSize();
+
+        initialStateEvent = false;
     }
 
     @Override
diff --git a/uitest/src/com/vaadin/tests/components/ui/IdOverrideTest.html b/uitest/src/com/vaadin/tests/components/ui/IdOverrideTest.html
new file mode 100644
index 0000000000..3a38712fb4
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/ui/IdOverrideTest.html
@@ -0,0 +1,64 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
+<head profile="http://selenium-ide.openqa.org/profiles/test-case">
+<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
+<link rel="selenium.base" href="http://localhost:8888/run/com.vaadin.tests.components.ui.IdOverrideTest?restartApplication" />
+<title>IdOverrideTest</title>
+</head>
+<body>
+<table cellpadding="1" cellspacing="1" border="1">
+<thead>
+<tr><td rowspan="1" colspan="3">IdOverrideTest</td></tr>
+</thead><tbody>
+<tr>
+	<td>open</td>
+	<td>/run/com.vaadin.tests.components.ui.IdOverrideTest?restartApplication</td>
+	<td></td>
+</tr>
+<!-- Ensure the default components are present with correct ids -->
+<tr>
+	<td>assertElementPresent</td>
+	<td>default10179</td>
+	<td></td>
+</tr>
+<tr>
+	<td>assertElementPresent</td>
+	<td>set10179</td>
+	<td></td>
+</tr>
+<!-- Remove id from the middle component -->
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentsuiIdOverrideTest::PID_Stoggle/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>assertElementPresent</td>
+	<td>default10179</td>
+	<td></td>
+</tr>
+<tr>
+	<td>assertElementNotPresent</td>
+	<td>set10179</td>
+	<td></td>
+</tr>
+<!-- Re-add id to the middle component -->
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentsuiIdOverrideTest::PID_Stoggle/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>assertElementPresent</td>
+	<td>default10179</td>
+	<td></td>
+</tr>
+<tr>
+	<td>assertElementPresent</td>
+	<td>set10179</td>
+	<td></td>
+</tr>
+</tbody></table>
+</body>
+</html>
\ No newline at end of file
diff --git a/uitest/src/com/vaadin/tests/components/ui/IdOverrideTest.java b/uitest/src/com/vaadin/tests/components/ui/IdOverrideTest.java
new file mode 100644
index 0000000000..eeadc89691
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/ui/IdOverrideTest.java
@@ -0,0 +1,73 @@
+package com.vaadin.tests.components.ui;
+
+import com.vaadin.annotations.Theme;
+import com.vaadin.annotations.Widgetset;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.shared.ui.MarginInfo;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.tests.widgetset.TestingWidgetSet;
+import com.vaadin.tests.widgetset.server.IdTestLabel;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.Button.ClickListener;
+import com.vaadin.ui.Label;
+
+@Widgetset(TestingWidgetSet.NAME)
+@Theme("tests-tickets")
+public class IdOverrideTest extends AbstractTestUI {
+
+    @Override
+    protected String getTestDescription() {
+        return "Id shouldn't get overridden unless specifically re-set.<br>"
+                + "First two are custom labels with a default id, third is an ordinary label for comparison.";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 10179;
+    }
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        getLayout().setSpacing(true);
+        getLayout().setMargin(new MarginInfo(true, false, false, false));
+
+        final IdTestLabel idTestLabel = new IdTestLabel("default id");
+        idTestLabel.setSizeUndefined();
+        addComponent(idTestLabel);
+
+        final IdTestLabel idTestLabelWithId = new IdTestLabel("set id");
+        idTestLabelWithId.setSizeUndefined();
+        idTestLabelWithId.setId("set10179");
+        idTestLabelWithId.setImmediate(true);
+        addComponent(idTestLabelWithId);
+
+        final Label label = new Label("no id");
+        label.setSizeUndefined();
+        addComponent(label);
+
+        Button button = new Button();
+        button.setCaption("Toggle");
+        button.addClickListener(new ClickListener() {
+
+            @Override
+            public void buttonClick(ClickEvent event) {
+                if (idTestLabelWithId.getId() == null) {
+                    idTestLabelWithId.setId("set10179");
+                    idTestLabelWithId.setValue("set id");
+                    idTestLabel.setValue("default id");
+                    label.setValue("no id");
+                } else {
+                    idTestLabelWithId.setId(null);
+                    idTestLabelWithId.setValue("removed id");
+                    idTestLabel.setValue("still default id");
+                    label.setValue("still no id");
+                }
+            }
+        });
+        button.setId("toggle");
+        button.setImmediate(true);
+        addComponent(button);
+    }
+
+}
diff --git a/uitest/src/com/vaadin/tests/widgetset/client/IdTestLabelConnector.java b/uitest/src/com/vaadin/tests/widgetset/client/IdTestLabelConnector.java
new file mode 100644
index 0000000000..5683ef03c6
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/widgetset/client/IdTestLabelConnector.java
@@ -0,0 +1,18 @@
+package com.vaadin.tests.widgetset.client;
+
+import com.vaadin.client.ui.label.LabelConnector;
+import com.vaadin.shared.ui.Connect;
+
+/**
+ * Connects server-side <code>IdTestLabel</code> component to client-side
+ * {@link VIdTestLabel} component (#10179).
+ * 
+ */
+@Connect(com.vaadin.tests.widgetset.server.IdTestLabel.class)
+public class IdTestLabelConnector extends LabelConnector {
+
+    @Override
+    public VIdTestLabel getWidget() {
+        return (VIdTestLabel) super.getWidget();
+    }
+}
diff --git a/uitest/src/com/vaadin/tests/widgetset/client/VIdTestLabel.java b/uitest/src/com/vaadin/tests/widgetset/client/VIdTestLabel.java
new file mode 100644
index 0000000000..52610e2a47
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/widgetset/client/VIdTestLabel.java
@@ -0,0 +1,15 @@
+package com.vaadin.tests.widgetset.client;
+
+import com.vaadin.client.ui.VLabel;
+
+/**
+ * Client-side implementation for IdTestLabel (#10179).
+ * 
+ */
+public class VIdTestLabel extends VLabel {
+
+    public VIdTestLabel() {
+        super();
+        getElement().setId("default10179");
+    }
+}
diff --git a/uitest/src/com/vaadin/tests/widgetset/server/IdTestLabel.java b/uitest/src/com/vaadin/tests/widgetset/server/IdTestLabel.java
new file mode 100644
index 0000000000..c505151341
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/widgetset/server/IdTestLabel.java
@@ -0,0 +1,16 @@
+package com.vaadin.tests.widgetset.server;
+
+import com.vaadin.ui.Label;
+
+/**
+ * Label that has a default id <code>default10179</code> for the use of
+ * IdOverrideTest (#10179).
+ * 
+ */
+public class IdTestLabel extends Label {
+
+    public IdTestLabel(String caption) {
+        super(caption);
+    }
+
+}
-- 
2.39.5