[BR] Fix DragAndDropImage to prevent looking for targets outside its container

I have a plug-in roughly structured like so:

the user can drag effects from a toolbox component on the right (grey), onto a main viewport in the center (also grey), which is therefore a DragAndDropTarget. toolbox and viewport are siblings so they are enclosed in a larger parent component (in this case the plug-in editor itself) which serves as the DragAndDropContainer for the dragging between the two.

On the left, I have an unrelated list (yellow) which shows a list of files whose order can be changed also by drag and drop. For that to happen, the yellow list itself is a DragAndDropTarget and it is a child of a DragAndDropContainer of its own (fainter yellow).

The intention of this is to keep the drag and drop operations pertaining to the list of files, contained between the list itself and the container that is its parent, and not creep them to the grey components that know nothing about it.

Unfortunately, the moment I inadvertently drag a file from the list over the viewport, I get an assert in the isInterestedInDragSource() from the viewport, which is (correctly) not recognizing the source that is being dragged on it (because it only expects stuff being dragged from the grey toolbox of effects).

This violates the doc for DragAndDropContainer, which says:

to start a drag operation, any sub-component can just call the startDragging() method, and this object will take over, tracking the mouse and sending appropriate callbacks to any child components derived from DragAndDropTarget which the mouse moves over.

The grey viewport in my case is not a child of the yellow DragAndDropContainer and its isInterestedInDragSource() callback should never be called for a drag operation that started from the yellow container.

If the change to fix this is too breaking, at least make the correct behaviour available as an opt-it, thanks.

EDIT: I believe the right place where to fix the algorithm is in DragImageComponent::findTarget()

My implementation of DragImageComponent::findTarget() that restores the documented contract is displayed below (the code I have added is between “yfede” and “/yfede” comments)

    std::tuple<DragAndDropTarget*, Component*, Point<int>> findTarget (Point<int> screenPos) const
    {
        auto* hit = getParentComponent();

        if (hit == nullptr)
            hit = findDesktopComponentBelow (screenPos);
        else
            hit = hit->getComponentAt (hit->getLocalPoint (nullptr, screenPos));

        // (note: use a local copy of this in case the callback runs
        // a modal loop and deletes this object before the method completes)
        auto details = sourceDetails;

        // yfede
        auto* const ownerComponent = dynamic_cast <Component*> (&owner);
        if (ownerComponent == nullptr)
        {
            jassertfalse;   // the owning DragAndDropContainer must also be a Component
            return {};
        }
        // /yfede

        while (hit != nullptr)
        {
            // yfede
            if (! ownerComponent->isParentOf (hit))
                break;
            // /yfede

            if (auto* ddt = dynamic_cast<DragAndDropTarget*> (hit))
                if (ddt->isInterestedInDragSource (details))
                    return std::tuple (ddt, hit, hit->getLocalPoint (nullptr, screenPos));

            hit = hit->getParentComponent();
        }

        return {};
    }

Is this even related to inheritance? The original Code reads a bit like side by side drag and drop containers would also suffer from this bug. That would IMO greatly increase the urgency of a fix.

Yes, exactly: with two adjacent DragAndDropContainers A and B that don’t even overlap, each containing their DragAndDropTargets as children, you can start a drag from container A, drag the cursor over a target in B, and that target will receive callbacks (that it shouldn’t).
Callbacks in a target contained in B, should only be called if the drag had originated from container B, which is a parent of it.

1 Like

Maybe there is a design pattern/usage we are missing here. I find It hard to imagine this is the first time someone stumbles about multiple Independent DnDs going on in the same application.

The startDragging() method has the bool flag “allowDraggingToOtherJuceWindows”. Maybe this could be an enum or BitFlag:

allowOtherDragAndDropContainers   // currently missing
allowDraggingToOtherJuceWindows   // settable in startDrag
allowDraggingToOtherApplications  // i.e. performExternalDragDropOfText
2 Likes

Thank you for the clear, detailed explanation. I agree this behaviour is not ideal, and we’ll track this bug.

To help understand the severity: you mention an

assert in the isInterestedInDragSource() from the viewport

Is this an assert coming from the JUCE framework? Or is this something you would be able to avoid without modifying the framework source code?

This is not an assert in the JUCE code.
It is an assert in my code, in the implementation of the overridden isInterestedInDragSource() in my Component that inherits from DragAndDropTarget.

The purpose of that assert was to detect whether handling of some possible drag sources was missing in the code.
I can certainly comment it out until a fix is in place. Sorry for the confusion.