sc drawstyles: Assign the Note style to imported comments

... that don't have a style assignment. Typically in ods files
created by older versions or by 3rd party.

- For hidden comments this should make no difference, as we used
to apply the default comment formatting as DF underneath their
defined DF, just now we do that as a style assignment instead.

- Same for comments from xlsx and xls files.

- For visible comments from ods files created by OOo/LO, there
should be no difference either, as such files typically include
the shape formatting in them.

- For visible comments from ods files created by Excel, there
will be a difference, as Excel used to not include the full shape
formatting (known to be the case with Excel 2007 and 2016; can't
test any other version). This resulted with a weird look, e.g.
a line instead of an arrow, a default blue fill color and too
distant shadow, which clearly not how comments supposed to look.
Moreover, the comment will turn to transparent after hiding or
copying the cell, and will revert to the default look anyway
with resaving. Given that we were already enforcing the default
formatting for hidden comments and for foreign formats, I think
it's reasonable to do that for visible comments too (and in
general it's unclear to me why the ODF import treats visible
comments differently than hidden ones).

The main motivation of this change is to aid solving the shadow
issue - see the next commits.

Regarding the comment height change in the testCommentSize test:
This does *not* mean there is a change in that comment's import.
What this test does is to replace the existing comment with
a new comment, and that use the default formatting instead of
inheriting the formatting of the old one. But while the current
default formatting is whatever defined in the Note style, the old
default formatting was actually the draw pool defaults. This is
because we used to apply the comment formatting as DF, and the
relevant svx code (in SdrTextObj::NbcSetText) wasn't aware of the
fact that part of the DF was considered as default in this case.
Which means that this test was actually asserting a buggy behavior.

Change-Id: I37723cce3c719ecaa9c57bef25bcb168e353c55c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/150489
Tested-by: Jenkins
Reviewed-by: Maxim Monastirsky <momonasmon@gmail.com>

4 files changed, 21 insertions, 7 deletions

diff --git a/sc/inc/postit.hxx b/sc/inc/postit.hxx
index 7da83a9ebea0..6b458ad41763 100644
--- a/sc/inc/postit.hxx
+++ b/sc/inc/postit.hxx
@@ -193,6 +193,8 @@ public:
             The underlying ScPostIt::ScNoteData::ScCaptionPtr takes managing
             ownership of the pointer.
 
+        @param bHasStyle  Is there a drawing style set for the note.
+
         @return  Pointer to the new cell note object if insertion was
             successful (i.e. the passed cell position was valid), null
             otherwise. The Calc document is the owner of the note object. The
@@ -201,7 +203,7 @@ public:
      */
     static ScPostIt*    CreateNoteFromCaption(
                             ScDocument& rDoc, const ScAddress& rPos,
-                            SdrCaptionObj* pCaption );
+                            SdrCaptionObj* pCaption, bool bHasStyle );
 
     /** Creates a cell note based on the passed caption object data.
 
diff --git a/sc/qa/unit/subsequent_filters_test4.cxx b/sc/qa/unit/subsequent_filters_test4.cxx
index 06af93de19d9..c439a02c6582 100644
--- a/sc/qa/unit/subsequent_filters_test4.cxx
+++ b/sc/qa/unit/subsequent_filters_test4.cxx
@@ -45,6 +45,8 @@
 #include <sortparam.hxx>
 #include <undomanager.hxx>
 #include <tabprotection.hxx>
+#include <globstr.hrc>
+#include <scresid.hxx>
 
 #include <orcusfilters.hxx>
 #include <filter.hxx>
@@ -1580,6 +1582,10 @@ CPPUNIT_TEST_FIXTURE(ScFiltersTest4, testCommentSize)
     SdrCaptionObj* pCaption = pNote->GetCaption();
     CPPUNIT_ASSERT(pCaption);
 
+    // The values below depend on particular font and char size.
+    // At least assert that the corresponding style was set:
+    CPPUNIT_ASSERT_EQUAL(ScResId(STR_STYLENAME_NOTE), pCaption->GetStyleSheet()->GetName());
+
     const tools::Rectangle& rOldRect = pCaption->GetLogicRect();
     CPPUNIT_ASSERT_EQUAL(tools::Long(2899), rOldRect.getOpenWidth());
     CPPUNIT_ASSERT_EQUAL(tools::Long(939), rOldRect.getOpenHeight());
@@ -1588,7 +1594,7 @@ CPPUNIT_TEST_FIXTURE(ScFiltersTest4, testCommentSize)
 
     const tools::Rectangle& rNewRect = pCaption->GetLogicRect();
     CPPUNIT_ASSERT_EQUAL(rOldRect.getOpenWidth(), rNewRect.getOpenWidth());
-    CPPUNIT_ASSERT_EQUAL(tools::Long(1605), rNewRect.getOpenHeight());
+    CPPUNIT_ASSERT_EQUAL(tools::Long(1386), rNewRect.getOpenHeight());
 
     pDoc->GetUndoManager()->Undo();
 
diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx
index 8f6c14f7901b..f74937377d8e 100644
--- a/sc/source/core/data/postit.cxx
+++ b/sc/source/core/data/postit.cxx
@@ -111,9 +111,6 @@ void ScCaptionUtil::SetDefaultItems( SdrCaptionObj& rCaption, ScDocument& rDoc,
 {
     SfxItemSet aItemSet = rCaption.GetMergedItemSet();
 
-    if (auto pStyleSheet = rDoc.GetStyleSheetPool()->Find(ScResId(STR_STYLENAME_NOTE), SfxStyleFamily::Frame))
-        aItemSet.Put(pStyleSheet->GetItemSet());
-
     const ScPatternAttr& rDefPattern = rDoc.GetPool()->GetDefaultItem( ATTR_PATTERN );
     rDefPattern.FillEditItemSet( &aItemSet );
 
@@ -680,6 +677,9 @@ void ScPostIt::CreateCaptionFromInitData( const ScAddress& rPos ) const
     }
     else
     {
+        if (auto pStyleSheet = mrDoc.GetStyleSheetPool()->Find(ScResId(STR_STYLENAME_NOTE), SfxStyleFamily::Frame))
+            maNoteData.mxCaption->SetStyleSheet(static_cast<SfxStyleSheet*>(pStyleSheet), true);
+
         // copy all items or set default items; reset shadow items
         ScCaptionUtil::SetDefaultItems( *maNoteData.mxCaption, mrDoc, xInitData->moItemSet ? &*xInitData->moItemSet : nullptr );
     }
@@ -879,7 +879,7 @@ rtl::Reference<SdrCaptionObj> ScNoteUtil::CreateTempCaption(
 }
 
 ScPostIt* ScNoteUtil::CreateNoteFromCaption(
-        ScDocument& rDoc, const ScAddress& rPos, SdrCaptionObj* pCaption )
+        ScDocument& rDoc, const ScAddress& rPos, SdrCaptionObj* pCaption, bool bHasStyle )
 {
     ScNoteData aNoteData( true/*bShown*/ );
     aNoteData.mxCaption = pCaption;
@@ -891,6 +891,12 @@ ScPostIt* ScNoteUtil::CreateNoteFromCaption(
     // ScNoteCaptionCreator c'tor updates the caption object to be part of a note
     ScNoteCaptionCreator aCreator( rDoc, rPos, aNoteData.mxCaption, true/*bShown*/ );
 
+    if (!bHasStyle)
+    {
+        if (auto pStyleSheet = rDoc.GetStyleSheetPool()->Find(ScResId(STR_STYLENAME_NOTE), SfxStyleFamily::Frame))
+            aNoteData.mxCaption->SetStyleSheet(static_cast<SfxStyleSheet*>(pStyleSheet), true);
+    }
+
     return pNote;
 }
 
diff --git a/sc/source/filter/xml/xmlcelli.cxx b/sc/source/filter/xml/xmlcelli.cxx
index c2841fa33c7f..55ed931b47c0 100644
--- a/sc/source/filter/xml/xmlcelli.cxx
+++ b/sc/source/filter/xml/xmlcelli.cxx
@@ -859,7 +859,7 @@ void ScXMLTableRowCellContext::SetAnnotation(const ScAddress& rPos)
             {
                 OSL_ENSURE( !pCaption->GetLogicRect().IsEmpty(), "ScXMLTableRowCellContext::SetAnnotation - invalid caption rectangle" );
                 // create the cell note with the caption object
-                pNote = ScNoteUtil::CreateNoteFromCaption( *pDoc, rPos, pCaption );
+                pNote = ScNoteUtil::CreateNoteFromCaption( *pDoc, rPos, pCaption, !aStyleName.isEmpty() );
                 // forget pointer to object (do not create note again below)
                 pObject = nullptr;
             }