Bug 106746 - copy/pasting revisions copy deleted words
Summary: copy/pasting revisions copy deleted words
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
5.1.0.3 release
Hardware: All All
: high normal
Assignee: Aron Budea
URL:
Whiteboard: target:6.1.0 target:5.4.7 target:6.0.4
Keywords: bibisected, bisected, regression
: 116199 (view as bug list)
Depends on:
Blocks: Track-Changes Paste
  Show dependency treegraph
 
Reported: 2017-03-24 12:09 UTC by Frederic Parrenin
Modified: 2018-04-08 10:50 UTC (History)
7 users (show)

See Also:
Crash report or crash signature:


Attachments
.docx file to reproduce the problem (16.13 KB, application/vnd.openxmlformats-officedocument.wordprocessingml.document)
2017-03-24 12:09 UTC, Frederic Parrenin
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Frederic Parrenin 2017-03-24 12:09:01 UTC
Created attachment 132126 [details]
.docx file to reproduce the problem

Steps to reproduce:
- open the attached .docx file
- copy the entire document
- paste it at the end
=> the words that were deleted in the revisions are copied and they should not
Comment 1 m.a.riosv 2017-03-24 12:17:18 UTC
Hidden the revisions works fine for me.
Menu/Edit/Track changes/Show
Comment 2 Frederic Parrenin 2017-03-24 12:23:44 UTC
Yes, it works when you do not show the track changes.
But if you show them and copy some text, it will copy the words that have been deleted in the track changes.
Please read again carefully my steps to reproduce.
Comment 3 m.a.riosv 2017-03-24 12:52:56 UTC Comment hidden (obsolete)
Comment 4 Frederic Parrenin 2017-03-24 12:56:35 UTC Comment hidden (obsolete)
Comment 5 m.a.riosv 2017-03-24 15:27:00 UTC Comment hidden (obsolete)
Comment 6 Frederic Parrenin 2017-03-24 15:30:22 UTC Comment hidden (obsolete)
Comment 7 Buovjaga 2017-03-29 14:46:08 UTC Comment hidden (obsolete)
Comment 8 Frederic Parrenin 2017-03-29 15:01:04 UTC Comment hidden (obsolete)
Comment 9 Buovjaga 2017-03-29 15:28:21 UTC
(In reply to Frederic Parrenin from comment #8)
> If you want to copy deleted text, you discard the changes, then copy/paste.
> The problem again is that it works as I expect when I start from a fresh LO
> document: deleted text is NOT copied. So there is a least a consistency
> problem here.

Ok, in my first test with a fresh document it was copied, but now I tried again and it was not copied..

I noticed that in the example docx, the final surveys is not copied.

..and now I noticed this is a regression: it works like expected in 3.6.

Arch Linux 64-bit, KDE Plasma 5
Version: 5.4.0.0.alpha0+
Build ID: 595e3971b130addbc6e5b749f53fc681fa6bc031
CPU threads: 8; OS: Linux 4.10; UI render: default; VCL: kde4; 
Locale: fi-FI (fi_FI.UTF-8); Calc: group
Built on March 27th 2016

Arch Linux 64-bit
Version 3.6.7.2 (Build ID: e183d5b)
Comment 10 Xisco Faulí 2017-03-30 09:53:23 UTC
Issue reproduced in

Version: 5.2.0.0.alpha1+
Build ID: 5b168b3fa568e48e795234dc5fa454bf24c9805e
CPU Threads: 4; OS Version: Linux 4.8; UI Render: default; 
Locale: ca-ES (ca_ES.UTF-8)

but not in

Version: 5.0.0.0.alpha1+
Build ID: 0db96caf0fcce09b87621c11b584a6d81cc7df86
Locale: ca-ES (ca_ES.UTF-8)
Comment 11 raal 2017-03-30 15:37:41 UTC
This seems to have begun at the below commit.
Adding Cc: to Noel Grandin; Could you possibly take a look at this one? Thanks


86334d4aee98252413f81e7fe4e2e0fbd01346de is the first bad commit
commit 86334d4aee98252413f81e7fe4e2e0fbd01346de
Author: Norbert Thiebaud <nthiebaud@gmail.com>
Date:   Thu Nov 12 10:00:12 2015 -0800

    source sha:db17d3c17c40d6b0e92392cf3c6e343d1d17b771

author	Noel Grandin <noel@peralex.com>	2015-11-10 11:36:34 (GMT)
committer	Noel Grandin <noelgrandin@gmail.com>	2015-11-11 07:16:20 (GMT)
commit	db17d3c17c40d6b0e92392cf3c6e343d1d17b771 (patch)
tree	9d562fcf764e7717df9585ef0e735a12ea4aaa16
parent	2ce9e4be4a438203382cb9cca824ce3e90647f3a (diff)
new loplugin: memoryvar
detect when we can convert a new/delete sequence on a local variable to
use std::unique_ptr
Comment 12 Aron Budea 2018-03-07 02:47:42 UTC
*** Bug 116199 has been marked as a duplicate of this bug. ***
Comment 13 Aron Budea 2018-03-07 04:56:44 UTC
The problem is with the following piece of change:
https://cgit.freedesktop.org/libreoffice/core/diff/sw/source/core/doc/DocumentContentOperationsManager.cxx?id=db17d3c17c40d6b0e92392cf3c6e343d1d17b771

The pointer in pDelPam actually needs to be preserved here:
pDelPam.reset(new SwPaM( *pCpyStt, pDelPam.get() ));

The following change fixes the behavior by releasing the smart pointer:
pDelPam.reset(new SwPaM( *pCpyStt, pDelPam.release() ));

Is the code otherwise correct? I'm surprised working with an invalid pointer didn't cause a serious issue.
Comment 14 Commit Notification 2018-03-26 13:45:18 UTC
Aron Budea committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=7b1d50e97eaa00855152e74f42b789fc643e0bac

tdf#106746: pDelPam is a bit special

It will be available in 6.1.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 15 Aron Budea 2018-03-26 13:47:27 UTC
Backport to 6.0 is in gerrit.
Comment 16 Xisco Faulí 2018-03-28 09:52:05 UTC
Verified in

Version: 6.1.0.0.alpha0+
Build ID: 8329f4541e27402d19729ae1588af8bfe61f7b49
CPU threads: 4; OS: Linux 4.13; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); Calc: group

Thank you Aron!
Comment 17 Commit Notification 2018-03-29 13:11:32 UTC
Aron Budea committed a patch related to this issue.
It has been pushed to "libreoffice-5-4":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=cba73f04699592c7038156f91ed92be026ed2536&h=libreoffice-5-4

tdf#106746: pDelPam is a bit special

It will be available in 5.4.7.

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 18 Commit Notification 2018-04-04 09:08:11 UTC
Aron Budea committed a patch related to this issue.
It has been pushed to "libreoffice-6-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=8e2850752bfa86f9b7c6f10a74d5ed69bc20ebee&h=libreoffice-6-0

tdf#106746: pDelPam is a bit special

It will be available in 6.0.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 19 Commit Notification 2018-04-08 10:50:02 UTC
Zdeněk Crhonek committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=4fda06ac4b584941561d3d0b757735e9e00827a2

uitest for bug tdf#106746

It will be available in 6.1.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.