Tab index out of bounds after adding new Tab when previously existing tabs contain a MenuBar
Description
The calculation of the selected tab becomes faulty when a new Tab instance is added to Tabs where at least one of the previously existing Tab instances contain a MenuBar. We noticed this bug after upgrading from Vaadin 23.3.3 to 24.3.11.
2024-05-17 13:22:38,384 ERROR [MyApp.ui.infrastructure.UncaughtExceptionHandler] (default task-9) error: Caught: Exception-Msg: Add. Info: java.lang.IllegalArgumentException: The 'index' argument should not be greater than or equals to the number of children tabs. It was: 4 at deployment.MyApp.ear.MyAppUI.war//com.vaadin.flow.component.tabs.Tabs.lambda$getTabAt$10(Tabs.java:792) at java.base/java.util.Optional.orElseThrow(Optional.java:403) at deployment.MyApp.ear.MyAppUI.war//com.vaadin.flow.component.tabs.Tabs.getTabAt(Tabs.java:792) at deployment.MyApp.ear.MyAppUI.war//com.vaadin.flow.component.tabs.Tabs.getSelectedTab(Tabs.java:558) at deployment.MyApp.ear.MyAppUI.war//com.vaadin.flow.component.tabs.Tabs.updateSelectedTab(Tabs.java:667) at deployment.MyApp.ear.MyAppUI.war//com.vaadin.flow.component.tabs.Tabs.lambda$new$c53a20e4$1(Tabs.java:96) at deployment.MyApp.ear.MyAppUI.war//com.vaadin.flow.internal.nodefeature.ElementPropertyMap.lambda$fireEvent$2(ElementPropertyMap.java:465) at java.base/java.util.ArrayList.forEach(ArrayList.java:1596) at deployment.MyApp.ear.MyAppUI.war//com.vaadin.flow.internal.nodefeature.ElementPropertyMap.fireEvent(ElementPropertyMap.java:465) at deployment.MyApp.ear.MyAppUI.war//com.vaadin.flow.internal.nodefeature.ElementPropertyMap$PutResult.run(ElementPropertyMap.java:170) at deployment.MyApp.ear.MyAppUI.war//com.vaadin.flow.server.communication.ServerRpcHandler.lambda$handleInvocations$1(ServerRpcHandler.java:436) at deployment.MyApp.ear.MyAppUI.war//com.vaadin.flow.server.communication.ServerRpcHandler.runMapSyncTask(ServerRpcHandler.java:452) at deployment.MyApp.ear.MyAppUI.war//com.vaadin.flow.server.communication.ServerRpcHandler.lambda$handleInvocations$3(ServerRpcHandler.java:446) at java.base/java.util.ArrayList.forEach(ArrayList.java:1596) at deployment.MyApp.ear.MyAppUI.war//com.vaadin.flow.server.communication.ServerRpcHandler.handleInvocations(ServerRpcHandler.java:446) at deployment.MyApp.ear.MyAppUI.war//com.vaadin.flow.server.communication.ServerRpcHandler.handleRpc(ServerRpcHandler.java:324) at deployment.MyApp.ear.MyAppUI.war//com.vaadin.flow.server.communication.UidlRequestHandler.synchronizedHandleRequest(UidlRequestHandler.java:114) at deployment.MyApp.ear.MyAppUI.war//com.vaadin.flow.server.SynchronizedRequestHandler.handleRequest(SynchronizedRequestHandler.java:40) at deployment.MyApp.ear.MyAppUI.war//com.vaadin.flow.server.VaadinService.handleRequest(VaadinService.java:1574) at deployment.MyApp.ear.MyAppUI.war//com.vaadin.flow.server.VaadinServlet.service(VaadinServlet.java:398) at deployment.MyApp.ear.MyAppUI.war//com.vaadin.cdi.CdiVaadinServlet.service(CdiVaadinServlet.java:66) at [email protected]//jakarta.servlet.http.HttpServlet.service(HttpServlet.java:614) at [email protected]//io.undertow.servlet.handlers.ServletHandler.handleRequest(ServletHandler.java:74) at [email protected]//io.undertow.servlet.handlers.FilterHandler$FilterChainImpl.doFilter(FilterHandler.java:129) at [email protected]//io.undertow.websockets.jsr.JsrWebSocketFilter.doFilter(JsrWebSocketFilter.java:172) at [email protected]//io.undertow.servlet.core.ManagedFilter.doFilter(ManagedFilter.java:67) at [email protected]//io.undertow.servlet.handlers.FilterHandler$FilterChainImpl.doFilter(FilterHandler.java:131) at [email protected]//io.undertow.servlet.handlers.FilterHandler.handleRequest(FilterHandler.java:84) at [email protected]//io.undertow.servlet.handlers.security.ServletSecurityRoleHandler.handleRequest(ServletSecurityRoleHandler.java:62) at [email protected]//io.undertow.servlet.handlers.ServletChain$1.handleRequest(ServletChain.java:68) at [email protected]//io.undertow.servlet.handlers.ServletDispatchingHandler.handleRequest(ServletDispatchingHandler.java:36) at [email protected]//org.wildfly.elytron.web.undertow.server.ElytronRunAsHandler.lambda$handleRequest$1(ElytronRunAsHandler.java:68) at [email protected]//org.wildfly.security.auth.server.FlexibleIdentityAssociation.runAsFunctionEx(FlexibleIdentityAssociation.java:103) at [email protected]//org.wildfly.security.auth.server.Scoped.runAsFunctionEx(Scoped.java:161) at [email protected]//org.wildfly.security.auth.server.Scoped.runAs(Scoped.java:73) at [email protected]//org.wildfly.elytron.web.undertow.server.ElytronRunAsHandler.handleRequest(ElytronRunAsHandler.java:67) at [email protected]//io.undertow.servlet.handlers.RedirectDirHandler.handleRequest(RedirectDirHandler.java:68) at [email protected]//io.undertow.servlet.handlers.security.SSLInformationAssociationHandler.handleRequest(SSLInformationAssociationHandler.java:117) at [email protected]//io.undertow.servlet.handlers.security.ServletAuthenticationCallHandler.handleRequest(ServletAuthenticationCallHandler.java:57) at [email protected]//io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43) at [email protected]//io.undertow.security.handlers.AbstractConfidentialityHandler.handleRequest(AbstractConfidentialityHandler.java:46) at [email protected]//io.undertow.servlet.handlers.security.ServletConfidentialityConstraintHandler.handleRequest(ServletConfidentialityConstraintHandler.java:64) at [email protected]//io.undertow.security.handlers.AbstractSecurityContextAssociationHandler.handleRequest(AbstractSecurityContextAssociationHandler.java:43) at org.wildfly.security.elytron-web.undertow-server-servlet@4.1.0.Final//org.wildfly.elytron.web.undertow.server.servlet.CleanUpHandler.handleRequest(CleanUpHandler.java:38) at [email protected]//io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43) at [email protected]//org.wildfly.extension.undertow.security.jacc.JACCContextIdHandler.handleRequest(JACCContextIdHandler.java:44) at [email protected]//io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43) at [email protected]//org.wildfly.extension.undertow.deployment.GlobalRequestControllerHandler.handleRequest(GlobalRequestControllerHandler.java:51) at [email protected]//io.undertow.servlet.handlers.SendErrorPageHandler.handleRequest(SendErrorPageHandler.java:52) at [email protected]//io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43) at [email protected]//io.undertow.servlet.handlers.ServletInitialHandler.handleFirstRequest(ServletInitialHandler.java:276) at [email protected]//io.undertow.servlet.handlers.ServletInitialHandler$2.call(ServletInitialHandler.java:135) at [email protected]//io.undertow.servlet.handlers.ServletInitialHandler$2.call(ServletInitialHandler.java:132) at [email protected]//io.undertow.servlet.core.ServletRequestContextThreadSetupAction$1.call(ServletRequestContextThreadSetupAction.java:48) at [email protected]//io.undertow.servlet.core.ContextClassLoaderSetupAction$1.call(ContextClassLoaderSetupAction.java:43) at [email protected]//org.wildfly.extension.undertow.deployment.UndertowDeploymentInfoService$UndertowThreadSetupAction.lambda$create$0(UndertowDeploymentInfoService.java:1421) at [email protected]//org.wildfly.extension.undertow.deployment.UndertowDeploymentInfoService$UndertowThreadSetupAction.lambda$create$0(UndertowDeploymentInfoService.java:1421) at [email protected]//org.wildfly.extension.undertow.deployment.UndertowDeploymentInfoService$UndertowThreadSetupAction.lambda$create$0(UndertowDeploymentInfoService.java:1421) at [email protected]//org.wildfly.extension.undertow.deployment.UndertowDeploymentInfoService$UndertowThreadSetupAction.lambda$create$0(UndertowDeploymentInfoService.java:1421) at [email protected]//org.wildfly.extension.undertow.deployment.UndertowDeploymentInfoService$UndertowThreadSetupAction.lambda$create$0(UndertowDeploymentInfoService.java:1421) at [email protected]//io.undertow.servlet.handlers.ServletInitialHandler.dispatchRequest(ServletInitialHandler.java:256) at [email protected]//io.undertow.servlet.handlers.ServletInitialHandler$1.handleRequest(ServletInitialHandler.java:101) at [email protected]//io.undertow.server.Connectors.executeRootHandler(Connectors.java:393) at [email protected]//io.undertow.server.HttpServerExchange$1.run(HttpServerExchange.java:859) at [email protected]//org.jboss.threads.ContextClassLoaderSavingRunnable.run(ContextClassLoaderSavingRunnable.java:35) at [email protected]//org.jboss.threads.EnhancedQueueExecutor.safeRun(EnhancedQueueExecutor.java:1990) at [email protected]//org.jboss.threads.EnhancedQueueExecutor$ThreadBody.doRunTask(EnhancedQueueExecutor.java:1486) at [email protected]//org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1377) at [email protected]//org.xnio.XnioWorker$WorkerThreadFactory$1$1.run(XnioWorker.java:1282) at java.base/java.lang.Thread.run(Thread.java:1583)
Expected outcome
The clicked tab should be selected.
Minimal reproducible example
@Route("tabDemo")
public class TabDemo extends VerticalLayout {
@Serial
private static final long serialVersionUID = -7222430498899820179L;
private int tabCounter = 0;
private void addTabWithMenuBar(Tabs tabs) {
var menuBar = new MenuBar();
var icon = VaadinIcon.ELLIPSIS_CIRCLE_O.create();
var menuItem = menuBar.addItem(icon);
menuItem.getSubMenu().addItem("Dummy");
var tab = new Tab("Tab " + tabCounter++);
tab.add(menuBar);
tabs.add(tab);
}
private void addSimpleTab(Tabs tabs) {
tabs.add(new Tab("Tab " + tabCounter++));
}
public TabDemo() {
var tabs = new Tabs();
addTabWithMenuBar(tabs);
addTabWithMenuBar(tabs);
// content of this tab doesn't matter
var tabCreator = new Button("Add tab", e -> addSimpleTab(tabs));
this.add(tabs, tabCreator);
}
}
Steps to reproduce
- Navigate to the view from the example
- Initial state: Clicking Tab#0 or Tab#1 works
- Click on the "Add tab" button
- Clicking on Tab#0 still works, but clicking on Tab#1 selects Tab#2 instead and clicking on Tab#2 leads to a IllegalArgumentException
Environment
Vaadin version(s): 24.3.11 OS: Windows 10
Browsers
Chrome, Firefox, Edge
Placing MenuBar inside a Tab isn't a supported use case (generally, you shouldn't place interactive content in Tab).
Consider using TabSheet and placing MenuBar inside the tab content (and not the tab itself) - this should work.
I'm sad to hear that it's not supported (anymore). With Vaadin 14 and 23 there was no problem using a MenuBar inside a Tab.
Your suggestion to move the MenuBar to the tab's content isn't appropriate for our use case unfortunately. We use the Tabs component to display the names of different views within our application and allow the user to switch between those views with the Tabs component. We use the MenuBar to provide an easy way to close or maximize views without taking too much space in the Tab name:
It's a bit weird that it worked before. I guess the reason might be related to the fact that in Vaadin 24, menu-bar renders buttons in light DOM and not shadow DOM. The issue needs further investigation though.
Dear F-Kamp, would context-menu be sufficient for your use-case? The exception itself should probably be fixed.
The context menu is sufficient but not optimal for our use case. Since there is no alternative, we have decided to use a context menu and only show a small "x" button to close a tab.
Using a MenuBar instead of a context menu would be our preferred solution because users that aren't familiar with our application don't know that there's the possibility to open a context menu. So from the UX perspective, MenuBar is the better choice.
I investigated the behavior and observed that as soon as a MenuBar with an MenuItem is added to a Tab in the TabSheet, the element "vaadin-menu-bar-item" is added after the "vaadin-tab" element to tabSheet.__data.items and tabSheet.__tabs.__data.items. These arrays should contain only the "vaadin-tab" elements. If you remove the unnecessary "vaadin-menu-bar-item" elements from these arrays (using javascript) the TabSheet behaves as expected. So why these "vaadin-menu-bar-item" elements are added to TabSheet data?
Thanks for looking into it. I think the reason could be that vaadin-menu-bar-item also uses the ItemMixin which is checked in ListMixin. I think we could update logic to use children instead of getFlattenedElements(this).
UPD: this was changed in https://github.com/vaadin/web-components/pull/4337. We probably can revert that change as the initial TabSheet wasn't wrapping Tabs as mentioned here but it was changed later.