From 1ac5c788a714ec09230dd1305fbbfc27fc284d2c Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Wed, 26 Aug 2026 20:32:30 +0200 Subject: [PATCH] Let windows follow the toolbar visibility preference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WorkbenchWindow copied coolBarVisible and perspectiveBarVisible into the window's persisted state at creation, so the preference was read once and never again, and themes had no way to hide the toolbar. The persisted state now holds a per-window choice only when the user toggles a window away from the preference, and that choice survives later preference changes. Windows without one follow the preference at runtime, so themes can set both keys through the IEclipsePreferences CSS element. Per-window toggling from bug 403461 keeps working. Values persisted by earlier versions are migrated: a hidden bar stays a per-window choice, a visible one was a copy of the preference and is dropped. Assisted-by: multiple AI agents and layers of automated tooling 🤖 --- .../eclipse/ui/internal/WorkbenchWindow.java | 127 ++++++++++--- .../ui/tests/internal/InternalTestSuite.java | 1 + .../TrimVisibilityPreferenceTest.java | 176 ++++++++++++++++++ 3 files changed, 274 insertions(+), 30 deletions(-) create mode 100644 tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/internal/TrimVisibilityPreferenceTest.java diff --git a/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/WorkbenchWindow.java b/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/WorkbenchWindow.java index 6b8ee339568..c490b24adce 100644 --- a/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/WorkbenchWindow.java +++ b/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/WorkbenchWindow.java @@ -275,6 +275,8 @@ public class WorkbenchWindow implements IWorkbenchWindow { private boolean shellActivated = false; + private boolean destroyed; + ProgressRegion progressRegion = null; private final List workbenchTrimElements = new ArrayList<>(); @@ -467,26 +469,9 @@ public void setup() { final IEclipseContext windowContext = model.getContext(); HandlerServiceImpl.push(windowContext.getParent(), null); - // Initialize a previous 'saved' state if applicable. We no longer - // update the preference store. - if (getModel().getPersistedState().containsKey(IPreferenceConstants.COOLBAR_VISIBLE)) { - this.coolBarVisible = Boolean - .parseBoolean(getModel().getPersistedState().get(IPreferenceConstants.COOLBAR_VISIBLE)); - } else { - this.coolBarVisible = PrefUtil.getInternalPreferenceStore() - .getBoolean(IPreferenceConstants.COOLBAR_VISIBLE); - getModel().getPersistedState().put(IPreferenceConstants.COOLBAR_VISIBLE, - Boolean.toString(this.coolBarVisible)); - } - if (getModel().getPersistedState().containsKey(IPreferenceConstants.PERSPECTIVEBAR_VISIBLE)) { - this.perspectiveBarVisible = Boolean - .parseBoolean(getModel().getPersistedState().get(IPreferenceConstants.PERSPECTIVEBAR_VISIBLE)); - } else { - this.perspectiveBarVisible = PrefUtil.getInternalPreferenceStore() - .getBoolean(IPreferenceConstants.PERSPECTIVEBAR_VISIBLE); - getModel().getPersistedState().put(IPreferenceConstants.PERSPECTIVEBAR_VISIBLE, - Boolean.toString(this.perspectiveBarVisible)); - } + this.coolBarVisible = resolveTrimVisibility(IPreferenceConstants.COOLBAR_VISIBLE); + this.perspectiveBarVisible = resolveTrimVisibility(IPreferenceConstants.PERSPECTIVEBAR_VISIBLE); + PrefUtil.getInternalPreferenceStore().addPropertyChangeListener(trimVisibilityListener); IServiceLocatorCreator slc = workbench.getService(IServiceLocatorCreator.class); this.serviceLocator = (ServiceLocator) slc.createServiceLocator(workbench, null, () -> { @@ -972,6 +957,8 @@ private void addZoomChangeListenerToPromptForRestart() { @PreDestroy void preDestroy() { + destroyed = true; + PrefUtil.getInternalPreferenceStore().removePropertyChangeListener(trimVisibilityListener); if (mainMenu != null) { renderer.clearModelToManager(mainMenu, menuManager); mainMenu = null; @@ -1516,6 +1503,10 @@ protected int perspectiveBarStyle() { private boolean statusLineVisible = true; + private static final String TRIM_OVERRIDE_SUFFIX = ".override"; //$NON-NLS-1$ + + private final IPropertyChangeListener trimVisibilityListener = this::preferredTrimVisibilityChanged; + /** * The handlers for global actions that were last submitted to the workbench * command support. This is a map of command identifiers to @@ -2783,14 +2774,21 @@ public void fillActionBars(IActionBarConfigurer2 proxyBars, int flags) { * @since 3.0 */ public void setCoolBarVisible(boolean visible) { + if (applyCoolBarVisible(visible)) { + recordTrimOverride(IPreferenceConstants.COOLBAR_VISIBLE, visible); + } + } + + private boolean applyCoolBarVisible(boolean visible) { boolean oldValue = coolBarVisible; coolBarVisible = visible; - if (oldValue != coolBarVisible) { - getModel().getPersistedState().put(IPreferenceConstants.COOLBAR_VISIBLE, Boolean.toString(visible)); - updateLayoutDataForContents(); - firePropertyChanged(PROP_COOLBAR_VISIBLE, oldValue ? Boolean.TRUE : Boolean.FALSE, - coolBarVisible ? Boolean.TRUE : Boolean.FALSE); + if (oldValue == coolBarVisible) { + return false; } + updateLayoutDataForContents(); + firePropertyChanged(PROP_COOLBAR_VISIBLE, oldValue ? Boolean.TRUE : Boolean.FALSE, + coolBarVisible ? Boolean.TRUE : Boolean.FALSE); + return true; } /** @@ -2821,14 +2819,21 @@ public ActionPresentation getActionPresentation() { * @since 3.0 */ public void setPerspectiveBarVisible(boolean visible) { + if (applyPerspectiveBarVisible(visible)) { + recordTrimOverride(IPreferenceConstants.PERSPECTIVEBAR_VISIBLE, visible); + } + } + + private boolean applyPerspectiveBarVisible(boolean visible) { boolean oldValue = perspectiveBarVisible; perspectiveBarVisible = visible; - if (oldValue != perspectiveBarVisible) { - getModel().getPersistedState().put(IPreferenceConstants.PERSPECTIVEBAR_VISIBLE, Boolean.toString(visible)); - updateLayoutDataForContents(); - firePropertyChanged(PROP_PERSPECTIVEBAR_VISIBLE, oldValue ? Boolean.TRUE : Boolean.FALSE, - perspectiveBarVisible ? Boolean.TRUE : Boolean.FALSE); + if (oldValue == perspectiveBarVisible) { + return false; } + updateLayoutDataForContents(); + firePropertyChanged(PROP_PERSPECTIVEBAR_VISIBLE, oldValue ? Boolean.TRUE : Boolean.FALSE, + perspectiveBarVisible ? Boolean.TRUE : Boolean.FALSE); + return true; } /** @@ -2959,6 +2964,10 @@ public void toggleToolbarVisibility() { if (getWindowConfigurer().getShowPerspectiveBar()) { setPerspectiveBarVisible(!perspectivebarVisible); } + refreshToggleToolbarElements(); + } + + private void refreshToggleToolbarElements() { ICommandService commandService = getService(ICommandService.class); Map filter = new HashMap<>(); filter.put(IServiceScopes.WINDOW_SCOPE, this); @@ -2977,6 +2986,64 @@ public boolean isToolbarVisible() { || (getPerspectiveBarVisible() && getWindowConfigurer().getShowPerspectiveBar()); } + /** + * Returns the per-window override of a trim element, or the workspace + * preference, after migrating a value persisted by older versions. + */ + private boolean resolveTrimVisibility(String key) { + Map state = getModel().getPersistedState(); + String overrideKey = key + TRIM_OVERRIDE_SUFFIX; + // older versions copied the preference here, so only a hidden bar was a user choice + String legacy = state.remove(key); + if (legacy != null && !Boolean.parseBoolean(legacy)) { + state.putIfAbsent(overrideKey, legacy); + } + String override = state.get(overrideKey); + if (override == null) { + return PrefUtil.getInternalPreferenceStore().getBoolean(key); + } + return Boolean.parseBoolean(override); + } + + /** + * Stores a per-window choice, or removes it when it equals the preference. + */ + private void recordTrimOverride(String key, boolean visible) { + String overrideKey = key + TRIM_OVERRIDE_SUFFIX; + if (visible == PrefUtil.getInternalPreferenceStore().getBoolean(key)) { + getModel().getPersistedState().remove(overrideKey); + } else { + getModel().getPersistedState().put(overrideKey, Boolean.toString(visible)); + } + } + + /** + * Follows preference changes, for example from a theme. + */ + private void preferredTrimVisibilityChanged(PropertyChangeEvent event) { + String key = event.getProperty(); + if (!IPreferenceConstants.COOLBAR_VISIBLE.equals(key) + && !IPreferenceConstants.PERSPECTIVEBAR_VISIBLE.equals(key)) { + return; + } + Display display = workbench.getDisplay(); + if (display == null || display.isDisposed()) { + return; + } + display.asyncExec(() -> { + Shell shell = getShell(); + if (destroyed || closing || (shell != null && shell.isDisposed())) { + return; + } + boolean visible = resolveTrimVisibility(key); + boolean changed = IPreferenceConstants.COOLBAR_VISIBLE.equals(key) ? applyCoolBarVisible(visible) + : applyPerspectiveBarVisible(visible); + if (changed) { + refreshToggleToolbarElements(); + } + }); + } + private void updateLayoutDataForContents() { MTrimBar topTrim = getTopTrim(); if (topTrim != null) { diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/internal/InternalTestSuite.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/internal/InternalTestSuite.java index ff24811f308..1f10d248fa8 100644 --- a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/internal/InternalTestSuite.java +++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/internal/InternalTestSuite.java @@ -55,6 +55,7 @@ MarkerQueryTest.class, Bug99858Test.class, WorkbenchWindowSubordinateSourcesTests.class, + TrimVisibilityPreferenceTest.class, ReopenMenuTest.class, UtilTest.class, MarkerTesterTest.class, diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/internal/TrimVisibilityPreferenceTest.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/internal/TrimVisibilityPreferenceTest.java new file mode 100644 index 00000000000..bedfde18b3a --- /dev/null +++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/internal/TrimVisibilityPreferenceTest.java @@ -0,0 +1,176 @@ +/******************************************************************************* + * Copyright (c) 2026 Vogella GmbH and others. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Lars Vogel - initial API and implementation + *******************************************************************************/ +package org.eclipse.ui.tests.internal; + +import static org.eclipse.ui.tests.harness.util.UITestUtil.openTestWindow; +import static org.eclipse.ui.tests.harness.util.UITestUtil.processEvents; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import org.eclipse.jface.preference.IPreferenceStore; +import org.eclipse.ui.internal.IPreferenceConstants; +import org.eclipse.ui.internal.WorkbenchWindow; +import org.eclipse.ui.internal.util.PrefUtil; +import org.eclipse.ui.tests.harness.util.CloseTestWindowsExtension; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +/** + * Tests that the toolbar and perspective bar follow their workspace preferences + * unless a window carries an override. + */ +@ExtendWith(CloseTestWindowsExtension.class) +public class TrimVisibilityPreferenceTest { + + private static final String KEY = IPreferenceConstants.COOLBAR_VISIBLE; + + private static final String OVERRIDE_KEY = KEY + ".override"; + + private static final String PERSPECTIVE_BAR_KEY = IPreferenceConstants.PERSPECTIVEBAR_VISIBLE; + + private static final String PERSPECTIVE_BAR_OVERRIDE_KEY = PERSPECTIVE_BAR_KEY + ".override"; + + private WorkbenchWindow window; + + private IPreferenceStore preferences; + + @BeforeEach + public void setUp() throws Exception { + preferences = PrefUtil.getInternalPreferenceStore(); + preferences.setToDefault(KEY); + preferences.setToDefault(PERSPECTIVE_BAR_KEY); + window = (WorkbenchWindow) openTestWindow(); + processEvents(); + } + + @AfterEach + public void tearDown() { + preferences.setToDefault(KEY); + preferences.setToDefault(PERSPECTIVE_BAR_KEY); + processEvents(); + } + + @Test + public void testWindowWithoutOverrideFollowsPreference() { + assertFalse(persistedState().containsKey(OVERRIDE_KEY), "a fresh window must not pin the value"); + assertTrue(window.getCoolBarVisible()); + List events = new ArrayList<>(); + window.addPropertyChangeListener(event -> events.add(event.getProperty())); + + preferences.setValue(KEY, false); + processEvents(); + + assertFalse(window.getCoolBarVisible()); + assertEquals(List.of(WorkbenchWindow.PROP_COOLBAR_VISIBLE), events); + assertFalse(persistedState().containsKey(OVERRIDE_KEY), "following the preference must not create an override"); + } + + @Test + public void testTogglingAwayFromPreferenceStoresOverride() { + window.setCoolBarVisible(false); + + assertEquals(Boolean.FALSE.toString(), persistedState().get(OVERRIDE_KEY)); + assertFalse(window.getCoolBarVisible()); + } + + @Test + public void testTogglingBackToPreferenceDropsOverride() { + window.setCoolBarVisible(false); + assertEquals(Boolean.FALSE.toString(), persistedState().get(OVERRIDE_KEY)); + + window.setCoolBarVisible(true); + + assertFalse(persistedState().containsKey(OVERRIDE_KEY), "an override equal to the preference must be dropped"); + } + + @Test + public void testOverrideSurvivesPreferenceRoundTrip() { + window.setCoolBarVisible(false); + + preferences.setValue(KEY, false); + processEvents(); + preferences.setValue(KEY, true); + processEvents(); + + assertFalse(window.getCoolBarVisible(), "a per-window choice must outlive preference changes"); + assertEquals(Boolean.FALSE.toString(), persistedState().get(OVERRIDE_KEY)); + } + + @Test + public void testOverrideAppliesToItsWindowOnly() { + WorkbenchWindow other = (WorkbenchWindow) openTestWindow(); + processEvents(); + other.setCoolBarVisible(false); + + preferences.setValue(KEY, false); + processEvents(); + assertFalse(window.getCoolBarVisible()); + preferences.setValue(KEY, true); + processEvents(); + + assertTrue(window.getCoolBarVisible()); + assertFalse(other.getCoolBarVisible(), "the override of one window must not affect another"); + assertFalse(persistedState().containsKey(OVERRIDE_KEY)); + } + + @Test + public void testPerspectiveBarFollowsPreference() { + assertTrue(window.getPerspectiveBarVisible()); + + preferences.setValue(PERSPECTIVE_BAR_KEY, false); + processEvents(); + + assertFalse(window.getPerspectiveBarVisible()); + assertFalse(persistedState().containsKey(PERSPECTIVE_BAR_OVERRIDE_KEY)); + + window.setPerspectiveBarVisible(true); + + assertEquals(Boolean.TRUE.toString(), persistedState().get(PERSPECTIVE_BAR_OVERRIDE_KEY)); + } + + @Test + public void testLegacyVisibleSnapshotDoesNotPinWindow() { + persistedState().put(KEY, Boolean.TRUE.toString()); + + preferences.setValue(KEY, false); + processEvents(); + + assertFalse(window.getCoolBarVisible(), "a copied preference must not act as an override"); + assertFalse(persistedState().containsKey(KEY)); + assertFalse(persistedState().containsKey(OVERRIDE_KEY)); + } + + @Test + public void testLegacyHiddenSnapshotBecomesOverride() { + persistedState().put(KEY, Boolean.FALSE.toString()); + + // an unchanged value, only to make the window read its persisted state again + preferences.firePropertyChangeEvent(KEY, Boolean.TRUE, Boolean.TRUE); + processEvents(); + + assertFalse(window.getCoolBarVisible(), "a hidden bar was a user choice and must stay hidden"); + assertFalse(persistedState().containsKey(KEY)); + assertEquals(Boolean.FALSE.toString(), persistedState().get(OVERRIDE_KEY)); + } + + private Map persistedState() { + return window.getModel().getPersistedState(); + } +}