Thanks Ken, I'll make those changes.
On Sunday, January 31, 2016, Ken Thomases <ken(a)codeweavers.com> wrote:
> On Jan 31, 2016, at 7:18 PM, Christopher Thielen <cthielen(a)gmail.com
> <javascript:;>> wrote:
> >
> > A window may be notified with WM_CAPTURECHANGED about itself
> > gaining mouse capture if it calls SetCapture() twice.
> >
> > Tested on Windows XP and Fedora Workstation 22.
> >
> > Revised to simplify based on wine-devel feedback.
> > Revised to address any side-effects of new WM_CAPTURECHANGED
> > messages.
>
> The changes to comctl32 should be in a separate patch. In general,
> changes to different DLLs should be in separate patches unless it can't be
> avoided. The comctl32 patch should come before the change to when
> WM_CAPTURECHANGED is sent so that things aren't broken part-way through the
> patch series. You might also put the button.c changes in a separate patch,
> too.
>
> > case WM_CAPTURECHANGED:
> > - return TOOLBAR_CaptureChanged(infoPtr);
> > + if(hwnd != lParam)
> > + {
> > + return TOOLBAR_CaptureChanged(infoPtr);
> > + }
> > + else
> > + {
> > + return 0;
> > + }
>
> The second return can be hoisted out of the else since the true branch
> can't fall through. Also, you can reverse the logic, so the code can look
> like:
>
> case WM_CAPTURECHANGED:
> if(hwnd == lParam)
> return 0;
> return TOOLBAR_CaptureChanged(infoPtr);
>
> I think that's clearer and also a smaller diff. Same for the other files.
>
> Cheers,
> Ken
>
>
On Jan 31, 2016, at 7:18 PM, Christopher Thielen <cthielen(a)gmail.com> wrote:
>
> A window may be notified with WM_CAPTURECHANGED about itself
> gaining mouse capture if it calls SetCapture() twice.
>
> Tested on Windows XP and Fedora Workstation 22.
>
> Revised to simplify based on wine-devel feedback.
> Revised to address any side-effects of new WM_CAPTURECHANGED
> messages.
The changes to comctl32 should be in a separate patch. In general, changes to different DLLs should be in separate patches unless it can't be avoided. The comctl32 patch should come before the change to when WM_CAPTURECHANGED is sent so that things aren't broken part-way through the patch series. You might also put the button.c changes in a separate patch, too.
> case WM_CAPTURECHANGED:
> - return TOOLBAR_CaptureChanged(infoPtr);
> + if(hwnd != lParam)
> + {
> + return TOOLBAR_CaptureChanged(infoPtr);
> + }
> + else
> + {
> + return 0;
> + }
The second return can be hoisted out of the else since the true branch can't fall through. Also, you can reverse the logic, so the code can look like:
case WM_CAPTURECHANGED:
if(hwnd == lParam)
return 0;
return TOOLBAR_CaptureChanged(infoPtr);
I think that's clearer and also a smaller diff. Same for the other files.
Cheers,
Ken
Hi,
While running your changed tests on Windows, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?
Full results can be found at
https://testbot.winehq.org/JobDetails.pl?Key=20122
Your paranoid android.
=== w1064 (32 bit msg) ===
msg.c:6290: Test failed: Update region shouldn't be empty
msg.c:6524: Test failed: InvalidateErase: 0: the msg sequence is not complete: expected 0085 - actual 0000
msg.c:6526: Test failed: Paint: 0: the msg sequence is not complete: expected 000f - actual 0000
msg.c:6296: Test failed: Regions are different
msg.c:6296: Test failed: Regions are different
msg.c:6296: Test failed: Regions are different
