[Win] Enabled Image+Text button ignores alignment #3277

[tmssngr]

It looks like the old comment The default button with an image doesn't respect the text alignment. So we do the same for styled buttons. is outdated.

[github-actions]

Test Results (win32)

   30 files  ±0     30 suites  ±0   5m 8s ⏱️ +11s
4 697 tests ±0  4 622 ✅ ±0  75 💤 ±0  0 ❌ ±0 
1 227 runs  ±0  1 203 ✅ ±0  24 💤 ±0  0 ❌ ±0 

Results for commit d3b5ec6. ± Comparison against base commit 65f43d4.

[tmssngr]

Are there any objections for this PR?

[HeikoKlare]

It looks like the old comment The default button with an image doesn't respect the text alignment. So we do the same for styled buttons. is outdated.

Do we know when the behavior changed? So do we need to guard this behavior with a specific Windows version constraint at which the default button style changed?
The code was introduced with 0822f20 and 82b8cf4, which was after the original Windows 10 release, of which we still support some later versions.

[tmssngr]

Unfortunately, I don't have older Windows 10 versions to test.

[HeikoKlare]

Did you take a look into the original bug report to maybe find out why that code was introduced, i.e., how the strange default behavior of Windows buttons was detected? There might be some pointer to where you could look up if/when that behavior changed. Or maybe there is at least some information that can be used as input for an AI agent making according research.

[ptziegler]

I just want to mention that there is a general problem with the alignment when both text and image is set. See #519. Perhaps something was missed in that fix?

[HeikoKlare]

That's a great pointer, thank you. I just reverted the referenced change for the sake of testing and see the disabled buttons not considering the alignment again (using the snippet from #3277).
So it seems like it was not a change on OS side that fixed the default button behavior but the mentioned SWT-side fix. The change proposed here may thus be considered a follow-up change to that PR to make the behavior consistent again.

[Copilot]

Pull request overview

This PR updates Win32 button custom drawing so enabled buttons with both image and text can honor SWT text alignment instead of always forcing left-aligned text.

Changes:

• Adds extra text margin when a non-check/radio button has an image.
• Removes the special-case image path that forced DT_LEFT, allowing existing left/right/center alignment logic to apply.

[HeikoKlare]

In general, the change looks sound to me. We do not need to guard this behavior in any way, now that we understood that the inconsistency came from:

• #895

I further tested with Issue0519_ButtonAlignmentExample added in the mentioned PR. I extended it with setting foreground for the buttons as done in the snippet for the issue to be solved by this PR, and I added a checkbox to enable/disable the buttons. You can see that the alignment of texts in enabled and disabled buttons does not fit for every alignment setting:

@tmssngr can you please extend the mentioned example for this PR to reflect the issue to be solved as well and further enhance the PR so that we see the alignment issues fixed there?

[tmssngr]

@HeikoKlare I've updated Issue0519_ButtonAlignmentExample (like you've did) and fixed also the tiny text movements between enabled and disabled state.

[github-actions]

Test Results

  182 files  ±0    182 suites  ±0   25m 32s ⏱️ - 1m 36s
4 723 tests ±0  4 700 ✅ ±0   23 💤 ±0  0 ❌ ±0 
6 812 runs  ±0  6 649 ✅ ±0  163 💤 ±0  0 ❌ ±0 

Results for commit c373a8b. ± Comparison against base commit 7f9089e.