aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorZhe Sun <31067185+ZheSun88@users.noreply.github.com>2020-07-01 12:44:55 +0300
committerGitHub <noreply@github.com>2020-07-01 12:44:55 +0300
commit89dfd79a7931835a6d9416f0e2f7371495667c2c (patch)
treeb0344c8ca1f9f5266a79a3efff51b2c0bde411f1
parentd1b3a396d890f5b6124cbc90adff9a24b76b8b1a (diff)
downloadvaadin-framework-89dfd79a7931835a6d9416f0e2f7371495667c2c.tar.gz
vaadin-framework-89dfd79a7931835a6d9416f0e2f7371495667c2c.zip
Cherry pick 8.11.1 (#12046)
* Fix rendering of TreeGrid's frozen columns after hierarchy-column reset (#12028) * Add tests * Fix getVisibleFrozenColumnCount() if SelectionMode is multi * Update ComboBox popup position comparison to use correct top value. (#12041) Fixes #12029 * Clear thread local instances on connection lost in push handler (#12042) Adopted from https://github.com/vaadin/flow/pull/8567 Co-authored-by: Tarek Oraby <42799254+tarekoraby@users.noreply.github.com> Co-authored-by: Anna Koskinen <Ansku@users.noreply.github.com> Co-authored-by: Tatu Lund <tatu@vaadin.com>
-rw-r--r--client/src/main/java/com/vaadin/client/ui/VComboBox.java4
-rwxr-xr-xclient/src/main/java/com/vaadin/client/widgets/Grid.java10
-rw-r--r--server/src/main/java/com/vaadin/server/communication/PushHandler.java32
-rw-r--r--server/src/test/java/com/vaadin/server/MockVaadinServletService.java66
-rw-r--r--server/src/test/java/com/vaadin/server/MockVaadinSession.java4
-rw-r--r--server/src/test/java/com/vaadin/server/communication/PushHandlerTest79
-rw-r--r--server/src/test/java/com/vaadin/tests/server/ClassesSerializableTest.java1
-rw-r--r--uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxAtBottomEdgeWithinHorizontalLayout.java2
-rw-r--r--uitest/src/main/java/com/vaadin/tests/components/treegrid/TreeGridChangeHierarchyColumn.java75
-rw-r--r--uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxAtBottomEdgeWithinHorizontalLayoutTest.java35
-rw-r--r--uitest/src/test/java/com/vaadin/tests/components/treegrid/TreeGridChangeHierarchyColumnTest.java40
11 files changed, 337 insertions, 11 deletions
diff --git a/client/src/main/java/com/vaadin/client/ui/VComboBox.java b/client/src/main/java/com/vaadin/client/ui/VComboBox.java
index 6ff2f31a9a..b079930364 100644
--- a/client/src/main/java/com/vaadin/client/ui/VComboBox.java
+++ b/client/src/main/java/com/vaadin/client/ui/VComboBox.java
@@ -906,7 +906,7 @@ public class VComboBox extends Composite implements Field, KeyDownHandler,
// ComboBox itself may be incorrectly positioned, don't try to
// adjust horizontal popup position yet. Earlier width
// calculations must be performed anyway to avoid flickering.
- if (top != topPosition) {
+ if (top != getAbsoluteTop()) {
// Variable 'left' still contains the original popupLeft,
// 'top' has been updated, thus vertical position needs
// adjusting.
@@ -935,7 +935,7 @@ public class VComboBox extends Composite implements Field, KeyDownHandler,
}
// Only update the position if it has changed.
- if (top != topPosition || left != getPopupLeft()) {
+ if (top != getAbsoluteTop() || left != getPopupLeft()) {
setPopupPosition(left, top);
}
menu.scrollSelectionIntoView();
diff --git a/client/src/main/java/com/vaadin/client/widgets/Grid.java b/client/src/main/java/com/vaadin/client/widgets/Grid.java
index 2faddaf68d..9f36864bc8 100755
--- a/client/src/main/java/com/vaadin/client/widgets/Grid.java
+++ b/client/src/main/java/com/vaadin/client/widgets/Grid.java
@@ -7416,7 +7416,15 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
// for the escalator the hidden columns are not in the frozen column
// count, but for grid they are. thus need to convert the index
- for (int i = 0; i < frozenColumnCount; i++) {
+ int limit = getFrozenColumnCount();
+ if (getSelectionColumn().isPresent()) {
+ // If the grid is in MultiSelect mode, getColumn(0) in the following
+ // for loop returns the selection column. Accordingly, verifying
+ // which frozen columns are visible if the selection column is
+ // present should take this fact into account.
+ limit++;
+ }
+ for (int i = 0; i < limit; i++) {
if (i >= getColumnCount() || getColumn(i).isHidden()) {
numberOfColumns--;
}
diff --git a/server/src/main/java/com/vaadin/server/communication/PushHandler.java b/server/src/main/java/com/vaadin/server/communication/PushHandler.java
index 030d4323ef..77dd1b9b57 100644
--- a/server/src/main/java/com/vaadin/server/communication/PushHandler.java
+++ b/server/src/main/java/com/vaadin/server/communication/PushHandler.java
@@ -44,6 +44,7 @@ import com.vaadin.shared.ApplicationConstants;
import com.vaadin.shared.JsonConstants;
import com.vaadin.shared.communication.PushMode;
import com.vaadin.ui.UI;
+import com.vaadin.util.CurrentInstance;
import elemental.json.JsonException;
@@ -316,13 +317,29 @@ public class PushHandler {
}
void connectionLost(AtmosphereResourceEvent event) {
+ VaadinSession session = null;
+ try {
+ session = handleConnectionLost(event);
+ } finally {
+ if (session != null) {
+ session.access(new Runnable() {
+ @Override
+ public void run() {
+ CurrentInstance.clearAll();
+ }
+ });
+ }
+ }
+ }
+
+ private VaadinSession handleConnectionLost(AtmosphereResourceEvent event) {
// We don't want to use callWithUi here, as it assumes there's a client
// request active and does requestStart and requestEnd among other
// things.
if (event == null) {
getLogger().log(Level.SEVERE,
"Could not get event. This should never happen.");
- return;
+ return null;
}
AtmosphereResource resource = event.getResource();
@@ -330,7 +347,7 @@ public class PushHandler {
if (resource == null) {
getLogger().log(Level.SEVERE,
"Could not get resource. This should never happen.");
- return;
+ return null;
}
VaadinServletRequest vaadinRequest = new VaadinServletRequest(
@@ -342,7 +359,7 @@ public class PushHandler {
} catch (ServiceException e) {
getLogger().log(Level.SEVERE,
"Could not get session. This should never happen", e);
- return;
+ return null;
} catch (SessionExpiredException e) {
// This happens at least if the server is restarted without
// preserving the session. After restart the client reconnects, gets
@@ -351,7 +368,7 @@ public class PushHandler {
getLogger().log(Level.FINER,
"Session expired before push disconnect event was received",
e);
- return;
+ return session;
}
UI ui = null;
@@ -375,13 +392,13 @@ public class PushHandler {
getLogger().log(Level.FINE,
"Could not get UI. This should never happen,"
+ " except when reloading in Firefox and Chrome -"
- + " see http://dev.vaadin.com/ticket/14251.");
- return;
+ + " see https://github.com/vaadin/framework/issues/5449.");
+ return session;
} else {
getLogger().log(Level.INFO,
"No UI was found based on data in the request,"
+ " but a slower lookup based on the AtmosphereResource succeeded."
- + " See http://dev.vaadin.com/ticket/14251 for more details.");
+ + " See https://github.com/vaadin/framework/issues/5449 for more details.");
}
}
@@ -426,6 +443,7 @@ public class PushHandler {
// can't call ErrorHandler, we (hopefully) don't have a lock
}
}
+ return session;
}
private static UI findUiUsingResource(AtmosphereResource resource,
diff --git a/server/src/test/java/com/vaadin/server/MockVaadinServletService.java b/server/src/test/java/com/vaadin/server/MockVaadinServletService.java
new file mode 100644
index 0000000000..dee45e99e6
--- /dev/null
+++ b/server/src/test/java/com/vaadin/server/MockVaadinServletService.java
@@ -0,0 +1,66 @@
+/*
+ * Copyright 2000-2020 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.server;
+
+import java.util.Collections;
+import java.util.List;
+
+import javax.servlet.ServletException;
+
+import com.vaadin.tests.util.MockDeploymentConfiguration;
+
+/**
+ *
+ * @author Vaadin Ltd
+ */
+public class MockVaadinServletService extends VaadinServletService {
+
+ public MockVaadinServletService() throws ServiceException {
+ this(new MockDeploymentConfiguration());
+ }
+
+ public MockVaadinServletService(
+ DeploymentConfiguration deploymentConfiguration) throws ServiceException {
+ this(new VaadinServlet(), deploymentConfiguration);
+ }
+
+ public MockVaadinServletService(VaadinServlet servlet,
+ DeploymentConfiguration deploymentConfiguration) throws ServiceException {
+ super(servlet, deploymentConfiguration);
+
+ try {
+ servlet.init(new MockServletConfig());
+ } catch (ServletException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ @Override
+ protected List<RequestHandler> createRequestHandlers()
+ throws ServiceException {
+ return Collections.emptyList();
+ }
+
+ @Override
+ public void init() {
+ try {
+ super.init();
+ } catch (ServiceException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+}
diff --git a/server/src/test/java/com/vaadin/server/MockVaadinSession.java b/server/src/test/java/com/vaadin/server/MockVaadinSession.java
index 51a29adcbf..a95bf8b55f 100644
--- a/server/src/test/java/com/vaadin/server/MockVaadinSession.java
+++ b/server/src/test/java/com/vaadin/server/MockVaadinSession.java
@@ -18,6 +18,10 @@ public class MockVaadinSession extends VaadinSession {
super(service);
}
+ public MockVaadinSession() throws ServiceException {
+ super(new MockVaadinServletService());
+ }
+
@Override
public void close() {
super.close();
diff --git a/server/src/test/java/com/vaadin/server/communication/PushHandlerTest b/server/src/test/java/com/vaadin/server/communication/PushHandlerTest
new file mode 100644
index 0000000000..7f2c0ffb98
--- /dev/null
+++ b/server/src/test/java/com/vaadin/server/communication/PushHandlerTest
@@ -0,0 +1,79 @@
+/*
+ * Copyright 2000-2020 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.server.communication;
+
+import java.util.concurrent.Future;
+
+import org.atmosphere.cpr.AtmosphereRequest;
+import org.atmosphere.cpr.AtmosphereResource;
+import org.atmosphere.cpr.AtmosphereResourceEvent;
+import org.junit.Assert;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import com.vaadin.server.MockVaadinServletService;
+import com.vaadin.server.MockVaadinSession;
+import com.vaadin.server.ServiceException;
+import com.vaadin.server.SessionExpiredException;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.server.VaadinSession;
+import com.vaadin.ui.UI;
+
+public class PushHandlerTest {
+
+ MockVaadinSession session = null;
+
+ @Test
+ public void connectionLost_currentInstancesAreCleared()
+ throws SessionExpiredException, ServiceException {
+ session = new MockVaadinSession() {
+ @Override
+ public Future<Void> access(Runnable runnable) {
+ runnable.run();
+ return Mockito.mock(Future.class);
+ }
+ };
+ VaadinSession.setCurrent(session);
+ Assert.assertNotNull(VaadinSession.getCurrent());
+ MockVaadinServletService service = null;
+ service = new MockVaadinServletService() {
+ @Override
+ public com.vaadin.server.VaadinSession findVaadinSession(
+ VaadinRequest request) throws SessionExpiredException {
+ return session;
+ }
+
+ @Override
+ public UI findUI(VaadinRequest request) {
+ return null;
+ }
+ };
+
+ service.init();
+ PushHandler handler = new PushHandler(service);
+
+ AtmosphereResource resource = Mockito.mock(AtmosphereResource.class);
+ AtmosphereRequest request = Mockito.mock(AtmosphereRequest.class);
+ Mockito.when(resource.getRequest()).thenReturn(request);
+
+ AtmosphereResourceEvent event = Mockito
+ .mock(AtmosphereResourceEvent.class);
+ Mockito.when(event.getResource()).thenReturn(resource);
+ handler.connectionLost(event);
+
+ Assert.assertNull(VaadinSession.getCurrent());
+ }
+}
diff --git a/server/src/test/java/com/vaadin/tests/server/ClassesSerializableTest.java b/server/src/test/java/com/vaadin/tests/server/ClassesSerializableTest.java
index 9d921e1a52..6626aa6046 100644
--- a/server/src/test/java/com/vaadin/tests/server/ClassesSerializableTest.java
+++ b/server/src/test/java/com/vaadin/tests/server/ClassesSerializableTest.java
@@ -61,6 +61,7 @@ public class ClassesSerializableTest {
"com\\.vaadin\\.server\\.VaadinPortlet", //
"com\\.vaadin\\.server\\.MockServletConfig", //
"com\\.vaadin\\.server\\.MockServletContext", //
+ "com\\.vaadin\\.server\\.MockVaadinServletService", //
"com\\.vaadin\\.server\\.Constants", //
"com\\.vaadin\\.server\\.VaadinServiceClassLoaderUtil", //
"com\\.vaadin\\.server\\.VaadinServiceClassLoaderUtil\\$GetClassLoaderPrivilegedAction", //
diff --git a/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxAtBottomEdgeWithinHorizontalLayout.java b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxAtBottomEdgeWithinHorizontalLayout.java
index 5c619bafc4..6a15de3876 100644
--- a/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxAtBottomEdgeWithinHorizontalLayout.java
+++ b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxAtBottomEdgeWithinHorizontalLayout.java
@@ -13,7 +13,7 @@ public class ComboBoxAtBottomEdgeWithinHorizontalLayout extends AbstractTestUI {
@Override
protected void setup(VaadinRequest request) {
ComboBox<Integer> comboBox = new ComboBox<>();
- comboBox.setItems(Arrays.asList(100, 200, 300, 400, 500));
+ comboBox.setItems(Arrays.asList(102, 205, 302, 402, 500));
HorizontalLayout horizontalLayout = new HorizontalLayout();
horizontalLayout.addComponent(comboBox);
diff --git a/uitest/src/main/java/com/vaadin/tests/components/treegrid/TreeGridChangeHierarchyColumn.java b/uitest/src/main/java/com/vaadin/tests/components/treegrid/TreeGridChangeHierarchyColumn.java
new file mode 100644
index 0000000000..0e6e650e78
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/treegrid/TreeGridChangeHierarchyColumn.java
@@ -0,0 +1,75 @@
+package com.vaadin.tests.components.treegrid;
+
+import com.vaadin.data.TreeData;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Component;
+import com.vaadin.ui.CssLayout;
+import com.vaadin.ui.Grid;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.TreeGrid;
+
+public class TreeGridChangeHierarchyColumn extends AbstractTestUI {
+
+ @Override
+ protected String getTestDescription() {
+ return "TreeGrid in MultiSelect mode should take hiden columns into account when"
+ + " rendering frozen columns after hierarchy-column reset.";
+ }
+
+ @Override
+ protected Integer getTicketNumber() {
+ return 12026;
+ }
+
+ @Override
+ protected void setup(VaadinRequest request) {
+ TreeGrid<String> treeGrid = new TreeGrid<>();
+ treeGrid.setId("TreeGrid");
+
+ for (int i = 0; i < 20; i++) {
+ String columnId = String.valueOf(i);
+ Grid.Column<String, Component> column = addColumn(treeGrid,
+ columnId);
+ column.setCaption(columnId);
+ column.setId(columnId);
+ }
+
+ TreeData<String> data = treeGrid.getTreeData();
+ data.addItem(null, "child");
+ data.addItem("child", "grandChild");
+
+ treeGrid.setHierarchyColumn(treeGrid.getColumns().get(0));
+
+ Button hideHierCol = new Button("Hide Hierarchy Column");
+ hideHierCol.addClickListener(e -> {
+ treeGrid.getHierarchyColumn().setHidden(true);
+ });
+ hideHierCol.setId("hideHierColButton");
+
+ Button setHierCol = new Button("Set new Hierarchy Column");
+ setHierCol.addClickListener(e -> {
+ treeGrid.getColumns().stream().filter(column -> !column.isHidden())
+ .findFirst().ifPresent(col -> {
+ treeGrid.setHierarchyColumn(col.getId());
+ });
+ });
+ setHierCol.setId("setHierColButton");
+
+ treeGrid.setSelectionMode(Grid.SelectionMode.MULTI);
+ treeGrid.setFrozenColumnCount(1);
+
+ addComponents(treeGrid, hideHierCol, setHierCol);
+ }
+
+ private Grid.Column<String, Component> addColumn(Grid<String> grid,
+ String columnId) {
+ return grid.addComponentColumn(val -> {
+ Label label = new Label(columnId);
+ label.setWidth(50, Unit.PIXELS);
+ return new CssLayout(label);
+ });
+ }
+
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxAtBottomEdgeWithinHorizontalLayoutTest.java b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxAtBottomEdgeWithinHorizontalLayoutTest.java
index f169e56ab5..f3da49fe75 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxAtBottomEdgeWithinHorizontalLayoutTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxAtBottomEdgeWithinHorizontalLayoutTest.java
@@ -3,6 +3,7 @@ package com.vaadin.tests.components.combobox;
import org.junit.Test;
import org.openqa.selenium.WebElement;
+import com.vaadin.testbench.By;
import com.vaadin.testbench.elements.ComboBoxElement;
import com.vaadin.tests.tb3.MultiBrowserTest;
@@ -26,4 +27,38 @@ public class ComboBoxAtBottomEdgeWithinHorizontalLayoutTest
cbBottom, popupBottom), cbBottom, popupBottom);
}
+ @Test
+ public void ensurePopupPositionUpdatesWhenFiltered() {
+ openTestURL();
+
+ ComboBoxElement cb = $(ComboBoxElement.class).first();
+ cb.openPopup();
+ WebElement popup = cb.getSuggestionPopup();
+
+ int initialTop = popup.getLocation().getY();
+
+ // filter a bit
+ cb.findElement(By.vaadin("#textbox")).sendKeys("2");
+ int updatedTop = popup.getLocation().getY();
+ assertLessThan(String.format(
+ "Popup should be repositioned when "
+ + "filtered. Initial: %s, Updated: %s",
+ initialTop, updatedTop), initialTop, updatedTop);
+ int cbBottom = cb.getLocation().getY() + cb.getSize().getHeight();
+ assertGreaterOrEqual(String.format(
+ "Popup should still open above the ComboBox when "
+ + "filtered a bit. ComboBox: %s, Popup: %s",
+ cbBottom, updatedTop), cbBottom, updatedTop);
+
+ // filter more
+ cb.clear();
+ cb.findElement(By.vaadin("#textbox")).sendKeys("1");
+ popup = cb.getSuggestionPopup();
+ updatedTop = popup.getLocation().getY();
+ assertLessThanOrEqual(String.format(
+ "Popup should open below the ComboBox when "
+ + "filtered down to one result. ComboBox: %s, Popup: %s",
+ cbBottom, updatedTop), cbBottom, updatedTop);
+ }
+
}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/treegrid/TreeGridChangeHierarchyColumnTest.java b/uitest/src/test/java/com/vaadin/tests/components/treegrid/TreeGridChangeHierarchyColumnTest.java
new file mode 100644
index 0000000000..77efba9d37
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/treegrid/TreeGridChangeHierarchyColumnTest.java
@@ -0,0 +1,40 @@
+package com.vaadin.tests.components.treegrid;
+
+import static org.junit.Assert.assertEquals;
+import java.util.List;
+
+import org.junit.Test;
+import org.openqa.selenium.WebElement;
+import com.vaadin.testbench.By;
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.TreeGridElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+public class TreeGridChangeHierarchyColumnTest extends MultiBrowserTest {
+
+ @Test
+ public void renderingFrozenColumnsShouldFactorInHiddenColumns() {
+ openTestURL();
+ waitForElementPresent(By.id("TreeGrid"));
+ waitForElementPresent(By.id("hideHierColButton"));
+ waitForElementPresent(By.id("setHierColButton"));
+
+ TreeGridElement treeGrid = $(TreeGridElement.class).id("TreeGrid");
+ ButtonElement hideHierCol = $(ButtonElement.class)
+ .id("hideHierColButton");
+ ButtonElement setHierCol = $(ButtonElement.class)
+ .id("setHierColButton");
+
+ hideHierCol.click();
+ setHierCol.click();
+
+ // Wait for the new hierarchy column to be rendered
+ waitForElementPresent(By.className("v-treegrid-expander"));
+
+ List<WebElement> frozenCells = treeGrid
+ .findElements(By.className("frozen"));
+
+ assertEquals("Only the MultiSelect column should have frozen cells.", 2,
+ frozenCells.size());
+ }
+}