diff options
author | Zhe Sun <31067185+ZheSun88@users.noreply.github.com> | 2020-07-01 12:44:55 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-07-01 12:44:55 +0300 |
commit | 89dfd79a7931835a6d9416f0e2f7371495667c2c (patch) | |
tree | b0344c8ca1f9f5266a79a3efff51b2c0bde411f1 | |
parent | d1b3a396d890f5b6124cbc90adff9a24b76b8b1a (diff) | |
download | vaadin-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>
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()); + } +} |