Bug 161189 - EDITING: Comment (aka "Note") on target cell is not replaced (deleted) when source cell has no comment
Summary: EDITING: Comment (aka "Note") on target cell is not replaced (deleted) when s...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
7.6.0.3 release
Hardware: All All
: medium normal
Assignee: Andreas Heinisch
URL:
Whiteboard: target:25.2.0 target:24.8.1
Keywords: bibisected, bisected, regression
Depends on:
Blocks: Calc-Comments
  Show dependency treegraph
 
Reported: 2024-05-20 17:06 UTC by Roland
Modified: 2024-08-20 14:37 UTC (History)
3 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 Roland 2024-05-20 17:06:51 UTC
When a source cell without comment is copied whith Ctrl-C and pasted to a target cell with Ctrl-V, an existing comment of the target cell does not disappear although it should. Function Edit/Insert Contents/Insert Contents/ALL=Yes shows same fault.
This wrong behaviour was seen since version 7.6.
actual System: Windows 10 x64, LibreOffice 24.2.3.2
Comment 1 raal 2024-05-20 21:23:02 UTC
Confirm with Version: 24.8.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: 6d5d9eaa61505cebaf3bde4bfc157d8e19fec8de
CPU threads: 4; OS: Linux 6.5; UI render: default; VCL: gtk3
Locale: cs-CZ (cs_CZ.UTF-8); UI: en-US
Calc: threaded Jumbo

Work in 7.3.3
Comment 2 raal 2024-05-20 21:32:51 UTC
This seems to have begun at the below commit in bibisect repository/OS linux-64-7.6.
Adding Cc: to Andreas Heinisch ; Could you possibly take a look at this one?
Thanks
 ca3d029dd18bba9118dce4bd591721002b7b4a4e is the first bad commit
commit ca3d029dd18bba9118dce4bd591721002b7b4a4e
Author: Jenkins Build User <tdf@pollux.tdf>
Date:   Sun Oct 29 06:16:11 2023 +0100

    source 535f8fde0c33c435e4a8e9f768003516ce933666

151759: tdf#141440 - Do not delete notes when pasting contents | https://gerrit.libreoffice.org/c/core/+/151759
Comment 3 Andreas Heinisch 2024-05-20 22:05:48 UTC
In bug 141440 this functionality was requested and implemented. I understand both workflows. Any thoughts?
Comment 4 ady 2024-05-20 22:16:49 UTC
(In reply to Andreas Heinisch from comment #3)
> In bug 141440 this functionality was requested and implemented. I understand
> both workflows. Any thoughts?

The request in tdf#141440 is about paste special, without having the comments/notes selected as part of the paste action. I understand the confusion. The intention (IIUC) in tdf#141440 is that if you paste, say, _only_ the content of the cell, the other factors should not be altered on the destination cell/range (e.g. the cell format remains, and also the comments/notes and everything other than the cell content in this example).

But when you paste all, or paste special when the checkbox for the comments is marked ON in the dialog, the comments are also replaced by the paste action, according to the original source cell/range. If the original cell/range contains a comment, it should end up pasted on the destination cell/range too in such case.
Comment 5 ady 2024-05-21 07:38:46 UTC
(In reply to ady from comment #4)
> But when you paste all, or paste special when the checkbox for the comments
> is marked ON in the dialog, the comments are also replaced by the paste

That should had been:
"...the comments should also be replaced by the paste..."

because ATM they aren't (that's the bug), but they _should_ under these conditions.
Comment 6 Roland 2024-05-21 09:24:11 UTC
I can confirm that an existing note gets replaced correctly when copied cell contains an note. The bug is only when source cell has not any note/comment.
Comment 7 ady 2024-05-21 10:58:32 UTC
(In reply to Roland from comment #6)
> I can confirm that an existing note gets replaced correctly when copied cell
> contains an note. The bug is only when source cell has not any note/comment.

I have modified the Summary accordingly.

I can see the potential advantage for some use-cases for such behavior. The problem is the inconsistency regarding pasting other factors.

I mean that someone might request that when a cell value is empty, then the copy+paste should skip the empty value replacing whichever non-empty value occupies the destination cell. Or some similar oddity with some other property (e.g. no borders in the source cell).

The current alternative is that users have to explicitly delete the comment of the target cell after pasting. I guess that UX has to opine on this one (independently of the original request in tdf#141440)?
Comment 8 ady 2024-05-21 11:07:58 UTC
To be clear, in any case the request from tdf#141440 should be canceled. The request in that report was and still is valid. When pasting _only_ values, then the other factors should not be affected in the target cell.

The potential confusion in tdf#141440 (which triggered this new report tdf#161189) comes from those paste special quick shortcuts and/or accelerators/icons/menus such as "paste text" or "paste formulas" or "paste numbers", which could mean either:

* paste all, but only when the source cells contain "text/formulas/numbers" (respectively); or,

* paste values only, but only when the source cells contain "text/formulas/numbers" (respectively).
Comment 9 ady 2024-05-21 11:09:30 UTC
Oops. Sorry:

(In reply to ady from comment #8)
> To be clear, in any case the request from tdf#141440 should be canceled.

To be clear, in any case the request from tdf#141440 should  **NOT**  be canceled.
Comment 10 Heiko Tietze 2024-05-21 12:56:41 UTC
(In reply to ady from comment #7)
> I guess that UX has to opine on this one
Andreas is assigned to fix the issue, so he prolly has a plan. Or will ask questions...
Comment 11 Andreas Heinisch 2024-06-19 08:14:22 UTC
This should be fixed after: https://gerrit.libreoffice.org/c/core/+/167202

Can anybody verify the fix?
Comment 12 Andreas Heinisch 2024-07-17 09:33:34 UTC
Should be resolved now with: https://gerrit.libreoffice.org/c/core/+/170039
Comment 13 ady 2024-07-17 16:03:38 UTC
(In reply to Andreas Heinisch from comment #12)
> Should be resolved now with: https://gerrit.libreoffice.org/c/core/+/170039

FWIW, while personally I am fine with the behavior as described in comment 0, I don't think the report is solved (in 24.8). The test is extremely simple.
Comment 14 ady 2024-07-17 17:17:16 UTC
If the source cell has no comment and the destination cell has a comment, currently the comment in the destination cell is not "replaced" (i.e. the comment is not deleted). In all other cases, the replacement is working as expected.

FWIW, I am in favor of leaving the current behavior.
Comment 15 Andreas Heinisch 2024-07-20 15:56:41 UTC
If you copy a source cell without a comment and you overwrite it using paste special, a confirmation dialog shows up and the comment will be overwritten. However, the original report is still valid.
Comment 16 Andreas Heinisch 2024-07-20 18:27:17 UTC
The implementation of the copy - either from paste special or from the clipboard - is confusing. The problem is that the cell note is note deleted since this is neede for paste special. 

Later it checks if the note needs to be copied, but there is none. So the old not will not be replaced.

I will try to delete the note afterwards if that is the case and the source still contains a note.
Comment 18 ady 2024-07-20 21:33:49 UTC
(In reply to Andreas Heinisch from comment #16)
> I will try to delete the note afterwards if that is the case and the source
> still contains a note.

Please beware that (multiple) undo actions won't generate more problems than it is worth.

I repeat: the particular situation is (more than) tolerable, as it has "pros", even if it is considered as (slightly) inconsistent. I would be OK leaving the behavior as it is today.
Comment 19 Commit Notification 2024-08-02 08:33:48 UTC
Andreas Heinisch committed a patch related to this issue.
It has been pushed to "master":

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

tdf#161189 - CopyFromClip: improve handling of deleting notes

It will be available in 25.2.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 20 Commit Notification 2024-08-20 14:37:23 UTC
Andreas Heinisch committed a patch related to this issue.
It has been pushed to "libreoffice-24-8":

https://git.libreoffice.org/core/commit/70a34d6b8bb4a4ecc89f1ceafaa2ea5d3a67acf8

tdf#161189 - CopyFromClip: improve handling of deleting notes

It will be available in 24.8.1.

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.