From 0bdba0086353a055aa18c8922473cc8c98f3c745 Mon Sep 17 00:00:00 2001 From: Philipp SALZGEBER Date: Fri, 2 Oct 2026 11:01:38 +0200 Subject: [PATCH] WizardDialog resizes itself in an endless loop after a DPI change eclipse-platform/eclipse.platform#2986 WizardDialog builds its shell with SWT.RESIZE but inherits isResizable() == false from Dialog, so hookZoomChangeListenerForDefaultSizedDialogs() installs adaptDefaultSizeListener on every wizard. That listener calls shell.setSize(shell.computeSize(...)), which sends the next SWT.Resize, and it has no re-entrancy guard: whenever the computed size is not a fixed point the listener recurses until the stack is exhausted. Let WizardDialog report the SWT.RESIZE bit of its shell style, and make the listener skip a nested call and a size that has not changed. Fixes https://github.com/eclipse-platform/eclipse.platform/issues/2986 Signed-off-by: Philipp SALZGEBER --- .../src/org/eclipse/jface/dialogs/Dialog.java | 18 ++- .../eclipse/jface/wizard/WizardDialog.java | 10 ++ .../jface/tests/dialogs/DialogTest.java | 110 ++++++++++++++++++ .../tests/wizards/TheTestWizardDialog.java | 5 + .../jface/tests/wizards/WizardTest.java | 37 ++++++ 5 files changed, 177 insertions(+), 3 deletions(-) diff --git a/bundles/org.eclipse.jface/src/org/eclipse/jface/dialogs/Dialog.java b/bundles/org.eclipse.jface/src/org/eclipse/jface/dialogs/Dialog.java index 12e534ad85b..11e5cbfdf73 100644 --- a/bundles/org.eclipse.jface/src/org/eclipse/jface/dialogs/Dialog.java +++ b/bundles/org.eclipse.jface/src/org/eclipse/jface/dialogs/Dialog.java @@ -16,6 +16,7 @@ import java.util.Arrays; import java.util.HashMap; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import org.eclipse.core.runtime.IProgressMonitor; @@ -1109,13 +1110,24 @@ private void hookZoomChangeListenerForDefaultSizedDialogs() { } AtomicReference customResizeListener = new AtomicReference<>(); + // setSize() sends the next SWT.Resize, which calls this listener again. Without the + // re-entrancy guard the listener recurses until the stack is exhausted whenever the + // computed size is not a fixed point, e.g. because a scrollbar appears and disappears + // with it. + AtomicBoolean adaptingDefaultSize = new AtomicBoolean(); Listener adaptDefaultSizeListener = event -> { Shell shell = getShell(); - if (shell == null || shell.isDisposed()) { + if (shell == null || shell.isDisposed() || !adaptingDefaultSize.compareAndSet(false, true)) { return; } - Point size = shell.computeSize(SWT.DEFAULT, SWT.DEFAULT, true); - shell.setSize(size); + try { + Point size = shell.computeSize(SWT.DEFAULT, SWT.DEFAULT, true); + if (!size.equals(shell.getSize())) { + shell.setSize(size); + } + } finally { + adaptingDefaultSize.set(false); + } }; AtomicReference zoomChangeListener = new AtomicReference<>(); zoomChangeListener.set(event -> { diff --git a/bundles/org.eclipse.jface/src/org/eclipse/jface/wizard/WizardDialog.java b/bundles/org.eclipse.jface/src/org/eclipse/jface/wizard/WizardDialog.java index 562d5493509..6b83af04a29 100644 --- a/bundles/org.eclipse.jface/src/org/eclipse/jface/wizard/WizardDialog.java +++ b/bundles/org.eclipse.jface/src/org/eclipse/jface/wizard/WizardDialog.java @@ -342,6 +342,16 @@ public int getShellStyle() { return super.getShellStyle(); } + /** + * A wizard dialog is resizable unless a caller has stripped {@link SWT#RESIZE} from its shell + * style, so the answer is read from that style instead of being inherited from + * Dialog, which always answers false. + */ + @Override + protected boolean isResizable() { + return (getShellStyle() & SWT.RESIZE) != 0; + } + private static int getShellModality(boolean modal) { return modal ? SWT.PRIMARY_MODAL : SWT.NONE; } diff --git a/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/dialogs/DialogTest.java b/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/dialogs/DialogTest.java index c8b47776b56..2739aef99b1 100644 --- a/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/dialogs/DialogTest.java +++ b/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/dialogs/DialogTest.java @@ -13,20 +13,30 @@ ******************************************************************************/ package org.eclipse.jface.tests.dialogs; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeFalse; +import static org.junit.jupiter.api.Assumptions.assumeTrue; import org.eclipse.jface.dialogs.Dialog; import org.eclipse.jface.dialogs.IDialogConstants; import org.eclipse.swt.SWT; +import org.eclipse.swt.graphics.Point; +import org.eclipse.swt.layout.GridData; import org.eclipse.swt.widgets.Button; import org.eclipse.swt.widgets.Composite; import org.eclipse.swt.widgets.Control; +import org.eclipse.swt.widgets.Event; import org.eclipse.swt.widgets.Shell; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; public class DialogTest { + /** Width the growing test content adds per size computation. */ + private static final int GROWTH = 50; + /** * The dialog being tested. */ @@ -77,6 +87,106 @@ public void testButtonAlignmentBug272583() { forceLayoutDialog.close(); } + /** + * A default-sized, non-resizable dialog adapts its size after a zoom change. + * The adaptation calls Shell.setSize, which sends the next + * SWT.Resize to the same listener. The listener must not recurse + * when the computed size keeps changing, must still adapt on a later resize, + * and must not resize when the computed size is unchanged. + */ + @Test + public void testZoomChangeAdaptationOfDefaultSizedDialogTerminates() { + GrowingContentDialog growingDialog = new GrowingContentDialog(); + dialog = growingDialog; + growingDialog.setBlockOnOpen(false); + growingDialog.create(); + Shell shell = growingDialog.getShell(); + Control firstChild = shell.getChildren()[0]; + Point initialSize = shell.getSize(); + // the adaptation is hooked only for a dialog that reports its default size + // after creation; where the platform reports another size for a shell that + // is not open yet, there is nothing to test + assumeFalse(growingDialog.resizeOccurredBeforeInitializeBounds(), + "a resize before initializeBounds marks the dialog as custom-sized"); + assumeTrue(initialSize.equals(shell.computeSize(SWT.DEFAULT, SWT.DEFAULT, true)), + "the dialog must report its default size for the adaptation to be hooked"); + + // every computed size differs from the previous one, so a listener without + // re-entrancy guard recurses until the stack is exhausted + int widthBeforeZoomChange = growingDialog.content.preferredWidth; + growingDialog.content.growOnEveryComputeSize = true; + firstChild.notifyListeners(SWT.ZoomChanged, new Event()); + shell.notifyListeners(SWT.Resize, new Event()); + assertTrue(growingDialog.content.preferredWidth > widthBeforeZoomChange, + "the adaptation computes the size after the zoom change"); + Point adaptedSize = shell.getSize(); + assertNotEquals(initialSize, adaptedSize, "the dialog adapts its size once after the zoom change"); + + // a later independent resize adapts to the new preferred size + growingDialog.content.growOnEveryComputeSize = false; + growingDialog.content.preferredWidth += GROWTH; + shell.notifyListeners(SWT.Resize, new Event()); + Point resizedSize = shell.getSize(); + assertNotEquals(adaptedSize, resizedSize, "a later resize adapts the dialog to its new preferred size"); + + // an unchanged computed size causes no further resize + int[] resizeEvents = { 0 }; + shell.addListener(SWT.Resize, event -> resizeEvents[0]++); + shell.notifyListeners(SWT.Resize, new Event()); + assertEquals(resizedSize, shell.getSize(), "an unchanged computed size keeps the size"); + assertEquals(1, resizeEvents[0], "an unchanged computed size sends no further resize"); + } + + /** + * A dialog whose content reports a preferred width that can grow with every + * computation, so that the computed shell size is never a fixed point while + * growing is on. + */ + private class GrowingContentDialog extends Dialog { + + GrowingContent content; + + GrowingContentDialog() { + super((Shell) null); + } + + boolean resizeOccurredBeforeInitializeBounds() { + return resizeHasOccurred; + } + + @Override + protected Control createDialogArea(Composite parent) { + Composite area = (Composite) super.createDialogArea(parent); + content = new GrowingContent(area); + content.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true)); + return area; + } + } + + /** + * The content has to be wider and higher than the rest of the dialog, so that + * its preferred size determines the computed shell size on every platform. A + * narrow content would leave the button bar as the widest part, and growing it + * would not change the size of the shell. + */ + private static class GrowingContent extends Composite { + + int preferredWidth = 600; + boolean growOnEveryComputeSize; + + GrowingContent(Composite parent) { + super(parent, SWT.NONE); + } + + @Override + public Point computeSize(int wHint, int hHint, boolean changed) { + if (growOnEveryComputeSize) { + preferredWidth += GROWTH; + } + return new Point(preferredWidth, 400); + } + } + /** * A dialog that explicitly invokes a layout operation prior to the shell being * opened. Calls to moveBelow(Control) should be followed by a layout operation diff --git a/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/wizards/TheTestWizardDialog.java b/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/wizards/TheTestWizardDialog.java index ad488612a48..8ccfd096843 100644 --- a/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/wizards/TheTestWizardDialog.java +++ b/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/wizards/TheTestWizardDialog.java @@ -35,6 +35,11 @@ public Button getFinishedButton() { return getButton(IDialogConstants.FINISH_ID); } + @Override + public boolean isResizable() { + return super.isResizable(); + } + @Override public Button getCancelButton() { return getButton(IDialogConstants.CANCEL_ID); diff --git a/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/wizards/WizardTest.java b/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/wizards/WizardTest.java index 46fb8d1b9b8..9e033f06bce 100644 --- a/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/wizards/WizardTest.java +++ b/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/wizards/WizardTest.java @@ -25,8 +25,11 @@ import org.eclipse.jface.dialogs.IPageChangingListener; import org.eclipse.jface.util.ILogger; import org.eclipse.jface.util.Policy; +import org.eclipse.swt.SWT; +import org.eclipse.swt.graphics.Point; import org.eclipse.swt.graphics.RGB; import org.eclipse.swt.widgets.Display; +import org.eclipse.swt.widgets.Event; import org.eclipse.swt.widgets.Shell; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -253,6 +256,40 @@ public void testWizardPageDispose() { } + /** + * A wizard dialog is resizable, so a zoom change must not install the + * adaptation to the computed size that non-resizable dialogs get: a size + * requested afterwards has to stay. + */ + @Test + public void testResizableWizardKeepsRequestedSizeAfterZoomChange() { + assertTrue(dialog.isResizable(), "a wizard dialog with the default shell style is resizable"); + Shell shell = dialog.getShell(); + + shell.getChildren()[0].notifyListeners(SWT.ZoomChanged, new Event()); + Point requestedSize = new Point(shell.getSize().x + 60, shell.getSize().y + 40); + shell.setSize(requestedSize); + shell.notifyListeners(SWT.Resize, new Event()); + + assertEquals(requestedSize, shell.getSize(), "the requested size is kept after a zoom change"); + } + + /** + * Removing SWT.RESIZE from the shell style before the shell is created gives a + * fixed-size wizard dialog. + */ + @Test + public void testWizardWithoutResizeStyleIsNotResizable() { + TheTestWizardDialog fixedSizeDialog = new TheTestWizardDialog(null, new TheTestWizard()); + fixedSizeDialog.setShellStyle(fixedSizeDialog.getShellStyle() & ~SWT.RESIZE); + try { + fixedSizeDialog.create(); + assertFalse(fixedSizeDialog.isResizable(), "a wizard dialog without SWT.RESIZE is not resizable"); + } finally { + fixedSizeDialog.close(); + } + } + //---------------------------------------------------- @BeforeEach