Bug 88200 - setString method deletes comment
Summary: setString method deletes comment
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: BASIC (show other bugs)
Version:
(earliest affected)
4.3.5.2 release
Hardware: Other All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:4.5.0 target:4.3.6 target:4.4.0
Keywords:
Depends on:
Blocks:
 
Reported: 2015-01-08 14:33 UTC by rebelxt
Modified: 2015-01-13 15:29 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 rebelxt 2015-01-08 14:33:09 UTC
I have LibreOffice 4.3.5.2 installed on Linux Mint 17. The following macro will delete any comment in the A1 cell. 

Sub Main
ThisComponent.CurrentController.getActiveSheet.getCellByPosition(0,0).setString("")
End Sub

setString("notnull") does not delete the comment. I see the same results in LO 4.3.4.1 in Windows XP. However, with LO 3.5.7.2 running in Linux Mint 13, the macro does not delete the comment.
Comment 1 Julien Nabet 2015-01-08 18:59:46 UTC
On pc Debian x86-64 with master sources updated today, I could reproduce this.
Comment 2 Julien Nabet 2015-01-08 20:51:43 UTC
Noel: would you have an idea about a code pointer? It doesn't seem on basic/ since fgrep -lR setString in it gave nothing interesting.
Comment 3 Noel Power 2015-01-09 09:36:57 UTC
(In reply to Julien Nabet from comment #2)
> Noel: would you have an idea about a code pointer? It doesn't seem on basic/
> since fgrep -lR setString in it gave nothing interesting.
it's nothing to do with basic, it's calc (uno) code called by basic e.g.

http://opengrok.libreoffice.org/xref/core/sc/source/ui/unoobj/cellsuno.cxx#6313
Comment 4 Julien Nabet 2015-01-09 09:46:18 UTC
Thank you Noel for your quick feedback, I'll give it a try.
Comment 5 Julien Nabet 2015-01-09 22:03:28 UTC
Lost in translation...
#0  ScColumn::ParseString (this=0x2aaad0d26010, rCell=..., nRow=0, nTabP=0, rString="", eConv=formula::FormulaGrammar::CONV_OOO, pParam=0x0)
    at /home/julien/compile-libreoffice/libreoffice/sc/source/core/data/column3.cxx:1594
#1  0x00002aaac92681be in ScColumn::SetString (this=0x2aaad0d26010, nRow=0, nTabP=0, rString="", eConv=formula::FormulaGrammar::CONV_OOO, pParam=0x0)
    at /home/julien/compile-libreoffice/libreoffice/sc/source/core/data/column3.cxx:1756
#2  0x00002aaac957e96e in ScTable::SetString (this=0x2aaad0d26010, nCol=0, nRow=0, nTabP=0, rString="", pParam=0x0)
    at /home/julien/compile-libreoffice/libreoffice/sc/source/core/data/table2.cxx:1342
#3  0x00002aaac9374b3b in ScDocument::SetString (this=0x8124058, nCol=0, nRow=0, nTab=0, rString="", pParam=0x0)
    at /home/julien/compile-libreoffice/libreoffice/sc/source/core/data/document.cxx:3204
#4  0x00002aaac9b0599d in ScDocFunc::SetNormalString (this=0x8128a40, o_rbNumFmtSet=@0x7fffffff2ca0: false, rPos=..., rText="", bApi=true)
    at /home/julien/compile-libreoffice/libreoffice/sc/source/ui/docshell/docfunc.cxx:815
#5  0x00002aaac9b0772d in ScDocFunc::SetCellText (this=0x8128a40, rPos=..., rText="", bInterpret=false, bEnglish=false, bApi=true, 
    eGrammar=formula::FormulaGrammar::GRAM_PODF_A1) at /home/julien/compile-libreoffice/libreoffice/sc/source/ui/docshell/docfunc.cxx:1218
#6  0x00002aaac9d36118 in ScCellObj::SetString_Impl (this=0x8683620, rString="", bInterpret=false, bEnglish=false)
    at /home/julien/compile-libreoffice/libreoffice/sc/source/ui/unoobj/cellsuno.cxx:6194
#7  0x00002aaac9d36a07 in ScCellObj::setString (this=0x8683620, aText="") at /home/julien/compile-libreoffice/libreoffice/sc/source/ui/unoobj/cellsuno.cxx:6317

I can't help here.
Comment 6 Julien Nabet 2015-01-09 22:12:21 UTC
I'm just wondering if this behaviour could be on purpose.

Kohei/Markus/Eike: any thoughts?
Comment 7 Eike Rathke 2015-01-09 23:24:55 UTC
Pretty sure it should only clear cell content, not the comment/note/annotation. Probably with an empty string a short cut is taken that deletes the entire cell.
Comment 8 Markus Mohrhard 2015-01-10 07:33:49 UTC
(In reply to Eike Rathke from comment #7)
> Pretty sure it should only clear cell content, not the
> comment/note/annotation. Probably with an empty string a short cut is taken
> that deletes the entire cell.

And most likely it just contains the wrong flags. Instead of just containing the flags to delete the cell content it contains IDF_ALL or something similar. Just try to step through the code and check where the delete call happens or set a breakpoint at some interesting methods (like the ScPostIt destructor).
Comment 9 Julien Nabet 2015-01-10 09:19:27 UTC
Thank you Eike and Markus for your feedback.

I tried again to find where the comment could be removed but failed.
Hope someone will be luckier than me :-)
Comment 10 Markus Mohrhard 2015-01-10 12:12:10 UTC
Ok. It was a bit more complicated. The code incorrectly deleted more cell properties than necessary in that case. I change the code to be consistent between the different cases. It now only deletes the cell content and leaves notes and cell text attribs.
Comment 11 Commit Notification 2015-01-10 12:49:43 UTC
Markus Mohrhard committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=8449e43255c0929af9c9a205e60db2bb0f4fbde8

only delete cell content for CELLTYPE_NONE, fdo#88200

It will be available in 4.5.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 12 Commit Notification 2015-01-10 12:49:52 UTC
Markus Mohrhard committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=29a5692bcf22f27a87959b87065e6c65b59ae886

add test for fdo#88200

It will be available in 4.5.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 13 rebelxt 2015-01-11 12:56:14 UTC
Will this patch be backported to 4.3.5?
Comment 14 Julien Nabet 2015-01-11 13:45:43 UTC
I could check it was ok with master sources updated yesterday.
Thank you Markus!

About backports, here are the commits to review:
- for 4.4: https://gerrit.libreoffice.org/#/c/13842/
- for 4.3: https://gerrit.libreoffice.org/#/c/13843/
It can't be in 4.3.5 since 4.3.5 has already been released.
I'll be perhaps in 4.3.6, if not, at least on 4.3.7 or 4.4.0
Comment 15 Commit Notification 2015-01-12 16:12:10 UTC
Markus Mohrhard committed a patch related to this issue.
It has been pushed to "libreoffice-4-3":

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

only delete cell content for CELLTYPE_NONE, fdo#88200

It will be available in 4.3.6.

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 16 Commit Notification 2015-01-12 16:20:03 UTC
Markus Mohrhard committed a patch related to this issue.
It has been pushed to "libreoffice-4-4":

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

only delete cell content for CELLTYPE_NONE, fdo#88200

It will be available in 4.4.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 17 Commit Notification 2015-01-13 14:49:38 UTC
Markus Mohrhard committed a patch related to this issue.
It has been pushed to "libreoffice-4-4-0":

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

only delete cell content for CELLTYPE_NONE, fdo#88200

It will be available in 4.4.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 18 Julien Nabet 2015-01-13 15:29:11 UTC
Let's simplify a bit.