[PATCH 3/3] user32: Fix groupbox rectangle calculation in the button's WM_SETTEXT handler.

Sebastian Lackner sebastian at fds-team.de
Mon Feb 6 01:41:02 CST 2017


On 06.02.2017 07:55, Dmitry Timoshkov wrote:
> Nikolay Sivov <bunglehead at gmail.com> wrote:
> 
>>>> +            /* FIXME: check other BS_* handlers */
>>>> +            if (btn_type == BS_GROUPBOX)
>>>> +                InflateRect(&rc, -7, 1); /* GB_Paint does this */
>>>
>>> How about doing what FIXME suggests?
>>
>> Sure, at some point. Original report is for group box, fix is limited to it.
> 
> Yes, my original patch is only for BS_GROUPBOX buttons, but that doesn't
> mean that other button style handlers don't have similar bugs. There are
> not that much of button styles to carfully review and fix if required,
> and if for some reason you've decided to resubmit the staged patches
> you should be prepared to spend time to improve the original patches,
> in this particular case take care of the FIXME.
> 
>>> Adding some tests wouldn't hurt either.
>>>
>>
>> Test what, filled rectangle size? I don't think it makes sense.
> 
> This patchset fixes several things, and each of them deserves its own
> testing. For instance testing an original font selected into DC should
> be easy enough.
> 
> There are multiple reasons why staged patches are not sent directly to
> wine-patches, and one of them is that they need more work. If you are
> not ready to do that part of the work (except of just copy/pasting an
> original patch) then you shouldn't touch the staged patches.
> 

It is good that you are pointing out possible issues, however it doesn't
make much sense to be too picky. What Nikolay is doing here is absolutely
fine, and I appreciate the effort to help with upstreaming. Unless you
are planning to send an improved version yourself in the near future, I
do not see why such a partial fix should not be accepted - especially when
its not a hack (other paint handlers already restore the font, for example)
and reviewers are fine with it.

For what its worth, the handler for BS_DEFPUSHBUTTON might be affected
by a similar problem. No big deal to send this change separately though.

Best regards,
Sebastian




More information about the wine-devel mailing list