Bug 132477 - Writer Navigator crashes on drag and drop action in Headings content navigation view
Summary: Writer Navigator crashes on drag and drop action in Headings content navigati...
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
7.0.0.0.alpha0+
Hardware: All All
: medium normal
Assignee: Caolán McNamara
URL:
Whiteboard: target:7.0.0
Keywords:
Depends on:
Blocks:
 
Reported: 2020-04-27 21:44 UTC by Jim Raykowski
Modified: 2020-05-08 15:35 UTC (History)
0 users

See Also:
Crash report or crash signature:


Attachments
non gtk3 headings drag crash (1.03 MB, video/x-matroska)
2020-05-03 08:16 UTC, Jim Raykowski
Details
gtk3 root drag changes pointer (215.95 KB, video/x-matroska)
2020-05-03 08:17 UTC, Jim Raykowski
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jim Raykowski 2020-04-27 21:44:59 UTC
Here are the repro steps using current master version 7.0.0.0alpha+

1) Open or create a Writer document with a headings outline structure.
2) Open the Navigator - F5 version or the one in the Sidebar (Menu->View->Sidebar and then click on the Navigator tab button in the tab bar on the left)
3) If not already selected, select the Headings entry or any of the headings listed below the Headings entry.
4) Click on the Content Navigation View button in the tool box above the content view. (first button on the second line in the tool box)
5) Click and hold on the Headings root entry and drag to any position in the tree.

Result: Crash for the following backends tested

Version: 7.0.0.0.alpha0+
Build ID: 5cff6410222c04f47661ca07a1bbe6e6337633be
CPU threads: 4; OS: Linux 5.0; UI render: default; VCL: gtk3, qt5, kf5, x11; 
Locale: en-US (en_US.UTF-8); UI-Language: en-US
Calc: threaded

Additional Results:
For kf5 and x11 crash happens for any headings entry drag.
Comment 1 Caolán McNamara 2020-05-01 16:27:37 UTC
I'll reuse this for the failure of the dnd cancel to do the right thing
Comment 2 Commit Notification 2020-05-02 18:18:26 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/ce019a60c0a348eb08453dc159e2805ffe2ba188

Related: tdf#132477 use GDK_KEY_Escape to cancel dnd

It will be available in 7.0.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 3 Commit Notification 2020-05-02 18:26:50 UTC
Jim Raykowski committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/9ba0df5d3ccb71acd912507ee0e9e4f3ab4a9b12

Resolves: tdf#132477 Fix Writer Navigator drag and drop crash

It will be available in 7.0.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 4 Jim Raykowski 2020-05-03 08:13:55 UTC
I confirm root drag crash does not happen for any backend. gtk3 under X still acts like it will drag and sometimes the pointer changes to a hand or + resulting in no chance for use after.

Crash still occurs for backends other than gtk3 when dragging non root headings.

warn:legacy.tools:29949:29949:vcl/source/treelist/treelist.cxx:1387: Entry not in model or wrong view

This message seems provide a clue for crash.
Comment 5 Jim Raykowski 2020-05-03 08:16:16 UTC
Created attachment 160265 [details]
non gtk3 headings drag crash

When applied, changes made in Patch Set 1 to 
void SvImpLBox::PaintDDCursor(SvTreeListEntry* pEntry, bool bShow) and SvViewDataEntry* SvListView::GetViewData( SvTreeListEntry* pEntry ) seem to prevent this crash
Comment 6 Jim Raykowski 2020-05-03 08:17:03 UTC
Created attachment 160266 [details]
gtk3 root drag changes pointer
Comment 7 Caolán McNamara 2020-05-07 09:02:25 UTC
for #5 we probably need to unset the drop highlight before we change the contents of the tree so it doesn't try and unset the highlight on a removed/replaced row.
Comment 8 Caolán McNamara 2020-05-07 10:37:43 UTC
wrt #6 do you have a similar problem if you drag some other entry which is allowed to be dragged, and hit ESC to cancel the drag ? or does that work ok
Comment 9 Commit Notification 2020-05-07 13:21:07 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/fd1a4fddad5932c514199de0f8a7b5a2f82ce437

tdf#132477 don't leave removed row highlighted

It will be available in 7.0.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 10 Jim Raykowski 2020-05-07 22:23:49 UTC
(In reply to Caolán McNamara from comment #7)
> for #5 we probably need to unset the drop highlight before we change the
> contents of the tree so it doesn't try and unset the highlight on a
> removed/replaced row.

I agree and confirm that patch in #9 fixes this in a better way than my effort.

(In reply to Caolán McNamara from comment #8)
>wrt #6 do you have a similar problem if you drag some other entry which is >allowed to be dragged, and hit ESC to cancel the drag ? or does that work ok

Problem is only for the root entry. All non root entry drags seem to work correctly. I didn't know about the ESC to cancel. For me this works for all entry drags other than the root. Doesn't change the pointer back to an arrow when root drag attempts changes it to a hand or plus with arrow pointer. No chance for mouse use after this happens.
Comment 11 Commit Notification 2020-05-08 08:10:59 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/ea6a978d873c5320d3cc2526f97636f815efde06

Resolves: tdf#132477 use "cancel" on GdkDragContext

It will be available in 7.0.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 12 Caolán McNamara 2020-05-08 08:16:58 UTC
With the ESC-press test my thought was if you dragged a non-root entry and then pressed ESC did the drag cancel correctly and everything go back to normal, cause that was what my earlier effort tried to emulate as a replacement for gtk_drag_cancel on the root element when it turned out that gtk_drag_cancel didn't work under X like I thought it did.

But turns out that wasn't working either, so we can forget about that. This latest effort with a "cancel" signal on the drag context seems to work reliably under X and wayland for me, I hope the same is true on your end ?
Comment 13 Jim Raykowski 2020-05-08 09:34:17 UTC
(In reply to Caolán McNamara from comment #12)
> With the ESC-press test my thought was if you dragged a non-root entry and
> then pressed ESC did the drag cancel correctly and everything go back to
> normal, cause that was what my earlier effort tried to emulate as a
> replacement for gtk_drag_cancel on the root element when it turned out that
> gtk_drag_cancel didn't work under X like I thought it did.

I follow your thoughts on the ESC emulation try. Yes, for me, ESC cancels dragging for non-root entries.

> 
> But turns out that wasn't working either, so we can forget about that. This
> latest effort with a "cancel" signal on the drag context seems to work
> reliably under X and wayland for me, I hope the same is true on your end ?

I confirm using g_signal_emit_by_name to cancel the drag works on my end.

Thanks for fixing!
Comment 14 Caolán McNamara 2020-05-08 13:53:20 UTC
excellent, esp. cause the same drag cancel thing is used in a bunch of places.