From 85cfa2c5a69d777c9dd2f283fb7561e608523c14 Mon Sep 17 00:00:00 2001 From: kasemir Date: Fri, 22 Aug 2025 10:40:20 -0400 Subject: [PATCH 1/2] NavTabs: Better handling of invalid `active_tab` If the active tab is outside of the range of defined tabs, it would show disfunctional tab content without macros and log an ArrayIndexOutOfBounds exception. Now it will log something like "Widget 'XXXX' (navtabs) active tab 5 needs to be within 0 ... 4" and fall back to the highest valid tab index. --- .../model/widgets/NavigationTabsWidget.java | 13 +++++++++---- .../widgets/NavigationTabsRepresentation.java | 14 +++++++++----- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/NavigationTabsWidget.java b/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/NavigationTabsWidget.java index 50d589456e..de21dfdcf0 100644 --- a/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/NavigationTabsWidget.java +++ b/app/display/model/src/main/java/org/csstudio/display/builder/model/widgets/NavigationTabsWidget.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2017-2023 Oak Ridge National Laboratory. + * Copyright (c) 2017-2025 Oak Ridge National Laboratory. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -45,7 +45,6 @@ /** Widget with tabs to select amongst several embedded displays * @author Kay Kasemir */ -@SuppressWarnings("nls") public class NavigationTabsWidget extends VisibleWidget { /** Widget descriptor */ @@ -210,7 +209,7 @@ public Macros getEffectiveMacros() { final Macros base = super.getEffectiveMacros(); // Join macros of active tab - final int index; + int index; // To fetch the effective macros, we want to add those of the selected tab. // But if we get the tab index via active.getValue(), AND 'active' itself contains macros, @@ -228,7 +227,13 @@ public Macros getEffectiveMacros() return base; } - if (index >= 0 && index < tabs.size()) + if (index >= tabs.size()) + { + logger.log(Level.WARNING, this + " active tab " + index + " needs to be within 0 ... " + (tabs.size() - 1)); + index = tabs.size()-1; + } + + if (index >= 0) try { final Macros selected = tabs.getElement(index).macros().getValue(); diff --git a/app/display/representation-javafx/src/main/java/org/csstudio/display/builder/representation/javafx/widgets/NavigationTabsRepresentation.java b/app/display/representation-javafx/src/main/java/org/csstudio/display/builder/representation/javafx/widgets/NavigationTabsRepresentation.java index 19144a0f6d..20a880c099 100644 --- a/app/display/representation-javafx/src/main/java/org/csstudio/display/builder/representation/javafx/widgets/NavigationTabsRepresentation.java +++ b/app/display/representation-javafx/src/main/java/org/csstudio/display/builder/representation/javafx/widgets/NavigationTabsRepresentation.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2017-2018 Oak Ridge National Laboratory. + * Copyright (c) 2017-2025 Oak Ridge National Laboratory. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -51,13 +51,16 @@ * * @author Kay Kasemir */ -@SuppressWarnings("nls") public class NavigationTabsRepresentation extends RegionBaseRepresentation { private final DirtyFlag dirty_sizes = new DirtyFlag(); private final DirtyFlag dirty_tabs = new DirtyFlag(); private final DirtyFlag dirty_tab_look = new DirtyFlag(); private final DirtyFlag dirty_active_tab = new DirtyFlag(); + + // Track the "active tab" of navigation tabs _inside_ this one. + // As user toggles between tabs, this will allow restoring the active tab + // of nested instances, which would otherwise start over at their default tab private class SelectedNavigationTabs extends MutablePair, HashMap>> { public SelectedNavigationTabs(int activeTab) { left = activeTab; @@ -244,9 +247,10 @@ private synchronized void updatePendingDisplay(final JobMonitor monitor) }); checkCompletion(model_widget, completion, "timeout representing new content"); - int tabNumber = model_widget.propActiveTab().getValue(); - String tabName = model_widget.propTabs().getValue().get(model_widget.propActiveTab().getValue()).name().getValue(); - Pair tabNumberAndTabName = new Pair<>(tabNumber, tabName); + final List tabs = model_widget.propTabs().getValue(); + final int tabNumber = Math.min(model_widget.propActiveTab().getValue(), tabs.size()-1); + final String tabName = tabs.get(tabNumber).name().getValue(); + final Pair tabNumberAndTabName = new Pair<>(tabNumber, tabName); if (!selectedNavigationTabs.right.containsKey(tabNumberAndTabName)) { selectedNavigationTabs.right.put(tabNumberAndTabName, new HashMap<>()); From 3d269fc2a9e7870b1b0147363fb193b9941a8be1 Mon Sep 17 00:00:00 2001 From: kasemir Date: Fri, 22 Aug 2025 16:27:19 -0400 Subject: [PATCH 2/2] NavTabs SelectedNavigationTabs cleanup Custom record/class to get more meaningful names than 'left' and 'right'. More comments on what's going on --- .../widgets/NavigationTabsRepresentation.java | 78 ++++++++++--------- 1 file changed, 43 insertions(+), 35 deletions(-) diff --git a/app/display/representation-javafx/src/main/java/org/csstudio/display/builder/representation/javafx/widgets/NavigationTabsRepresentation.java b/app/display/representation-javafx/src/main/java/org/csstudio/display/builder/representation/javafx/widgets/NavigationTabsRepresentation.java index 20a880c099..6f42f09f06 100644 --- a/app/display/representation-javafx/src/main/java/org/csstudio/display/builder/representation/javafx/widgets/NavigationTabsRepresentation.java +++ b/app/display/representation-javafx/src/main/java/org/csstudio/display/builder/representation/javafx/widgets/NavigationTabsRepresentation.java @@ -18,8 +18,6 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Level; -import javafx.util.Pair; -import org.apache.commons.lang3.tuple.MutablePair; import org.csstudio.display.builder.model.DirtyFlag; import org.csstudio.display.builder.model.DisplayModel; import org.csstudio.display.builder.model.UntypedWidgetPropertyListener; @@ -61,10 +59,22 @@ public class NavigationTabsRepresentation extends RegionBaseRepresentation, HashMap>> { - public SelectedNavigationTabs(int activeTab) { - left = activeTab; - right = new HashMap<>(); // 'right' is a mapping of the form: Tab Number & Tab Name (Integer, String) -> Name of Navigator Tab Widget (String) -> Opened Tab and Sub-Tabs (SelectedNavigationTabs) + + // Tab index and name as unique identifier for a tab + private static record UniqueTabID(int index, String name) {} + + private static class SelectedNavigationTabs + { + // Index of active tab in this nav tabs widget + int active_tab; + + // Map from UniqueTabID -> ( Map from Name of child Navigator Tab Widget -> Opened Tab and Sub-Tabs ) + final HashMap> child_navtab_selections = new HashMap<>(); + + SelectedNavigationTabs(int activeTab) + { + active_tab = activeTab; } }; protected SelectedNavigationTabs selectedNavigationTabs = new SelectedNavigationTabs(0); @@ -190,7 +200,7 @@ private void activeTabChanged(final WidgetProperty property, final Inte dirty_active_tab.mark(); toolkit.scheduleUpdate(this); tab_display_listener.propertyChanged(null, null, null); - selectedNavigationTabs.left = tab_index; + selectedNavigationTabs.active_tab = tab_index; } /** Update to the next pending display @@ -250,35 +260,33 @@ private synchronized void updatePendingDisplay(final JobMonitor monitor) final List tabs = model_widget.propTabs().getValue(); final int tabNumber = Math.min(model_widget.propActiveTab().getValue(), tabs.size()-1); final String tabName = tabs.get(tabNumber).name().getValue(); - final Pair tabNumberAndTabName = new Pair<>(tabNumber, tabName); + final UniqueTabID tab_id = new UniqueTabID(tabNumber, tabName); - if (!selectedNavigationTabs.right.containsKey(tabNumberAndTabName)) { - selectedNavigationTabs.right.put(tabNumberAndTabName, new HashMap<>()); - } - HashMap selectedNavigationTabsHashMapForCurrentTab = selectedNavigationTabs.right.get(tabNumberAndTabName); - - new_model.getChildren() - .stream() - .filter(widget -> widget instanceof NavigationTabsWidget) - .forEach(widget -> { - NavigationTabsWidget nestedNavigationTabsWidget = (NavigationTabsWidget) widget; - NavigationTabsRepresentation nestedNavigationTabsRepresentation = (NavigationTabsRepresentation) nestedNavigationTabsWidget.getUserData(Widget.USER_DATA_REPRESENTATION); - if (nestedNavigationTabsRepresentation != null) { - SelectedNavigationTabs nestedNavigationTabsRepresentation_selectedNavigationTabs; - - if (!selectedNavigationTabsHashMapForCurrentTab.containsKey(nestedNavigationTabsWidget.getName())) { - nestedNavigationTabsRepresentation_selectedNavigationTabs = new SelectedNavigationTabs(nestedNavigationTabsWidget.propActiveTab().getValue()); - selectedNavigationTabsHashMapForCurrentTab.put(nestedNavigationTabsWidget.getName(), nestedNavigationTabsRepresentation_selectedNavigationTabs); - } - else { - nestedNavigationTabsRepresentation_selectedNavigationTabs = selectedNavigationTabsHashMapForCurrentTab.get(nestedNavigationTabsWidget.getName()); - if (nestedNavigationTabsWidget.propTabs().size() > nestedNavigationTabsRepresentation_selectedNavigationTabs.left) { - nestedNavigationTabsWidget.propActiveTab().setValue(nestedNavigationTabsRepresentation_selectedNavigationTabs.left); - } - } - nestedNavigationTabsRepresentation.selectedNavigationTabs = nestedNavigationTabsRepresentation_selectedNavigationTabs; - } - }); + // For this tab_id, create or update map of NavTabs children to their selected tab index and sub-navtabs + final HashMap selectedNavTabsMapForCurrentTab = selectedNavigationTabs.child_navtab_selections.computeIfAbsent(tab_id, tid -> new HashMap<>()); + + for (Widget w : new_model.getChildren()) + if (w instanceof NavigationTabsWidget childNavTab) + { + final NavigationTabsRepresentation childRepr = childNavTab.getUserData(Widget.USER_DATA_REPRESENTATION); + if (childRepr != null) + { + SelectedNavigationTabs childRepr_selectedNavigationTabs = selectedNavTabsMapForCurrentTab.get(childNavTab.getName()); + if (childRepr_selectedNavigationTabs == null) + { // See childNavTab for the first time? Create its SelectedNavigationTabs with its propActiveTab as start value + childRepr_selectedNavigationTabs = new SelectedNavigationTabs(childNavTab.propActiveTab().getValue()); + selectedNavTabsMapForCurrentTab.put(childNavTab.getName(), childRepr_selectedNavigationTabs); + } + // Known childNavTab? Re-select its last active tab (if valid index) + else if (childRepr_selectedNavigationTabs.active_tab < childNavTab.propTabs().size()) + childNavTab.propActiveTab().setValue(childRepr_selectedNavigationTabs.active_tab); + + // childRepr.selectedNavigationTabs was set when the childNavTabRepr got constructed, + // but it's empty and its reference will be lost when we switch tabs. + // Replace it with the one that's tracked by the parent navtab (this one) + childRepr.selectedNavigationTabs = childRepr_selectedNavigationTabs; + } + } model_widget.runtimePropEmbeddedModel().setValue(new_model); }