[Win32] Complete zoom change processing despite failing widgets - #3578
Open
HeikoKlare wants to merge 1 commit into
Open
[Win32] Complete zoom change processing despite failing widgets#3578HeikoKlare wants to merge 1 commit into
HeikoKlare wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
It changes core zoom-change exception propagation semantics across many Win32 widget types and should receive a final human review for behavioral compatibility and edge cases.
Pull request overview
This PR strengthens Win32 zoom/DPI change propagation so that failures in individual widgets (often due to disposed app-owned resources) no longer abort processing of the remaining widget tree, and so shell layout / monitor move still completes; any unconsumed failures are deferred and surfaced after processing finishes.
Changes:
- Introduces a per-zoom-change failure collection mechanism (using
ExceptionStash) so exceptions don’t short-circuit propagation. - Wraps zoom-changed notifications to isolate failures per widget/subtree and ensure remaining widgets still receive
SWT.ZoomChanged. - Adds Win32 tests covering sibling/subtree continuation and “not consumed” exception propagation timing.
File summaries
| File | Description |
|---|---|
| bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Widget.java | Adds notifyZoomChanged and stashZoomChangeFailure helpers to isolate/stash failures during zoom change propagation. |
| bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Control.java | Extends zoom change execution to collect exceptions across tasks and rethrow after completion (post-layout). |
| bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Shell.java | Uses notifyZoomChanged and ensures shell is moved to new monitor even if some widget processing failed. |
| bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Composite.java | Ensures a composite’s own adaptation failure doesn’t prevent its children from being adapted. |
| bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/TreeItem.java | Isolates item adaptation failures so child items still receive zoom change propagation. |
| bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Tree.java | Uses notifyZoomChanged for columns/items to avoid aborting on individual failures. |
| bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Table.java | Uses notifyZoomChanged for items/columns to avoid aborting on individual failures. |
| bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolBar.java | Uses notifyZoomChanged per item to continue processing despite failures. |
| bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/TabFolder.java | Uses notifyZoomChanged for tab items to isolate failures. |
| bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Menu.java | Uses notifyZoomChanged for menu items to isolate failures. |
| bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/MenuItem.java | Uses notifyZoomChanged for sub-menu propagation to isolate failures. |
| bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Decorations.java | Uses notifyZoomChanged for menu bar and tracked menus to isolate failures. |
| bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/CoolBar.java | Uses notifyZoomChanged for embedded controls during zoom change. |
| bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ExpandBar.java | Uses notifyZoomChanged for expand items and continues with layout/redraw. |
| bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/widgets/ZoomChangeExceptionHandlingWin32Tests.java | Adds targeted Win32 tests validating continuation and deferred propagation semantics. |
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+2736
to
+2738
| * outside the control of SWT. A failure of SWT's own native bookkeeping is a | ||
| * defect that is not worth continuing the propagation over an inconsistent | ||
| * state for. |
Contributor
Author
There was a problem hiding this comment.
Half right, and the useful half is fixed.
There is no behavioural mismatch: that paragraph is not about what notifyZoomChanged() does with what it catches, it is the rule for where a widget places its own guard inside handleDPIChange. "Continuing the propagation" meant that widget's propagation to its sub-widgets, not the global one. If, say, Table's image list bookkeeping fails, the table has no local guard and its items are not notified, while the failure is still collected by whoever sent the event to the table, so the rest of the tree and the concluding layout are unaffected.
The wording did invite exactly your reading, though, especially next to the first paragraph, which talks about the global propagation. I have reworded it to say explicitly which propagation is skipped and that every other widget is adapted in either case.
Contributor
A zoom change is propagated from the shell through the whole widget tree and is concluded by laying out the shell. Adapting a widget to the new zoom can fail because of resources provided by the application, such as an image or a region that has already been disposed when the adaptation re-applies it. Such a failure is contained per listener by the event table, which reports it to the exception handlers of the display. It is not contained in two cases, each of which leaves parts of the UI rendered with a stale zoom: * If no handler consuming the failure is installed, it is re-thrown by the event table and aborts the propagation to all widgets that have not been processed yet, as well as the concluding layout of the shell and its move to the new monitor. * A failure of a widget's own adaptation that happens before the widget propagates the event further, as done by composites for their children and by tree items for their child items, skips the complete subtree of that widget. The execution of a zoom change, which already knows when the zoom change has been processed by all widgets, therefore also becomes the place where failures are collected. Every widget the event is sent to is isolated: its failure is reported to the exception handlers immediately and, if those do not consume it, propagated only once the zoom change is complete, so that no widget is left behind and the shell is laid out in any case. Contributes to eclipse-platform#2432 Assisted-by: Claude Opus 5 <noreply@anthropic.com>
HeikoKlare
force-pushed
the
zoom-change-exception-isolation
branch
from
September 7, 2026 18:58
cc4c987 to
4486be1
Compare
HeikoKlare
marked this pull request as ready for review
September 7, 2026 18:58
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A zoom change is propagated from the shell through the whole widget tree and is concluded by laying out the shell. Adapting a widget to the new zoom can fail because of resources provided by the application, such as an image or a region that has already been disposed by the time the adaptation re-applies it.
Such a failure is contained per listener by the event table, which reports it to the exception handlers of the display. It is not contained in two cases, each of which leaves parts of the UI rendered with a stale zoom. First, if the application installs no handler that consumes the failure, the event table re-throws it, which aborts the propagation to all widgets that have not been processed yet, the concluding layout of the shell, and its move to the new monitor. Second, a failure of a widget's own adaptation that happens before the widget propagates the event further, as composites do for their children and tree items for their child items, skips the complete subtree of that widget.
The execution of a zoom change already knows when the change has been processed by all widgets, so it also becomes the place where failures are collected. Every widget the event is sent to is isolated: its failure is reported to the exception handlers immediately and, if those do not consume it, propagated only once the zoom change is complete. No widget is left behind and the shell is laid out in any case. The added tests cover the siblings of a failing control, the subtree of a failing composite and of a failing tree item, the remaining items and columns of a table, the remaining menu items, and the case in which no handler consumes the failure, where everything is still adapted before the failure surfaces. Applications notice a difference only if they have no consuming exception handler installed: the failure then arrives after the zoom change has been processed instead of in the middle of it.
The gap was identified as a side outcome of the review of #3553, see #3553 (review). The same class of problem was reported and fixed a year ago in #2432, at a time when the zoom change processing was still synchronous; that safety net was lost with the switch to asynchronous processing, which this change restores in a form that fits the asynchronous execution.
🤖 Generated with Claude Code