sw/qa/extras/unowriter/data/image-comment-at-char.odt |binary sw/qa/extras/unowriter/unowriter.cxx | 30 ++++++++++++ sw/source/core/unocore/unofield.cxx | 6 +- sw/source/core/unocore/unoportenum.cxx | 44 +++++++++++++++--- 4 files changed, 70 insertions(+), 10 deletions(-)
New commits: commit d2e2760eb1ce392422237b776610350a05188bd2 Author: Miklos Vajna <vmik...@collabora.com> AuthorDate: Fri Jun 21 13:53:24 2019 +0200 Commit: Miklos Vajna <vmik...@collabora.com> CommitDate: Thu Jul 4 09:02:10 2019 +0200 sw comments on frames: fix annotation start order in UNO portion enum The problem was similar to commit 76a4305d1e90b6617054dd33036e64f005dbcf04 (sw: fix inconsistent bookmark behavior around at-char/as-char anchored frames, 2017-12-21), except here we have an (annotation) mark which does have a range, but still around at-char anchored frames, similar to the bookmark situation in that commit. Fix the problem similarly, by first adding comment-start portions to the enumeration, then frames, and finally the rest of the comment portions (as before). With this, an ODF <office:annotation/><draw:frame text:anchor-type="char"/><office:annotation-end/> sequence (commented at-char image) is roundtripped correctly. Reviewed-on: https://gerrit.libreoffice.org/74503 Reviewed-by: Miklos Vajna <vmik...@collabora.com> Tested-by: Jenkins (cherry picked from commit bf4b40f720146e7f76dab4deb72e85ea158e2d17) Conflicts: sw/source/core/unocore/unoportenum.cxx Change-Id: I8790d9efae625de48c689ca4180fe75f15b4833c Reviewed-on: https://gerrit.libreoffice.org/75036 Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com> Tested-by: Miklos Vajna <vmik...@collabora.com> diff --git a/sw/qa/extras/unowriter/unowriter.cxx b/sw/qa/extras/unowriter/unowriter.cxx index 2d21cf9dbef2..464b150dadfa 100644 --- a/sw/qa/extras/unowriter/unowriter.cxx +++ b/sw/qa/extras/unowriter/unowriter.cxx @@ -137,6 +137,20 @@ DECLARE_UNOAPI_TEST(testImageCommentAtChar) // is also the anchor position of the image). IDocumentMarkAccess* pMarks = pDoc->getIDocumentMarkAccess(); CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(1), pMarks->getAnnotationMarksCount()); + + uno::Reference<text::XTextRange> xPara = getParagraph(1); + CPPUNIT_ASSERT_EQUAL(OUString("Text"), + getProperty<OUString>(getRun(xPara, 1), "TextPortionType")); + // Without the accompanying fix in place, this test would have failed with 'Expected: + // Annotation; Actual: Frame', i.e. the comment-start portion was after the commented image. + CPPUNIT_ASSERT_EQUAL(OUString("Annotation"), + getProperty<OUString>(getRun(xPara, 2), "TextPortionType")); + CPPUNIT_ASSERT_EQUAL(OUString("Frame"), + getProperty<OUString>(getRun(xPara, 3), "TextPortionType")); + CPPUNIT_ASSERT_EQUAL(OUString("AnnotationEnd"), + getProperty<OUString>(getRun(xPara, 4), "TextPortionType")); + CPPUNIT_ASSERT_EQUAL(OUString("Text"), + getProperty<OUString>(getRun(xPara, 5), "TextPortionType")); } CPPUNIT_PLUGIN_IMPLEMENT(); diff --git a/sw/source/core/unocore/unoportenum.cxx b/sw/source/core/unocore/unoportenum.cxx index 7ca0c2e8fe87..b4828c18a660 100644 --- a/sw/source/core/unocore/unoportenum.cxx +++ b/sw/source/core/unocore/unoportenum.cxx @@ -1174,12 +1174,25 @@ static void lcl_ExportBkmAndRedline( lcl_ExportSoftPageBreak(rPortions, xParent, pUnoCursor, rBreakArr, nIndex); } +/** + * Exports all start annotation marks from rAnnotationStartArr into rPortions that have the same + * start position as nIndex. + * + * @param rAnnotationStartArr the array of annotation marks. Consumed entries are removed. + * + * @param rFramePositions the list of positions where there is an at-char anchored frame. + * + * @param bOnlyFrame If true: export only the start of annotation marks which cover an at-char + * anchored frame. If false: export everything else. + */ static void lcl_ExportAnnotationStarts( TextRangeList_t & rPortions, Reference<XText> const & xParent, const SwUnoCursor * const pUnoCursor, SwAnnotationStartPortion_ImplList& rAnnotationStartArr, - const sal_Int32 nIndex) + const sal_Int32 nIndex, + const std::set<sal_Int32>& rFramePositions, + bool bOnlyFrame) { if ( !rAnnotationStartArr.empty() ) { @@ -1197,12 +1210,18 @@ static void lcl_ExportAnnotationStarts( break; } - SwXTextPortion* pPortion = - new SwXTextPortion( pUnoCursor, xParent, PORTION_ANNOTATION ); - pPortion->SetTextField( pPtr->mxAnnotationField ); - rPortions.emplace_back(pPortion); + bool bFrameStart = rFramePositions.find(nIndex) != rFramePositions.end(); + if (bFrameStart || !bOnlyFrame) + { + SwXTextPortion* pPortion = + new SwXTextPortion( pUnoCursor, xParent, PORTION_ANNOTATION ); + pPortion->SetTextField( pPtr->mxAnnotationField ); + rPortions.emplace_back(pPortion); - aIter = rAnnotationStartArr.erase(aIter); + aIter = rAnnotationStartArr.erase(aIter); + } + else + ++aIter; } } } @@ -1372,6 +1391,15 @@ static void lcl_CreatePortions( lcl_ExportBkmAndRedline( *PortionStack.top().first, i_xParentText, pUnoCursor, Bookmarks, Redlines, SoftPageBreaks, nCurrentIndex, aFramePositions, /*bOnlyFrameBookmarkStarts=*/true ); + lcl_ExportAnnotationStarts( + *PortionStack.top().first, + i_xParentText, + pUnoCursor, + AnnotationStarts, + nCurrentIndex, + aFramePositions, + /*bOnlyFrame=*/true ); + const sal_Int32 nFirstFrameIndex = lcl_ExportFrames( *PortionStack.top().first, i_xParentText, pUnoCursor, i_rFrames, nCurrentIndex); @@ -1386,7 +1414,9 @@ static void lcl_CreatePortions( i_xParentText, pUnoCursor, AnnotationStarts, - nCurrentIndex ); + nCurrentIndex, + aFramePositions, + /*bOnlyFrame=*/false ); bool bCursorMoved( false ); sal_Int32 nNextAttrIndex = -1; commit e969f039d1988c7bc908386dbcbd250410bbd3f8 Author: Miklos Vajna <vmik...@collabora.com> AuthorDate: Thu Jun 20 16:58:06 2019 +0200 Commit: Miklos Vajna <vmik...@collabora.com> CommitDate: Thu Jul 4 09:01:55 2019 +0200 sw comments: allow ranges to be created which cover only the placeholder char First, the old condition was a bit tricky: it wanted to avoid the case when only the placeholder character is covered, but given that start and end can be the same for collapsed ranges, this was true for comments without a range, too. In practice what remains blacklisted is just the case when the PaM has a mark, but it's the same as the point. The change has two motivations: 1) ODF can have '<office:annotation/>...<office:annotation-end/>', where '...' may be content which has no document position itself in the Writer implementation, like at-char anchored images. This change avoids loosing the range of such annotations during ODF roundtrip. 2) This starts adding support for comments on objects which don't have an own document position, though UI for that is still missing. (cherry picked from commit 34c008380b9c17a78b9fde8cfa2102ab14c2392b) Conflicts: sw/qa/extras/unowriter/unowriter.cxx Change-Id: If151b8e00e37e07830c0582b8f0920a91a787363 Reviewed-on: https://gerrit.libreoffice.org/75035 Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com> Tested-by: Miklos Vajna <vmik...@collabora.com> diff --git a/sw/qa/extras/unowriter/data/image-comment-at-char.odt b/sw/qa/extras/unowriter/data/image-comment-at-char.odt new file mode 100644 index 000000000000..ba959d68ace9 Binary files /dev/null and b/sw/qa/extras/unowriter/data/image-comment-at-char.odt differ diff --git a/sw/qa/extras/unowriter/unowriter.cxx b/sw/qa/extras/unowriter/unowriter.cxx index 3fefa93f4d87..2d21cf9dbef2 100644 --- a/sw/qa/extras/unowriter/unowriter.cxx +++ b/sw/qa/extras/unowriter/unowriter.cxx @@ -123,6 +123,22 @@ DECLARE_UNOAPI_TEST_FILE(testRenderablePagePosition, "renderable-page-position.o CPPUNIT_ASSERT_GREATER(aPosition1.Y, aPosition2.Y); } +DECLARE_UNOAPI_TEST(testImageCommentAtChar) +{ + // Load a document with an at-char image in it (and a comment on the image). + load(mpTestDocumentPath, "image-comment-at-char.odt"); + SwXTextDocument* pTextDoc = dynamic_cast<SwXTextDocument*>(mxComponent.get()); + CPPUNIT_ASSERT(pTextDoc); + SwDoc* pDoc = pTextDoc->GetDocShell()->GetDoc(); + + // Verify that we have an annotation mark (comment with a text range) in the document. + // Without the accompanying fix in place, this test would have failed, as comments lost their + // ranges on load when their range only covered the placeholder character of the comment (which + // is also the anchor position of the image). + IDocumentMarkAccess* pMarks = pDoc->getIDocumentMarkAccess(); + CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(1), pMarks->getAnnotationMarksCount()); +} + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sw/source/core/unocore/unofield.cxx b/sw/source/core/unocore/unofield.cxx index 7f5b6e96242e..cc093131db90 100644 --- a/sw/source/core/unocore/unofield.cxx +++ b/sw/source/core/unocore/unofield.cxx @@ -1967,9 +1967,9 @@ void SAL_CALL SwXTextField::attach( if ( !::sw::XTextRangeToSwPaM( aIntPam, xTextRange ) ) throw lang::IllegalArgumentException(); - // nothing to do, if the text range only covers the former annotation field - if ( aIntPam.Start()->nNode != aIntPam.End()->nNode - || aIntPam.Start()->nContent.GetIndex() != aIntPam.End()->nContent.GetIndex()-1 ) + // Nothing to do, if the text range has a separate start and end, but they have the same + // value. + if (!aIntPam.HasMark() || *aIntPam.Start() != *aIntPam.End()) { UnoActionContext aCont( m_pImpl->m_pDoc ); // insert copy of annotation at new text range _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits