Call "destroy" instead of "destory" in JavaScriptRenders (#8263)

* Call "destroy" instead of "destory" in JavaScriptRenders

For backwards compatibility, calls "destory" if no "destroy" exists

Fixes #8162

* Merge branch '7.7' into _js-renderer-destory

* Merge branch '7.7' into _js-renderer-destory

* Merge branch '7.7' into _js-renderer-destory

* Merge branch '7.7' into _js-renderer-destory

* Merge branch '7.7' into _js-renderer-destory

7 files changed, 115 insertions, 8 deletions

diff --git a/client/src/main/java/com/vaadin/client/connectors/JavaScriptRendererConnector.java b/client/src/main/java/com/vaadin/client/connectors/JavaScriptRendererConnector.java
index 503884b566..88cc20b7a2 100644
--- a/client/src/main/java/com/vaadin/client/connectors/JavaScriptRendererConnector.java
+++ b/client/src/main/java/com/vaadin/client/connectors/JavaScriptRendererConnector.java
@@ -17,6 +17,7 @@ package com.vaadin.client.connectors;
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.logging.Logger;
 
 import com.google.gwt.core.client.JavaScriptObject;
 import com.google.gwt.core.client.JsArrayString;
@@ -141,9 +142,15 @@ public class JavaScriptRendererConnector
                     "JavaScriptRenderer " + helper.getInitFunctionName()
                             + " must have a function named 'render'");
         }
+        if (hasFunction("destory")) {
+            getLogger().severe("Your JavaScript connector ("
+                    + helper.getInitFunctionName()
+                    + ") has a typo. The destory method should be renamed to destroy.");
 
+        }
         final boolean hasInit = hasFunction("init");
-        final boolean hasDestroy = hasFunction("destroy");
+        final boolean hasDestroy = hasFunction("destroy")
+                || hasFunction("destory");
         final boolean hasOnActivate = hasFunction("onActivate");
         final boolean hasGetConsumedEvents = hasFunction("getConsumedEvents");
         final boolean hasOnBrowserEvent = hasFunction("onBrowserEvent");
@@ -190,16 +197,20 @@ public class JavaScriptRendererConnector
             @Override
             public void destroy(RendererCellReference cell) {
                 if (hasDestroy) {
-                    destory(helper.getConnectorWrapper(), getJsCell(cell));
+                    destroy(helper.getConnectorWrapper(), getJsCell(cell));
                 } else {
                     super.destroy(cell);
                 }
             }
 
-            private native void destory(JavaScriptObject wrapper,
+            private native void destroy(JavaScriptObject wrapper,
                     JavaScriptObject cell)
             /*-{
-                wrapper.destory(cell);
+                if (wrapper.destroy) {
+                    wrapper.destroy(cell);
+                } else  {
+                    wrapper.destory(cell);
+                }
             }-*/;
 
             @Override
@@ -265,6 +276,10 @@ public class JavaScriptRendererConnector
         };
     }
 
+    private Logger getLogger() {
+        return Logger.getLogger(JavaScriptRendererConnector.class.getName());
+    }
+
     @Override
     public Object decode(JsonValue value) {
         // Let the js logic decode the raw json that the server sent
diff --git a/server/src/main/java/com/vaadin/ui/renderers/AbstractJavaScriptRenderer.java b/server/src/main/java/com/vaadin/ui/renderers/AbstractJavaScriptRenderer.java
index e196d861c1..c13e303212 100644
--- a/server/src/main/java/com/vaadin/ui/renderers/AbstractJavaScriptRenderer.java
+++ b/server/src/main/java/com/vaadin/ui/renderers/AbstractJavaScriptRenderer.java
@@ -71,7 +71,7 @@ import elemental.json.JsonValue;
  * <li><code>init(cell)</code> - Prepares a cell for rendering. Corresponds to
  * {@link com.vaadin.client.renderers.ComplexRenderer#init(com.vaadin.client.widget.grid.RendererCellReference)}
  * .</li>
- * <li><code>destory(cell)</code> - Allows the renderer to release resources
+ * <li><code>destroy(cell)</code> - Allows the renderer to release resources
  * allocate for a cell that will no longer be used. Corresponds to
  * {@link com.vaadin.client.renderers.ComplexRenderer#destroy(com.vaadin.client.widget.grid.RendererCellReference)}
  * .</li>
diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/JavaScriptRenderers.java b/uitest/src/main/java/com/vaadin/tests/components/grid/JavaScriptRenderers.java
index bb7a93d378..d6a6373541 100644
--- a/uitest/src/main/java/com/vaadin/tests/components/grid/JavaScriptRenderers.java
+++ b/uitest/src/main/java/com/vaadin/tests/components/grid/JavaScriptRenderers.java
@@ -19,10 +19,16 @@ import com.vaadin.data.Item;
 import com.vaadin.data.util.IndexedContainer;
 import com.vaadin.server.VaadinRequest;
 import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.Button.ClickListener;
 import com.vaadin.ui.Grid;
+import com.vaadin.ui.Label;
 
 public class JavaScriptRenderers extends AbstractTestUI {
 
+    private Grid grid;
+
     public static class MyBean {
         private int integer;
         private String string;
@@ -52,10 +58,11 @@ public class JavaScriptRenderers extends AbstractTestUI {
 
     @Override
     protected void setup(VaadinRequest request) {
-        IndexedContainer container = new IndexedContainer();
+        final IndexedContainer container = new IndexedContainer();
         container.addContainerProperty("id", Integer.class, Integer.valueOf(0));
         container.addContainerProperty("bean", MyBean.class, null);
         container.addContainerProperty("string", String.class, "");
+        container.addContainerProperty("string2", String.class, "");
 
         for (int i = 0; i < 1000; i++) {
             Integer itemId = Integer.valueOf(i);
@@ -65,15 +72,34 @@ public class JavaScriptRenderers extends AbstractTestUI {
                     .setValue(new MyBean(i + 1, Integer.toString(i - 1)));
             item.getItemProperty("string").setValue("string" + i);
         }
+        Label clientLog = new Label();
+        clientLog.setId("clientLog");
+        addComponent(clientLog);
+        grid = createGrid(container);
+        addComponent(grid);
+
+        addComponent(new Button("Recreate grid", new ClickListener() {
+
+            @Override
+            public void buttonClick(ClickEvent event) {
+                Grid newGrid = createGrid(container);
+                replaceComponent(grid, newGrid);
+                grid = newGrid;
 
+            }
+        }));
+    }
+
+    private Grid createGrid(IndexedContainer container) {
         Grid grid = new Grid(container);
 
         grid.getColumn("bean").setRenderer(new MyBeanJSRenderer());
         grid.getColumn("bean").setWidth(250);
 
         grid.getColumn("string").setRenderer(new JavaScriptStringRenderer());
-
-        addComponent(grid);
+        grid.getColumn("string2")
+                .setRenderer(new JavaScriptStringRendererWithDestoryMethod());
+        return grid;
     }
 
 }
diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/JavaScriptStringRendererWithDestoryMethod.java b/uitest/src/main/java/com/vaadin/tests/components/grid/JavaScriptStringRendererWithDestoryMethod.java
new file mode 100644
index 0000000000..990a3617bd
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/grid/JavaScriptStringRendererWithDestoryMethod.java
@@ -0,0 +1,29 @@
+/*
+ * Copyright 2000-2014 Vaadin Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.tests.components.grid;
+
+import com.vaadin.annotations.JavaScript;
+import com.vaadin.ui.renderers.AbstractJavaScriptRenderer;
+
+@JavaScript("JavaScriptStringRendererWithDestoryMethod.js")
+public class JavaScriptStringRendererWithDestoryMethod
+        extends AbstractJavaScriptRenderer<String> {
+
+    protected JavaScriptStringRendererWithDestoryMethod() {
+        super(String.class);
+    }
+
+}
diff --git a/uitest/src/main/resources/com/vaadin/tests/components/grid/JavaScriptStringRenderer.js b/uitest/src/main/resources/com/vaadin/tests/components/grid/JavaScriptStringRenderer.js
index e3061bf12f..44467cd59f 100644
--- a/uitest/src/main/resources/com/vaadin/tests/components/grid/JavaScriptStringRenderer.js
+++ b/uitest/src/main/resources/com/vaadin/tests/components/grid/JavaScriptStringRenderer.js
@@ -4,4 +4,9 @@ com_vaadin_tests_components_grid_JavaScriptStringRenderer = function() {
 		// This one is for IE8
 		cell.element.innerText = data;
 	}
+
+	this.destroy = function(cell) {
+		document.getElementById("clientLog").innerHTML += "destroy: "+cell.rowIndex+"/"+cell.columnIndex+"<br>";
+	}
+
 }
\ No newline at end of file
diff --git a/uitest/src/main/resources/com/vaadin/tests/components/grid/JavaScriptStringRendererWithDestoryMethod.js b/uitest/src/main/resources/com/vaadin/tests/components/grid/JavaScriptStringRendererWithDestoryMethod.js
new file mode 100644
index 0000000000..8b6b8ede0b
--- /dev/null
+++ b/uitest/src/main/resources/com/vaadin/tests/components/grid/JavaScriptStringRendererWithDestoryMethod.js
@@ -0,0 +1,12 @@
+com_vaadin_tests_components_grid_JavaScriptStringRendererWithDestoryMethod = function() {
+	this.render = function(cell, data) {
+		cell.element.textContent = data;
+		// This one is for IE8
+		cell.element.innerText = data;
+	}
+
+	this.destory = function(cell) {
+		document.getElementById("clientLog").innerHTML += "destory: "+cell.rowIndex+"/"+cell.columnIndex+"<br>";
+	}
+
+}
\ No newline at end of file
diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/JavaScriptRenderersTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/JavaScriptRenderersTest.java
index 803c3cedf1..4cc5125721 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/grid/JavaScriptRenderersTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/grid/JavaScriptRenderersTest.java
@@ -17,7 +17,10 @@ package com.vaadin.tests.components.grid;
 
 import org.junit.Assert;
 import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.WebElement;
 
+import com.vaadin.testbench.elements.ButtonElement;
 import com.vaadin.testbench.elements.GridElement;
 import com.vaadin.testbench.elements.GridElement.GridCellElement;
 import com.vaadin.testbench.parallel.TestCategory;
@@ -49,4 +52,21 @@ public class JavaScriptRenderersTest extends MultiBrowserTest {
         Assert.assertTrue(
                 cell_1_1.getText().startsWith("Clicked 1 with key 2 at"));
     }
+
+    @Test
+    public void testJavaScriptRendererDestroy() {
+        openTestURL("debug");
+        waitForDebugMessage(
+                "Your JavaScript connector (com_vaadin_tests_components_grid_JavaScriptStringRendererWithDestoryMethod) has a typo. The destory method should be renamed to destroy.");
+
+        $(ButtonElement.class).first().click();
+
+        WebElement log = findElement(By.id("clientLog"));
+        String text = log.getText();
+        Assert.assertTrue(text.contains("destory: 19/3"));
+        Assert.assertTrue(text.contains("destroy: 19/2"));
+        Assert.assertTrue(text.contains("destroy: 0/2"));
+        Assert.assertTrue(text.contains("destory: 0/3"));
+    }
+
 }