Thanks for the patch :) From a behavioural standpoint it works very well, and you seem to have covered a lot of edge cases.

Nevertheless, I have a couple of points.

I'm not able to compile all of Gwenview:

part/gvpart.cpp: In constructor ‘Gwenview::GVPart::GVPart(QWidget*, QObject*, const QVariantList&)’:
part/gvpart.cpp:64:78: error: no matching function for call to ‘Gwenview::DocumentViewContainer::DocumentViewContainer(QWidget*&)’
     DocumentViewContainer* container = new DocumentViewContainer(parentWidget);
                                                                              ^

Note that this might indicate a larger problem with your patch, i.e. ViewMainPage might not be the best place to implement drop handling. Using Gwenview's KPart in Konqueror should also support dropping of URLs (at least in theory, not sure if this would need more work, so likely out-of-scope for this patch).

I don't see yet why you'd need to use an eventFilter and add a dependency on ViewMainPage. Normally reimplementing the corresponding event like it's done in ThumbnailView::dropEvent should be enough.

Qt's docs advice

To handle the incoming drag, reimplement QGraphicsItem::dragEnterEvent()

Perhaps AbstractImageView would be a good candidate, because it already handles key and mouse events, and is (indirectly) derived from QGraphicsItem. If you don't have direct access to openDirUrl there, perhaps some signal/slot magic will work then (e.g. see previousImageRequested)?

When trying to drop text dragged from Kate, Gwenview aborts:

ASSERT: "!isEmpty()" in file /usr/include/qt5/QtCore/qlist.h, line 345
Aborted (core dumped)

It should be enough to reject unsupported mimetypes in the first place, just like Browse is already showing the "cannot drop here" cursor in that case.

In D11879#265080, @rkflx wrote:

Thanks for the patch :) From a behavioural standpoint it works very well, and you seem to have covered a lot of edge cases.

Nevertheless, I have a couple of points.

---

I'm not able to compile all of Gwenview:

part/gvpart.cpp: In constructor ‘Gwenview::GVPart::GVPart(QWidget*, QObject*, const QVariantList&)’:
part/gvpart.cpp:64:78: error: no matching function for call to ‘Gwenview::DocumentViewContainer::DocumentViewContainer(QWidget*&)’
     DocumentViewContainer* container = new DocumentViewContainer(parentWidget);
                                                                              ^

Note that this might indicate a larger problem with your patch, i.e. ViewMainPage might not be the best place to implement drop handling. Using Gwenview's KPart in Konqueror should also support dropping of URLs (at least in theory, not sure if this would need more work, so likely out-of-scope for this patch).

I don't see yet why you'd need to use an eventFilter and add a dependency on ViewMainPage. Normally reimplementing the corresponding event like it's done in ThumbnailView::dropEvent should be enough.

Qt's docs advice

To handle the incoming drag, reimplement QGraphicsItem::dragEnterEvent()

I definitely tried this initially, but it didn't work. My memory fails me as to the specifics, but the only way I could get it to work was by handling the event at the QGraphicsScene level (which is in DocumentViewContainer).
I'll definitely have another look at it though.

Perhaps AbstractImageView would be a good candidate, because it already handles key and mouse events, and is (indirectly) derived from QGraphicsItem. If you don't have direct access to openDirUrl there, perhaps some signal/slot magic will work then (e.g. see previousImageRequested)?

AbstractImageView might not work, but I'll aim for that if possible. As I said, my memory fails me, but I think the events weren't getting that far down in the hierarchy. Perhaps DocumentViewContainer would be a good alternative.

When trying to drop text dragged from Kate, Gwenview aborts:

ASSERT: "!isEmpty()" in file /usr/include/qt5/QtCore/qlist.h, line 345
Aborted (core dumped)

It should be enough to reject unsupported mimetypes in the first place, just like Browse is already showing the "cannot drop here" cursor in that case.

Will investigate.

TODO:

• Move code as close to AbstractImageView as possible, use signals/slots to get back to MainWindow
• Fix dropping unsupported mimetypes e.g. text
• Fix compile error