Bug Hunting Session
Bug 99870 - EDITING: "Delete Comment" in Comment drop-down menu sometimes deletes the commented text!
Summary: EDITING: "Delete Comment" in Comment drop-down menu sometimes deletes the com...
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
5.1.2.2 release
Hardware: x86-64 (AMD64) All
: medium normal
Assignee: Justin L
URL:
Whiteboard: target:5.3.0 target:5.1.4 target:5.2.0.1
Keywords: bibisected, bisected, regression
Depends on:
Blocks:
 
Reported: 2016-05-16 02:37 UTC by Luke Kendall
Modified: 2016-10-25 18:55 UTC (History)
5 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 Luke Kendall 2016-05-16 02:37:05 UTC
Sometimes (about one time in 5 or 10), when I click on the icon to bring up the menu of commands that can be applied to a comment, and choose Delete, it deletes the text in the body of the document, that is being commented on.

(I thought I had reported this bug months ago, but couldn't find any report of it, so perhaps I overlooked doing so, sorry.)

I've just now experimented and found steps that appear to make the bug (completely?) reproducible:

1.
Select the text in the body of the document which the comment highlights (being careful to select the text and using the extra Shift-(Right-arrow) key to make sure the comment itself is also selected
2.
Without de-selecting the body text, bring up the comment's menu.
3.
Click on the "Delete Comment" menu item.

Result:
Comment *and body text selected* is deleted when you choose "Delete comment".

Expected result:
Just the comment is deleted.  (Ideally, the body text would remain selected.)

Workaround:
4. Undo to undelete the text and comment that was just deleted. Ensure text is deselected. Bring the comment menu back; hit Delete Comment again.

I can't test the reproducibility as much as I'd have liked, because at "step 4" (for working around the problem), usually the comment has been broken in that it no longer highlights the text it used to.

That in fact may be a separate bug?  But if this is fixed, the other bug would not be detectable, I imagine.


This happens very frequently to me since a good workflow when working with someone who has provided comments to documents I'm writing(*) is to copy the commented text and the comment across to the current version of my document. 

Because of bug 60418 it's important to delete the comment from the (copy of the commented-document), so that performance gradually improves when working on the old, commented, document.

Because of this, I find I often have to use the obvious workaround (described above); but each of these operations takes typically 10 secs or so, because of bug 60418.  (Made worse because sometimes a click on the icon to bring up the menu appears to "get lost" before LO wakes up and does something, so you have to click, wait a bit, click again, wait, ....)  So the problem I'm reporting is a substantial waste of time until the number of comments has dropped below a thousand or so.  And it's easy to forget to de-select the body text before bringing up the comment's menu; and especially so because I suspect that mouse-clicks into the body of the document (to de-select the text) also get "lost" sometimes before LO "wakes up".  Probably because it's struggling due to bug 60418.

(*)
Partly due to bug 60418.  But it's also a good workflow if you are taking comments while continuing to work on the document yourself, so you have to work from your own latest up-to-date version.
Comment 1 Luke Kendall 2016-05-16 03:25:05 UTC
I just watched very closely regarding the detail I reported, about LO sometimes ignoring the first click into a document (and leaving the highlighted comment text selected).

It definitely happens, but that itself is hard to reproduce.

I see that when the text that's commented is selected, you can tell because the area gets a reddish colour wash (including a single pixel red outline).  But just now, after copying the text and comment across, and spending a few minutes working in the newer document, when I clicked back into the body of the older document with the comment, the text was still selected; I waited 40 seconds but the mouse click into that document never caused the selection to de-select.  I had to click a second time to make it de-select.

But when I tried to reproduce this small aspect of the behaviour, I couldn't.  I think this is the source of the randomness that made the problem not happen all the time: now I see that usually, the selection in one document is de-selected when you change focus to a 2nd document.  But occasionally, it doesn't de-select: it remains selected in the 1st doc.
Comment 2 Buovjaga 2016-05-18 07:09:34 UTC
(In reply to Luke Kendall from comment #0)
> 1.
> Select the text in the body of the document which the comment highlights
> (being careful to select the text and using the extra Shift-(Right-arrow)
> key to make sure the comment itself is also selected
> 2.
> Without de-selecting the body text, bring up the comment's menu.
> 3.
> Click on the "Delete Comment" menu item.
> 
> Result:
> Comment *and body text selected* is deleted when you choose "Delete comment".

Repro.

No problem in 5.0.2.2

Luke: you might want to make a habit of testing with older versions :) https://wiki.documentfoundation.org/Installing_in_parallel/Linux

Win 7 Pro 64-bit Version: 5.2.0.0.alpha1+
Build ID: f688acfdae00ebdd891737e533d54368810185e1
CPU Threads: 4; OS Version: Windows 6.1; UI Render: default; 
TinderBox: Win-x86@62-merge-TDF, Branch:MASTER, Time: 2016-05-18_00:11:31
Locale: fi-FI (fi_FI)
Comment 3 Luke Kendall 2016-05-18 08:54:21 UTC
I'd do more if I had time, but I'm absolutely flat out, trying to get this 2nd book ready for the series launch on July 9th!
Comment 4 raal 2016-05-19 06:01:25 UTC
d9f4bb0bec4f5c5b8485a6ba3b693f15480d2f08 is the first bad commit
commit d9f4bb0bec4f5c5b8485a6ba3b693f15480d2f08
Author: Norbert Thiebaud <nthiebaud@gmail.com>
Date:   Fri Oct 9 19:25:20 2015 -0700

    source sha:fb62052d5ac069d700a5410db35d6949a4c4008b
	
author	Justin Luth <justin_luth@sil.org>	2015-10-03 06:35:26 (GMT)
committer	Caolán McNamara <caolanm@redhat.com>	2015-10-09 13:57:18 (GMT)
commit	fb62052d5ac069d700a5410db35d6949a4c4008b (patch)
tree	c1196bde7be30eb500296e3a475b2ea7a9766712
parent	4850edd31afa16401ac7c94d8bd1b02525fbe718 (diff)
tdf#94679 Writer: fix lost selection with Shift-PageDown
Comment 5 Justin L 2016-05-20 16:42:44 UTC
Pushing and popping the SwWrtShell does MORE than just save and restore the current state. This bug's "delete comment" action DEPENDS on the extra side-effects of the pop routine to function, specifically
m_fnKillSel = &SwWrtShell::ResetSelect

proposed fix at https://gerrit.libreoffice.org/25216
Comment 6 Luke Kendall 2016-05-24 06:15:09 UTC
In case it helps, I just discovered a related odd behaviour.

I had two comments, in nearby sections of text.  I had copied the 2nd block of text + comment to the other doc. Then I went back to the 1st doc, and before I noticed that the 2nd block of text was still selected, I chose Delete Comment from the menu of the 1st comment. 
In this case, LO deleted all that 1st comment's text (*not* selected), and all the following text, up to the start of the selected text of the 2nd comment!
Comment 7 Justin L 2016-05-28 18:57:00 UTC
I bibisected this bug further back. LO 3.5 also deletes the hand-selected characters along with the comment (but not with mouse selection - same as present situation.)  Bibisect-43all indicates this was fixed between Mar 1 2013 (commit 5da10275a7475efdbfd9de14ea58cf8f4c6c1582) and Jan 23 2013 (commit ba446dd58a4ad324d242afcd5b28d3b4dff5a881).
However, at that point the comment itself is not being deleted, and the selection is de-selected.

In bibisect-43all, the deleted selection bug shows up again starting between Jan 10 2014 (commit ed6a2f3bb94b4ffdb2fe95e167fcb0ce38a4cb58) and Jan 9 2014 (commit 4356aef48a8fcbd9dd019c0ca2d6a189d7332d0c) and remains until last43onmaster (12.04/16.04).  However, I cannot replicate this in bibisect-43max where it works perfectly (16.04).

Previously, the selection has always been cleared (de-selected) when the comment is deleted.  I'm reworking my fix and trying to figure out how to handle the selection part of it.
Comment 8 Commit Notification 2016-05-30 05:33:30 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=12868914a39b0e8fb32f63d1307c1bb1c535fb9c

tdf#99870 writer: don't delete selection with delete-comment

It will be available in 5.3.0.

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

Affected users are encouraged to test the fix and report feedback.
Comment 9 Luke Kendall 2016-05-30 05:44:27 UTC
Thanks, Justin!
Comment 10 Commit Notification 2016-05-30 08:43:02 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-5-1":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=2a5fe37dfc48bfddc4fd4e47c1034fd76d53ba84&h=libreoffice-5-1

tdf#99870 writer: don't delete selection with delete-comment

It will be available in 5.1.4.

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

Affected users are encouraged to test the fix and report feedback.
Comment 11 Commit Notification 2016-05-30 09:36:01 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-5-2":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=8038cab377a5ac726cf38f5f95bc987063b95de5&h=libreoffice-5-2

tdf#99870 writer: don't delete selection with delete-comment

It will be available in 5.2.0.1.

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

Affected users are encouraged to test the fix and report feedback.
Comment 12 Daniël van Vuuren 2016-06-03 10:16:47 UTC
Verified fix in:

Version: 5.3.0.0.alpha0+
Build ID: 6b3b352b06d92ef20194b9a992a521af2ef07b48
CPU Threads: 2; OS Version: Linux 4.4; UI Render: default; 
TinderBox: Linux-rpm_deb-x86@71-TDF, Branch:master, Time: 2016-06-03_01:35:48

Version: 5.2.0.0.beta1+
Build ID: b6230835b927e0053687fae6026fa3603600f321
CPU Threads: 2; OS Version: Linux 4.4; UI Render: default; 
TinderBox: Linux-rpm_deb-x86@71-TDF, Branch:libreoffice-5-2, Time: 2016-06-03_02:09:38

Version: 5.1.5.0.0+
Build ID: 1245bead3a68c9495a870f194f3c523b3b78cf87
CPU Threads: 2; OS Version: Linux 4.4; UI Render: default; 
TinderBox: Linux-rpm_deb-x86@71-TDF, Branch:libreoffice-5-1, Time: 2016-06-02_04:43:39