Bug 126560 - KDE-integration (Plasma 5): shifting/inserting rows with Alt+Shift not working correctly
Summary: KDE-integration (Plasma 5): shifting/inserting rows with Alt+Shift not workin...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
6.2.5.2 release
Hardware: All Linux (All)
: medium normal
Assignee: Jan-Marek Glogowski
URL:
Whiteboard: target:6.4.0 target:6.3.2
Keywords:
Depends on:
Blocks: KDE, KF5
  Show dependency treegraph
 
Reported: 2019-07-26 07:52 UTC by Rainer Fiebig
Modified: 2019-08-23 05:08 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Rainer Fiebig 2019-07-26 07:52:59 UTC
Hi!

With KDE-integration active, shifting and inserting rows by moving selected rows with the mouse while pressing Alt+Shift does not work, i.e. existing content is overwritten instead of being shifted down as expected.

Without KDE-integration behavior is OK.

Thanks.

__

Version: 6.2.5.2
Build-ID: 1ec314fa52f458adc18c4f025c545a4e8b22c159

Qt Version: 5.10.1
Frameworks: 5.46.0
Plasma version: 5.12.8
Comment 1 Roman Kuznetsov 2019-07-27 09:28:37 UTC
Please add full info from Help->About dialog here.
Comment 2 Rainer Fiebig 2019-07-28 06:55:11 UTC
(In reply to Roman Kuznetsov from comment #1)
> Please add full info from Help->About dialog here.

I guess that is what you want:

Version: 6.2.5.2
Build-ID: 1ec314fa52f458adc18c4f025c545a4e8b22c159
CPU-Threads: 4; BS: Linux 4.19; UI-Render: Standard; VCL: kde5; 
Gebietsschema: de-DE (de_DE.UTF-8); UI-Sprache: de-DE
Calc: threaded
Comment 3 Jan-Marek Glogowski 2019-07-30 14:55:03 UTC
There is definitely some problem with D'n'D handling inside LO with kde5, also with Writer. Normally it just works, if you press the modifier before starting the drag operation, which is really annoying. Also the cursor doesn't adapt, but that also seems to be the problem inside KDE apps, like kate.
Comment 4 Jan-Marek Glogowski 2019-08-06 09:02:20 UTC
BTW: I find it easier to test this in Writer with copying / moving selected text in a document.
Comment 5 Rainer Fiebig 2019-08-06 10:14:56 UTC
(In reply to Jan-Marek Glogowski from comment #4)
> BTW: I find it easier to test this in Writer with copying / moving selected
> text in a document.


You seem to be talking of a different problem. The one that I'm having here is exactly as described above. And if you can't reproduce it, you don't have it. Maybe only certain QT- or Frameworks-versions are affected.

I can confirm however that the mouse pointer doesn't change when selecting and then moving text with the mouse in Writer which is somewhat irritating. But this also occurs without KDE-integration.
Comment 6 Rainer Fiebig 2019-08-07 08:09:13 UTC
(In reply to Jan-Marek Glogowski from comment #4)
> BTW: I find it easier to test this in Writer with copying / moving selected
> text in a document.

I have checked the behaviour of selecting and then moving text with the mouse in Writer (6.0 / 6.2) in xfce. And it's the same as in KDE. So this is a generic problem and *not* KDE-specific. And if it has not yet been reported, it's imo worth a new bug-report - but I leave that to you. ;)

When one selects text in MS Word (with the mouse or keyboard), the mouse pointer immediately changes to an arrow when it is over the selected text. And this is how it should be.
Comment 7 Jan-Marek Glogowski 2019-08-08 09:23:47 UTC
(In reply to Rainer Fiebig from comment #5)
> (In reply to Jan-Marek Glogowski from comment #4)
> > BTW: I find it easier to test this in Writer with copying / moving selected
> > text in a document.
> 
> You seem to be talking of a different problem. The one that I'm having here
> is exactly as described above. And if you can't reproduce it, you don't have
> it. Maybe only certain QT- or Frameworks-versions are affected.

I can reproduce this problem. As I wrote I find it easier to Drag and Drop (DnD) text inside Writer, nothing else. From the source codes POV both are the same. Broken DnD handling.

While I already have a fix for the drop target handling implemented, there seems to be no way to fix the visual cursor feedback for DnD. I already found that I don't get any Qt events, if I just press a modifier button while in DnD. I tested a key handlers, the DnD handers, the "ActionChanged" event of QDrag; nothing happens. And then I found this open Qt bug: https://bugreports.qt.io/browse/QTBUG-56218 . It was opened in 2017-09 for Qt 5.6 :-( The last comments claim the modifier handling is fixed in Qt 5.12, but it's still open.

> I can confirm however that the mouse pointer doesn't change when selecting
> and then moving text with the mouse in Writer which is somewhat irritating.
> But this also occurs without KDE-integration.

That is quite probably a completely different problem, but I'm not so sure. QDrag can set a different icon / cursor per drag action: https://doc.qt.io/qt-5/qdrag.html#setDragCursor.

Still my cursor changes for the gtk3 and gen backend, if I press Ctrl. And if I press the Alt key, the border of the selected cells changes in Calc to indicate the move, if I move the dragged cells around. "Fun" fact that I haven't found any code handling Alt in DnD handlers.

From my current POV the qt5 code does the same then the gtk+ code, but the latter works and my code doesn't, so there is still something missing. Maybe it's the special modifiers LO key event, which isn't yet implemented in qt5 but only in gtk+. Never saw it called yet.
Comment 8 Jan-Marek Glogowski 2019-08-08 09:25:53 UTC
And I tested the general event handler, which also shows no event on modifier press. In theory this all would require a Qt version bump to 5.12, if that bug is really just fixed there :-(
Comment 9 Rainer Fiebig 2019-08-08 10:39:13 UTC
(In reply to Jan-Marek Glogowski from comment #7)
> (In reply to Rainer Fiebig from comment #5)
> > (In reply to Jan-Marek Glogowski from comment #4)
> > > BTW: I find it easier to test this in Writer with copying / moving selected
> > > text in a document.
> > 
> > You seem to be talking of a different problem. The one that I'm having here
> > is exactly as described above. And if you can't reproduce it, you don't have
> > it. Maybe only certain QT- or Frameworks-versions are affected.
> 
> I can reproduce this problem. As I wrote I find it easier to Drag and Drop

Ah - always reassuring to not be the only one being hit. I'm a bit surprised though that not more users are reporting this.

> (DnD) text inside Writer, nothing else. From the source codes POV both are
> the same. Broken DnD handling.
> 
> While I already have a fix for the drop target handling implemented, there
> seems to be no way to fix the visual cursor feedback for DnD. I already
> found that I don't get any Qt events, if I just press a modifier button
> while in DnD. I tested a key handlers, the DnD handers, the "ActionChanged"
> event of QDrag; nothing happens. And then I found this open Qt bug:
> https://bugreports.qt.io/browse/QTBUG-56218 . It was opened in 2017-09 for
> Qt 5.6 :-( The last comments claim the modifier handling is fixed in Qt
> 5.12, but it's still open.
> 
> > I can confirm however that the mouse pointer doesn't change when selecting
> > and then moving text with the mouse in Writer which is somewhat irritating.
> > But this also occurs without KDE-integration.
> 
> That is quite probably a completely different problem, but I'm not so sure.
> QDrag can set a different icon / cursor per drag action:
> https://doc.qt.io/qt-5/qdrag.html#setDragCursor.

In my view this probably is an LO-problem because I see exactly the same behaviour in a GTK-environment (xfce).

In Kate and Kwrite (18.12.2), the pointer does change to an arrow.

> 
> Still my cursor changes for the gtk3 and gen backend, if I press Ctrl. And
> if I press the Alt key, the border of the selected cells changes in Calc to
> indicate the move, if I move the dragged cells around. "Fun" fact that I
> haven't found any code handling Alt in DnD handlers.
> 
> From my current POV the qt5 code does the same then the gtk+ code, but the
> latter works and my code doesn't, so there is still something missing. Maybe
> it's the special modifiers LO key event, which isn't yet implemented in qt5
> but only in gtk+. Never saw it called yet.
Comment 10 Rainer Fiebig 2019-08-08 10:47:22 UTC
(In reply to Jan-Marek Glogowski from comment #8)
> And I tested the general event handler, which also shows no event on
> modifier press. In theory this all would require a Qt version bump to 5.12,
> if that bug is really just fixed there :-(

Why should it be? If it hasn't been fixed up to 5.10 (!)... 
So, I wouldn't hope for too much. ;)
Comment 11 Rainer Fiebig 2019-08-09 10:09:21 UTC
> While I already have a fix for the drop target handling implemented, there

So the initially reported problem is fixed? That would be great!

> seems to be no way to fix the visual cursor feedback for DnD. I already

This is definitely not KDE 5 specific. I see this with the gtk-backend, within a gtk-environment (xfce) and with with LO-3.5.4.2 (and probably earlier) in KDE 4. It seems to be a long existing glitch in LO which might perhaps better be tackled separately.
Comment 12 Jan-Marek Glogowski 2019-08-09 15:44:11 UTC
Tentative patch: https://gerrit.libreoffice.org/#/c/77174

This was a much bigger problem then I originally expected. Not really unit-testable IMHO, so testers would really be welcome to report bugs, either here or in Gerrit.
Comment 13 Jan-Marek Glogowski 2019-08-21 13:32:03 UTC
Almost two weeks later (I didn't spend full time on it ;-) I have something, that passes various manual tests here. Normally the following tests should work in both directions with LOs own windows and at least twice. We had some bugs, where dragging just worked once, because of the wrong internal drag state in the qt VCL plugin.

* DnD text between Writer windows => copy
* DnD cells between Calc windows => move
* DnD of slides between Impress windows => copy
* DnD of gallery graphics to an Impress slide => copy
* Dragging to an "error" position correctly => aborts
* Dropping to an external text editor => copy
* Dragging text from an external text editor => copy
* Dropping a file from a file manager => opens the file

There are four different possible cursors, best visible in Calc:
* A hand, indicating a move (and replace) action (default)
* A plus sign, indicating a copy action (Ctrl)
* An arrow, indicating a link action (Ctrl+Shift)
* An veto / error sign, indicating a prohibited drop area

There is an additional indicator in Calc, when you press Alt, which doesn't change the cursor (hand), but the selected rectangle, to indicate a Move + Insert action, instead of replace.

Would be nice to get some additional scenarios, if you think something in general is still missing and should be tested. Otherwise this will be merged (probably tomorrow) and a backport provided for 6.3.
Comment 14 Rainer Fiebig 2019-08-21 16:39:35 UTC
(In reply to Jan-Marek Glogowski from comment #13)
> Almost two weeks later (I didn't spend full time on it ;-) I have something,
> that passes various manual tests here. Normally the following tests should
> work in both directions with LOs own windows and at least twice. We had some
> bugs, where dragging just worked once, because of the wrong internal drag
> state in the qt VCL plugin.
> 
> * DnD text between Writer windows => copy
> * DnD cells between Calc windows => move
> * DnD of slides between Impress windows => copy
> * DnD of gallery graphics to an Impress slide => copy
> * Dragging to an "error" position correctly => aborts
> * Dropping to an external text editor => copy
> * Dragging text from an external text editor => copy
> * Dropping a file from a file manager => opens the file
> 
> There are four different possible cursors, best visible in Calc:
> * A hand, indicating a move (and replace) action (default)
> * A plus sign, indicating a copy action (Ctrl)
> * An arrow, indicating a link action (Ctrl+Shift)
> * An veto / error sign, indicating a prohibited drop area
> 
> There is an additional indicator in Calc, when you press Alt, which doesn't
> change the cursor (hand), but the selected rectangle, to indicate a Move +
> Insert action, instead of replace.
> 
> Would be nice to get some additional scenarios, if you think something in
> general is still missing and should be tested. Otherwise this will be merged
> (probably tomorrow) and a backport provided for 6.3.

Great, this looks good and rather complete - nothing to add from my side. So just go ahead. 

Looking forward for 6.3.1! :)

Thanks for your time and effort!
Comment 15 Commit Notification 2019-08-22 12:29:44 UTC
Jan-Marek Glogowski committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/3355be0616c24c5e44b71e7623c4191ed9c69074%5E%21

tdf#126560 Qt5 fix D'n'D key-modifier handling

It will be available in 6.4.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 16 Commit Notification 2019-08-22 14:39:34 UTC
Jan-Marek Glogowski committed a patch related to this issue.
It has been pushed to "libreoffice-6-3":

https://git.libreoffice.org/core/+/658ab1db3abce8941352d8d9def1468496985a60%5E%21

tdf#126560 Qt5 fix D'n'D key-modifier handling

It will be available in 6.3.2.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 17 Rainer Fiebig 2019-08-22 17:07:54 UTC
(In reply to Commit Notification from comment #16)
> Jan-Marek Glogowski committed a patch related to this issue.
> It has been pushed to "libreoffice-6-3":
> 
> https://git.libreoffice.org/core/+/
> 658ab1db3abce8941352d8d9def1468496985a60%5E%21
> 
> tdf#126560 Qt5 fix D'n'D key-modifier handling
> 
> It will be available in 6.3.2.
> 
6.3.2 is much better than 6.4. And if it proves OK it should also be backported to 6.2, imo. It's a bugfix after all, isn't it? ;)
Comment 18 Jan-Marek Glogowski 2019-08-22 18:36:48 UTC
(In reply to Rainer Fiebig from comment #17)
> (In reply to Commit Notification from comment #16)
> > Jan-Marek Glogowski committed a patch related to this issue.
> > It has been pushed to "libreoffice-6-3":
> > 
> > https://git.libreoffice.org/core/+/
> > 658ab1db3abce8941352d8d9def1468496985a60%5E%21
> > 
> > tdf#126560 Qt5 fix D'n'D key-modifier handling
> > 
> > It will be available in 6.3.2.
> > 
> 6.3.2 is much better than 6.4. And if it proves OK it should also be
> backported to 6.2, imo. It's a bugfix after all, isn't it? ;)

No chance for 6.2, really.

This code depends on the fix for for bug 122239, which rewrote the whole clipboard and external mime type handling (+526, -352 diff) and needed 20 revisions to get "right" and still needed two follow up patches. And this fix itself is "10 files changed, 174 insertions(+), 158 deletions(-)", which needed 10 revisions. And all this depends on additional larger fixes, like the Qt5Transferable refactoring.

Basically we (as in the people handling the qt5 / kf5 backend of LO) decided, that we would just backport critical stuff after 6.2.5. This bug is annoying, but there are enough ways to workaround it, eventually even using a gtk3 based backend and deinstalling the kde integration package.
Comment 19 Rainer Fiebig 2019-08-23 05:08:34 UTC
(In reply to Jan-Marek Glogowski from comment #18)
> (In reply to Rainer Fiebig from comment #17)
> > (In reply to Commit Notification from comment #16)
> > > Jan-Marek Glogowski committed a patch related to this issue.
> > > It has been pushed to "libreoffice-6-3":
> > > 
> > > https://git.libreoffice.org/core/+/
> > > 658ab1db3abce8941352d8d9def1468496985a60%5E%21
> > > 
> > > tdf#126560 Qt5 fix D'n'D key-modifier handling
> > > 
> > > It will be available in 6.3.2.
> > > 
> > 6.3.2 is much better than 6.4. And if it proves OK it should also be
> > backported to 6.2, imo. It's a bugfix after all, isn't it? ;)
> 
> No chance for 6.2, really.
> 
> This code depends on the fix for for bug 122239, which rewrote the whole
> clipboard and external mime type handling (+526, -352 diff) and needed 20
> revisions to get "right" and still needed two follow up patches. And this
> fix itself is "10 files changed, 174 insertions(+), 158 deletions(-)", which
> needed 10 revisions. And all this depends on additional larger fixes, like
> the Qt5Transferable refactoring.
> 
> Basically we (as in the people handling the qt5 / kf5 backend of LO)
> decided, that we would just backport critical stuff after 6.2.5. This bug is
> annoying, but there are enough ways to workaround it, eventually even using
> a gtk3 based backend and deinstalling the kde integration package.

Valid arguments, 6.3 then. Although 6.2 is positioned as the more mature (stable) one by LO. ;)

Anyway - thanks for fixing this obviously complex problem quickly. Have a good weekend!