wpf icon indicating copy to clipboard operation
wpf copied to clipboard

DragLeave handlers can modify AllowedEffects of following DragEnter handlers

Open weltkante opened this issue 7 years ago • 7 comments

  • .NET Core Version: 3.0.100-preview-009812
  • Windows version: 1809
  • Does the bug reproduce also in WPF for .NET Framework 4.8? Yes

Project maintainers will consider changes that improve the product or fix known bugs (please file issues to make bugs "known").

Re-reporting a bug I previously reported against Desktop Framework WPF which was decided not to get fixed because "it isn't broken enough to require a fix". My opinion differs so I'll try again to get this fixed.

Problem description: DragLeave handlers can modify the AllowedEffects of the following DragEnter handler by modifying their DragEventArgs.Effects property. Can happen accidently when pushing all the different drag events through a common handler. The problem is transient (fixes itself on the next mouse move because the DragLeave handler won't be called again) but causes cursor flickering due to the incorrect AllowedEffects in DragEnter. If the DragLeave handler is in a 3rd party control you can't easily fix the cursor flickering.

Root cause is that effects are passed by-ref (input is AllowedEffects, output is Effects) to two drag handlers in succession in DragDrop.cs, OleDragOver implementation at line 1024 and line 1033.

The simplest fix would be to store the original effect variable before calling the DragLeave handler and restore it before calling the DragEnter handler.

Actual behavior: AllowedEffects in DragEnter handler are wrong if the DragLeave handler was setting DragEventArgs.Effects - in the given example this manifests itself in a flickering cursor when starting a drag from the green area and moving the cursor between red and blue areas. When you have more complex behavior attached to D&D like a thumbnail following the cursor this can get even more obvious.

Expected behavior: AllowedEffects in DragEnter handler should match those specified by the drag originator and be independent of whatever DragLeave handlers returned from their DragEventArgs.Effects

Minimal repro:

<Window x:Class="MyWPFApp.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml">
    <Grid>
        <Grid.RowDefinitions>
            <RowDefinition/>
            <RowDefinition/>
        </Grid.RowDefinitions>
        <Grid.ColumnDefinitions>
            <ColumnDefinition/>
            <ColumnDefinition/>
        </Grid.ColumnDefinitions>
        <Border Grid.Row="0" Grid.ColumnSpan="2"
                MouseLeftButtonDown="StartDrag"
                Background="DarkGreen">
            <TextBlock Foreground="White" Text="drag from here"
                       HorizontalAlignment="Center" VerticalAlignment="Center"/>
        </Border>
        <Border Grid.Row="1" Grid.Column="0" AllowDrop="True"
                DragEnter="CopyDragHandler"
                DragOver="CopyDragHandler"
                DragLeave="CopyDragHandler"
                Drop="CopyDragHandler"
                Background="DarkRed">
            <TextBlock Foreground="White" Text="accepts copy"
                       HorizontalAlignment="Center" VerticalAlignment="Center"/>
        </Border>
        <Border Grid.Row="1" Grid.Column="1" AllowDrop="True"
                DragEnter="LinkDragHandler"
                DragOver="LinkDragHandler"
                DragLeave="LinkDragHandler"
                Drop="LinkDragHandler"
                Background="DarkBlue">
            <TextBlock Foreground="White" Text="accepts link"
                       HorizontalAlignment="Center" VerticalAlignment="Center"/>
        </Border>
    </Grid>
</Window>
using System.Windows;
using System.Windows.Input;

namespace MyWPFApp
{
    public partial class MainWindow : Window
    {
        public MainWindow()
        {
            InitializeComponent();
        }

        private void StartDrag(object sender, MouseButtonEventArgs e)
        {
            DragDrop.DoDragDrop((DependencyObject)sender, "data", DragDropEffects.Copy | DragDropEffects.Link);
        }

        private void CopyDragHandler(object sender, DragEventArgs e)
        {
            e.Effects = e.AllowedEffects & DragDropEffects.Copy;
        }

        private void LinkDragHandler(object sender, DragEventArgs e)
        {
            e.Effects = e.AllowedEffects & DragDropEffects.Link;
        }
    }
}

weltkante avatar Dec 06 '18 21:12 weltkante

Seconded. Times 5 for each colleague that tried to make a workaround for this in past week.

janovrom avatar Jun 12 '20 13:06 janovrom

@weltkante - Taking a look at this issue, I've tested out your proposed simple fix and can confirm that it fixes the cursor flicker reported in the minimal repro.

willibrandon avatar Sep 08 '24 20:09 willibrandon

Here is video demonstrating the flicker using the minimal repro.

https://github.com/user-attachments/assets/df76a3b7-0425-4295-bae2-756874c8d617

willibrandon avatar Sep 09 '24 00:09 willibrandon

I agree this issue is due to a bug in the implementation and the last drop target should not be setting the effects for the new drop target in OleDragOver which is the root cause of the flicker. I would be willing to submit a PR with a proposed fix.

willibrandon avatar Sep 11 '24 08:09 willibrandon

Works for me, the main question is how to cover it in unit testing. Back when I was still actively looking at the repo we were waiting for the test suite getting open sourced, but I don't think it ever made it? Overall it should be a low risk change though, since its pretty clear whats going on.

weltkante avatar Sep 11 '24 09:09 weltkante

I haven't looked closely yet but WPF tests can be found here wpf-test and I see there are 2 drag and drop related test classes here.

It should be possible to cover this in a similar fashion as the DragDropEvents test which according to the doc verifies the drag and drop related events fire correctly. Taking a look at it now.

willibrandon avatar Sep 17 '24 03:09 willibrandon

After some minimal changes I'm able to build and run DragDrop tests.

.\RunTests.cmd /Area=Editing /SubArea=DragDrop

A total of 755 test Infos were processed, with the following results.
 Passed: 2
 Failed (need to analyze): 0
 Failed (with BugIDs): 0
 Ignore: 753

Done.
done with tests
RunTests.cmd command executed

Note that most of DragDrop tests are currently disabled.

willibrandon avatar Sep 17 '24 05:09 willibrandon