Re: wineconsole[4/6]: allow making selections with a mouse also when not in the QuickEdit mode

[Eric Pouech]

Mikołaj Zalewski wrote:
> The user may start the selection from the popup menu. The mouse should 
> work then
>
> ------------------------------------------------------------------------
>
> diff --git a/programs/wineconsole/user.c b/programs/wineconsole/user.c
> index 557ccfc..c4250e5 100644
> --- a/programs/wineconsole/user.c
> +++ b/programs/wineconsole/user.c
> @@ -1116,12 +1116,14 @@ static LRESULT CALLBACK WCUSER_Proc(HWND
>   WCUSER_GenerateKeyInputRecord(data, uMsg == WM_SYSKEYDOWN, wParam, lParam, TRUE);
>   break;
>     case WM_LBUTTONDOWN:
> -        if (data->curcfg.quick_edit)
> +        if (data->curcfg.quick_edit || PRIVATE(data)->has_selection)
>         {
>             if (PRIVATE(data)->has_selection)
> +                WCUSER_SetSelection(data, 0);
> +            
> +            if (data->curcfg.quick_edit && PRIVATE(data)->has_selection)
>             {
>                 PRIVATE(data)->has_selection = FALSE;
> -                WCUSER_SetSelection(data, 0);
>             }
>             else
>             {
> @@ -1137,7 +1139,7 @@ static LRESULT CALLBACK WCUSER_Proc(HWND
>         }
>   break;
>     case WM_MOUSEMOVE:
> -        if (data->curcfg.quick_edit)
> +        if (data->curcfg.quick_edit || PRIVATE(data)->has_selection)
>         {
>             if (GetCapture() == PRIVATE(data)->hWnd && PRIVATE(data)->has_selection &&
>                 (wParam & MK_LBUTTON))
> @@ -1151,14 +1153,13 @@ static LRESULT CALLBACK WCUSER_Proc(HWND
>         }
>   break;
>     case WM_LBUTTONUP:
> -        if (data->curcfg.quick_edit)
> +        if (data->curcfg.quick_edit || PRIVATE(data)->has_selection)
>         {
>             if (GetCapture() == PRIVATE(data)->hWnd && PRIVATE(data)->has_selection &&
>                 (wParam& MK_LBUTTON))
>             {
>                 WCUSER_MoveSelection(data, PRIVATE(data)->selectPt1, WCUSER_GetCell(data, lParam));
>                 ReleaseCapture();
> -                PRIVATE(data)->has_selection = FALSE;

this looks suspicious to me ? how to do end the selection ? the only 
time you do it is when a second button down :-/
> }
>         }
>         else
>
> ------------------------------------------------------------------------

wineconsole[4/6]: allow making selections with a mouse also when not in the QuickEdit mode

[Mikołaj Zalewski]

> > case WM_LBUTTONUP:
> > -        if (data->curcfg.quick_edit)
> > +        if (data->curcfg.quick_edit || PRIVATE(data)->has_selection)
> >         {
> >             if (GetCapture() == PRIVATE(data)->hWnd && 
> > PRIVATE(data)->has_selection &&
> >                 (wParam& MK_LBUTTON))
> >             {
> >                 WCUSER_MoveSelection(data, PRIVATE(data)->selectPt1, 
> > WCUSER_GetCell(data, lParam));
> >                 ReleaseCapture();
> > -                PRIVATE(data)->has_selection = FALSE;
>
> this looks suspicious to me ? how to do end the selection ? the only 
> time you do it is when a second button down :-/

The selection is cancelled if the hits Enter to copy the data to the 
clipboard (or after the later patch hit any other key to cancel). The 
current behaviour is strange as why to select a rectangle with a mouse 
if it will be unselected on WM_LBUTTONUP? Currectly the user would need 
to keep the mouse button down and press Enter to copy the data and have 
any use of the mouse selection.

Mikolaj Zalewski

wineconsole[4/6]: allow making selections with a mouse also when not in the QuickEdit mode

[Eric Pouech]

Mikołaj Zalewski wrote:
> > > case WM_LBUTTONUP:
> > > -        if (data->curcfg.quick_edit)
> > > +        if (data->curcfg.quick_edit || PRIVATE(data)->has_selection)
> > >         {
> > >             if (GetCapture() == PRIVATE(data)->hWnd && 
> > > PRIVATE(data)->has_selection &&
> > >                 (wParam& MK_LBUTTON))
> > >             {
> > >                 WCUSER_MoveSelection(data, PRIVATE(data)->selectPt1, 
> > > WCUSER_GetCell(data, lParam));
> > >                 ReleaseCapture();
> > > -                PRIVATE(data)->has_selection = FALSE;
> >
> > this looks suspicious to me ? how to do end the selection ? the only 
> > time you do it is when a second button down :-/
>
> The selection is cancelled if the hits Enter to copy the data to the 
> clipboard (or after the later patch hit any other key to cancel). The 
> current behaviour is strange as why to select a rectangle with a mouse 
> if it will be unselected on WM_LBUTTONUP? Currectly the user would 
> need to keep the mouse button down and press Enter to copy the data 
> and have any use of the mouse selection.
>
> Mikolaj Zalewski

but your code doesn't release the selection on button up, which is 
rather strange when you do only mouse selection
A+

wineconsole[4/6]: allow making selections with a mouse also when not in the QuickEdit mode

[Mikołaj Zalewski]

> but your code doesn't release the selection on button up, which is 
> rather strange when you do only mouse selection
> A+

I've checked it and currently the whole section is dead code as on 
WM_LBUTTONUP the (wParam&MK_LBUTTON) is always false.

However if I remove this condition I think my code is correct. I release 
the mouse capture - that's enough as the handler for WM_MOUSEMOVE and 
WM_LBUTTONUP checks if GetCapture() == PRIVATE(data)->hWnd. Setting 
has_selection = FALSE would means that the seletion has been removed - 
there would be no way to copy it to the clipboard. Also the window is 
not updated to show that the selection is removed.

I'll check if the spaces are removed for other console modes. I don't 
know if it's possible to write an automatic test as this requires mouse 
interaction, I don't know how to get a hWnd of a console windows. Also 
it would fail on wine as by default it starts the ncurses backend.

Mikolaj Zalewski

wineconsole[4/6]: allow making selections with a mouse also when not in the QuickEdit mode

[Eric Pouech]

Mikołaj Zalewski wrote:
> > but your code doesn't release the selection on button up, which is 
> > rather strange when you do only mouse selection
> > A+
>
> I've checked it and currently the whole section is dead code as on 
> WM_LBUTTONUP the (wParam&MK_LBUTTON) is always false.
>
> However if I remove this condition I think my code is correct. I 
> release the mouse capture - that's enough as the handler for 
> WM_MOUSEMOVE and WM_LBUTTONUP checks if GetCapture() == 
> PRIVATE(data)->hWnd. Setting has_selection = FALSE would means that 
> the seletion has been removed - there would be no way to copy it to 
> the clipboard. Also the window is not updated to show that the 
> selection is removed.

I was mostly surprised by the fact you only removed the 
has_selection=false part. The whole change (the new version) makes more 
sense
> I'll check if the spaces are removed for other console modes.

I tested it too (less extensively than you did), but came to the same 
conclusion :-/
> I don't know if it's possible to write an automatic test as this 
> requires mouse interaction, I don't know how to get a hWnd of a 
> console windows.

GetConsoleWindow(), which is not implemented in Wine yet. Also, it's not 
clear whether it's the top level window or the child window. You could 
send the mouse events to that window. But that's not a top priority 
feature. It's better to fix the broken parts of wineconsole ;-)
> Also it would fail on wine as by default it starts the ncurses backend.

the test could force the user backend (see the console test in kernel32 
directory)
A+

wineconsole[4/6]: allow making selections with a mouse also when not in the QuickEdit mode

[Mikołaj Zalewski]

> the test could force the user backend (see the console test in 
> kernel32 directory)

I see - I haven't noticed that --use-event automatically chooses the 
user backend.

Mikolaj Zalewski