Skip to content

Workaround for unexpected text wrapping in ExpandableComposite #2986

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

akoch-yatta
Copy link
Contributor

This PR adds one additional point to the width when calculating the minimum size of the textLabel in ExpandableComposite. This serves as a workaround for a current limitation in the SWT implementation on Windows. With certain zoom settings (e.g., 125% or 175%), depending on the length of the label text, the calculated width may be too small, causing the text to wrap unnecessarily.

This is intended as a temporary workaround. If we merge this, we should either revert it after the release to reintroduce the issue or have hopefully a proper fix inside of SWT.

I want to emphasize this PR is only a workaround not my preferred one, but the only one, I see feasible as of now. We could still come to the conclusion to not merge/close this PR if the effect is acceptable for now until this issue can be solved inside of SWT.

How to test

You can see this effect e.g. in the Manifest Editor as well. You should see the issue in 175% in the Build tab and the Runtime Information section.

Screenshot before

before

Screenshot after

after

Copy link
Contributor

github-actions bot commented May 15, 2025

Test Results

 1 824 files  ±0   1 824 suites  ±0   1h 38m 2s ⏱️ - 6m 23s
 7 925 tests ±0   7 697 ✅ ±0  228 💤 ±0  0 ❌ ±0 
23 862 runs  ±0  23 114 ✅ ±0  748 💤 ±0  0 ❌ ±0 

Results for commit fa193e6. ± Comparison against base commit 208faf9.

♻️ This comment has been updated with latest results.

This commit adds one additional point to the width when calculating the
minimum size of the textLabel in ExpandableComposite. This serves as a
workaround for a current limitation in the SWT implementation on Windows.
With certain zoom settings (e.g., 125% or 175%), depending on the length
of the label text, the calculated width may be too small, causing the
text to wrap unnecessarily. This is intended as a temporary workaround.

Contributes to eclipse-platform#2980
Copy link
Contributor

@fedejeanne fedejeanne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this and it fixes the issue without any side-effects.

The class is referenced a lot and it is widely used in the code so it's difficult to foresee any unwanted side-effect (it might be that I didn't find any because I tested only superficially), but since the bug being fixed is so present and I expect it to happen quite often and since this change is "only 1 pixel" and it is only for Windows, I think we could take a chance and merge it for RC1.

Any objections?

@akoch-yatta
Copy link
Contributor Author

I tried to evaluate the layout a bit more. one thing to mention is this change cannot change the overall size, because of the min check in int labelWidthBeforeSplit = Math.min(width, labelDefault.x + additionalLabelWidthPadding).

@fedejeanne
Copy link
Contributor

So what you're saying is that this PR only affects the label in the header (title) of the section but not the overall size of the section itself...
... which for me translates as "it's very unlikely/impossible that this has side-effects"

Am I correct?

@akoch-yatta
Copy link
Contributor Author

So what you're saying is that this PR only affects the label in the header (title) of the section but not the overall size of the section itself... ... which for me translates as "it's very unlikely/impossible that this has side-effects"

Am I correct?

I think so. The size of the section itself should depend on computeSize and this method is not touched.

@fedejeanne fedejeanne merged commit 710a26a into eclipse-platform:master May 19, 2025
18 checks passed
@fedejeanne fedejeanne deleted the workaround-missing-text-in-jface-forms-section branch May 19, 2025 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants