From 7a453dce177d849305c549296986eb21e171bae0 Mon Sep 17 00:00:00 2001
From: Tatu Lund <tatu@vaadin.com>
Date: Fri, 7 Jun 2019 13:14:38 +0300
Subject: Fix regression that broke PreserveOnRefresh functionality with
 Navigator (#11615)

* Fix regression that broke PreserveOnRefresh functionality with Navigator

Fixes https://github.com/vaadin/framework/issues/11614

Old patch https://github.com/vaadin/framework/issues/11416 calls navigateTo allways when Navigator is present, which is wrong, since it is needed only when navigation state has truly changed.

* Add test to the fix
---
 server/src/main/java/com/vaadin/ui/UI.java         | 15 +++++++++--
 .../com/vaadin/tests/components/ui/RefreshUI.java  | 29 ++++++++++++++++++++++
 .../vaadin/tests/components/ui/RefreshUITest.java  | 25 +++++++++++++++++++
 3 files changed, 67 insertions(+), 2 deletions(-)
 create mode 100644 uitest/src/main/java/com/vaadin/tests/components/ui/RefreshUI.java
 create mode 100644 uitest/src/test/java/com/vaadin/tests/components/ui/RefreshUITest.java

diff --git a/server/src/main/java/com/vaadin/ui/UI.java b/server/src/main/java/com/vaadin/ui/UI.java
index 682e1f38f2..2a3b913e16 100644
--- a/server/src/main/java/com/vaadin/ui/UI.java
+++ b/server/src/main/java/com/vaadin/ui/UI.java
@@ -161,6 +161,12 @@ public abstract class UI extends AbstractSingleComponentContainer
     private LoadingIndicatorConfiguration loadingIndicatorConfiguration = new LoadingIndicatorConfigurationImpl(
             this);
 
+    /**
+     * Holder for old navigation state, needed in doRefresh in order not to call
+     * navigateTo too often
+     */
+    private String oldNavigationState;
+
     /**
      * Scroll Y position.
      */
@@ -871,10 +877,15 @@ public abstract class UI extends AbstractSingleComponentContainer
         page.updateBrowserWindowSize(newWidth, newHeight, true);
 
         // Navigate if there is navigator, this is needed in case of
-        // PushStateNavigation
+        // PushStateNavigation. Call navigateTo only if state have
+        // truly changed
         Navigator navigator = getNavigator();
         if (navigator != null) {
-            navigator.navigateTo(navigator.getState());
+            if (oldNavigationState == null) oldNavigationState = getNavigator().getState();
+            if (!navigator.getState().equals(oldNavigationState)) {
+                navigator.navigateTo(navigator.getState());
+                oldNavigationState = navigator.getState();
+            }
         }
     }
 
diff --git a/uitest/src/main/java/com/vaadin/tests/components/ui/RefreshUI.java b/uitest/src/main/java/com/vaadin/tests/components/ui/RefreshUI.java
new file mode 100644
index 0000000000..135a6880f9
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/ui/RefreshUI.java
@@ -0,0 +1,29 @@
+package com.vaadin.tests.components.ui;
+
+import com.vaadin.annotations.PreserveOnRefresh;
+import com.vaadin.navigator.Navigator;
+import com.vaadin.navigator.View;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.VerticalLayout;
+
+@PreserveOnRefresh
+public class RefreshUI extends AbstractTestUI {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        final Navigator navigator = new Navigator(this, this);
+        navigator.addView("", MyView.class);
+        setNavigator(navigator);
+    }
+
+    public static class MyView extends VerticalLayout implements View {
+        private static int instanceNumber = 0;
+
+        public MyView() {
+            instanceNumber++;
+            addComponent(new Label("This is instance no " + instanceNumber));
+        }
+    }
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/ui/RefreshUITest.java b/uitest/src/test/java/com/vaadin/tests/components/ui/RefreshUITest.java
new file mode 100644
index 0000000000..16f279e1e5
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/ui/RefreshUITest.java
@@ -0,0 +1,25 @@
+package com.vaadin.tests.components.ui;
+
+import org.junit.Test;
+
+import com.vaadin.testbench.elements.LabelElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+import static org.junit.Assert.assertEquals;
+
+public class RefreshUITest extends MultiBrowserTest {
+
+    @Test
+    public void testUIRefresh_viewNotRecreated() {
+        openTestURL();
+        assertEquals("The Label content is not matching",
+                "This is instance no 1",
+                $(LabelElement.class).first().getText());
+
+        // Reload the page; UI.refresh should be invoked
+        openTestURL();
+        assertEquals("The Label content is not matching",
+                "This is instance no 1",
+                $(LabelElement.class).first().getText());
+    }
+}
-- 
cgit v1.2.3