sw/qa/extras/uiwriter/data/text-with-formula-one-paragraph.fodt | 32 +++ sw/qa/extras/uiwriter/data/text-with-formula.fodt | 39 ++++ sw/qa/extras/uiwriter/uiwriter9.cxx | 88 ++++++++++ sw/source/core/layout/anchoredobject.cxx | 11 - sw/source/core/text/txtfrm.cxx | 7 sw/source/uibase/docvw/edtwin.cxx | 6 sw/source/uibase/inc/wrtsh.hxx | 2 sw/source/uibase/wrtsh/wrtsh1.cxx | 58 ++++++ 8 files changed, 232 insertions(+), 11 deletions(-)
New commits: commit 344d67ecebf93ca61569f9e57d318abea3d0ac18 Author: Mike Kaganski <[email protected]> AuthorDate: Thu Jun 26 23:19:50 2025 +0500 Commit: Christian Lohmaier <[email protected]> CommitDate: Fri Jul 11 14:36:32 2025 +0200 tdf#167132: Reimplement SwWrtShell::InsertEnclosingChars It used IDocumentContentOperations::ReplaceRange, and that created a number of problems. 1. The method is documented to only work in a single node. That meant, that trying to apply it on a selection over several paragraphs would make odd things: it would replace the selected part only of the first paragraph with the text of all the selected paragraphs, separated by hard line breaks (and enclosed in the requested characters). In debug builds, it would even fail an assert here. 2. It would replace all the content of the selection with just text, removing objects. 3. The implementation didn't check if the operation is permitted. 4. The implementation didn't create a single undo operation, and for a multi-selection, the operation would create multiple "replace" undo entries. This change re-implements it, trying to address these shortcomings. - It checks if the operation is permitted, consistent with how other methods there do it. - It merges all changes inti a single undo operation (with a proper description). - It inserts the requested strings in the start and ent boundaries of the selection, without replacing it. In this case, the start poses a problem: insertion inherits the formatting from behind, so may give the result having different formatting compared to the first selected character. This is workarounded by inserting the start string after the first character initially; then copying it (with its formatting) to the correct place, and removing the temporary insertion. - Change-Id: Ie3bf4d961ff5de5d2687692dac230579a176b960 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/187072 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> Signed-off-by: Xisco Fauli <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/187088 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/187104 Signed-off-by: Xisco Fauli <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/187314 Reviewed-by: Ilmari Lauhakangas <[email protected]> Reviewed-by: Christian Lohmaier <[email protected]> Tested-by: Christian Lohmaier <[email protected]> diff --git a/sw/qa/extras/uiwriter/data/text-with-formula.fodt b/sw/qa/extras/uiwriter/data/text-with-formula.fodt new file mode 100644 index 000000000000..17b77fdfe7d8 --- /dev/null +++ b/sw/qa/extras/uiwriter/data/text-with-formula.fodt @@ -0,0 +1,39 @@ +<?xml version="1.0" encoding="UTF-8"?> + +<office:document xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0" xmlns:fo="urn:oasis:names:tc:opendocument:xmlns:xsl-fo-compatible:1.0" xmlns:style="urn:oasis:names:tc:opendocument:xmlns:style:1.0" xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0" xmlns:draw="urn:oasis:names:tc:opendocument:xmlns:drawing:1.0" xmlns:svg="urn:oasis:names:tc:opendocument:xmlns:svg-compatible:1.0" office:version="1.4" office:mimetype="application/vnd.oasis.opendocument.text"> + <office:automatic-styles> + <style:style style:name="T1" style:family="text"> + <style:text-properties fo:font-weight="bold"/> + </style:style> + </office:automatic-styles> + <office:body> + <office:text> + <text:p>Paragraph one</text:p> + <text:p>Paragraph <text:span text:style-name="T1">two</text:span>: <draw:frame text:anchor-type="as-char" svg:width="15.54mm" svg:height="5.31mm"> + <draw:object> + <math xmlns="http://www.w3.org/1998/Math/MathML"; display="block"> + <semantics> + <mrow> + <mrow> + <msup> + <mi mathvariant="normal">e</mi> + <mrow> + <mi>i</mi> + <mi>π</mi> + </mrow> + </msup> + <mo stretchy="false">+</mo> + <mn>1</mn> + </mrow> + <mo stretchy="false">=</mo> + <mn>0</mn> + </mrow> + <annotation encoding="StarMath 5.0">{func e}^{i %pi} + 1 = 0</annotation> + </semantics> + </math> + </draw:object> + </draw:frame></text:p> + <text:p>Paragraph three</text:p> + </office:text> + </office:body> +</office:document> \ No newline at end of file diff --git a/sw/qa/extras/uiwriter/uiwriter9.cxx b/sw/qa/extras/uiwriter/uiwriter9.cxx index abd4800b8514..6171f329a238 100644 --- a/sw/qa/extras/uiwriter/uiwriter9.cxx +++ b/sw/qa/extras/uiwriter/uiwriter9.cxx @@ -17,6 +17,7 @@ #include <com/sun/star/awt/FontWeight.hpp> #include <com/sun/star/awt/FontSlant.hpp> +#include <com/sun/star/container/XContentEnumerationAccess.hpp> #include <com/sun/star/table/TableBorder2.hpp> #include <com/sun/star/text/XDocumentIndex.hpp> #include <com/sun/star/text/XTextFrame.hpp> @@ -576,6 +577,81 @@ CPPUNIT_TEST_FIXTURE(SwUiWriterTest9, testTdf151710) CPPUNIT_ASSERT_EQUAL(sStartDoubleQuote, xTextDocument->getText()->getString()); } +CPPUNIT_TEST_FIXTURE(SwUiWriterTest9, testTdf167132) +{ + // Given a document with several paragraphs, and a formula object + createSwDoc("text-with-formula.fodt"); + CPPUNIT_ASSERT_EQUAL(3, getParagraphs()); + SwXTextDocument* pTextDoc = getSwTextDoc(); + auto& rObjectContainer = pTextDoc->GetObjectShell()->GetEmbeddedObjectContainer(); + CPPUNIT_ASSERT(rObjectContainer.HasEmbeddedObjects()); + auto xMathObject = rObjectContainer.GetEmbeddedObject(rObjectContainer.GetObjectNames()[0]); + CPPUNIT_ASSERT(xMathObject); + + auto xSelSupplier = pTextDoc->getCurrentController().queryThrow<view::XSelectionSupplier>(); + auto xCursor = pTextDoc->getText()->createTextCursor(); + // 1. Select almost whole text, except the first and the last characters (to test the stability + // of the selection boundaries) - meaning that the selection spans across three paragraphs: + xCursor->gotoRange(pTextDoc->getText()->getStart(), false); + xCursor->goRight(1, false); + xCursor->gotoRange(pTextDoc->getText()->getEnd(), true); + xCursor->goLeft(1, true); + xSelSupplier->select(css::uno::Any(xCursor)); + + pTextDoc->postKeyEvent(LOK_KEYEVENT_KEYINPUT, '(', 0); + Scheduler::ProcessEventsToIdle(); + // The formula must not be removed from the document + CPPUNIT_ASSERT(rObjectContainer.HasEmbeddedObject(xMathObject)); + + auto xSelections = xSelSupplier->getSelection().queryThrow<css::container::XIndexAccess>(); + CPPUNIT_ASSERT_EQUAL(sal_Int32(1), xSelections->getCount()); + auto xSelection = xSelections->getByIndex(0).queryThrow<css::text::XTextRange>(); + // Check that selection includes parentheses + CPPUNIT_ASSERT_EQUAL(u"(aragraph one" SAL_NEWLINE_STRING "Paragraph two: " SAL_NEWLINE_STRING + "Paragraph thre)"_ustr, + xSelection->getString()); + // Check that the selection includes the formula + auto xContentEnumAccess = xSelection.queryThrow<css::container::XContentEnumerationAccess>(); + auto xContentEnum + = xContentEnumAccess->createContentEnumeration(u"com.sun.star.text.TextContent"_ustr); + CPPUNIT_ASSERT(xContentEnum->hasMoreElements()); + + // 2. Select part of the second paragraph, starting with the bold word, to the end (including + // the formula), to test that the added characters assume the formatting of selection: + auto xPara2 = getParagraph(2); + xCursor->gotoRange(xPara2->getStart(), false); + xCursor->goRight(10, false); // to start of "two", which is bold + xCursor->gotoRange(xPara2->getEnd(), true); + CPPUNIT_ASSERT_EQUAL(u"two: "_ustr, xCursor->getString()); + xContentEnumAccess.set(xCursor, css::uno::UNO_QUERY_THROW); + xContentEnum + = xContentEnumAccess->createContentEnumeration(u"com.sun.star.text.TextContent"_ustr); + CPPUNIT_ASSERT(xContentEnum->hasMoreElements()); // The selection contains the formula + xSelSupplier->select(css::uno::Any(xCursor)); + + pTextDoc->postKeyEvent(LOK_KEYEVENT_KEYINPUT, '[', 0); + Scheduler::ProcessEventsToIdle(); + // The formula must not be removed from the document + CPPUNIT_ASSERT(rObjectContainer.HasEmbeddedObject(xMathObject)); + + // The paragraph now must consist of five runs: first, the non-bold "Paragraph "; + // then bold "[two"; then non-bold ": "; then the formula; and then non-bold "]": + auto xRun = getRun(xPara2, 1, u"Paragraph "_ustr); + CPPUNIT_ASSERT_EQUAL(100.0f, getProperty<float>(xRun, u"CharWeight"_ustr)); + xRun = getRun(xPara2, 2, u"[two"_ustr); + CPPUNIT_ASSERT_EQUAL(150.0f, getProperty<float>(xRun, u"CharWeight"_ustr)); + xRun = getRun(xPara2, 3, u": "_ustr); + CPPUNIT_ASSERT_EQUAL(100.0f, getProperty<float>(xRun, u"CharWeight"_ustr)); + xRun = getRun(xPara2, 4, u""_ustr); + // Check that the run includes the formula + xContentEnumAccess.set(xRun, css::uno::UNO_QUERY_THROW); + xContentEnum + = xContentEnumAccess->createContentEnumeration(u"com.sun.star.text.TextContent"_ustr); + CPPUNIT_ASSERT(xContentEnum->hasMoreElements()); // The selection contains the formula + xRun = getRun(xPara2, 5, u"]"_ustr); + CPPUNIT_ASSERT_EQUAL(100.0f, getProperty<float>(xRun, u"CharWeight"_ustr)); +} + CPPUNIT_TEST_FIXTURE(SwUiWriterTest9, testTdf167133) { // Given a document with a single paragraph, having a formula object diff --git a/sw/source/uibase/docvw/edtwin.cxx b/sw/source/uibase/docvw/edtwin.cxx index f8c8798c7a5b..3aafc2574a4e 100644 --- a/sw/source/uibase/docvw/edtwin.cxx +++ b/sw/source/uibase/docvw/edtwin.cxx @@ -2601,13 +2601,13 @@ KEYINPUT_CHECKTABLE_INSDEL: switch (aCh) { case '(': - rSh.InsertEnclosingChars(u"(", u")"); + rSh.InsertEnclosingChars(u"("_ustr, u")"_ustr); break; case '[': - rSh.InsertEnclosingChars(u"[", u"]"); + rSh.InsertEnclosingChars(u"["_ustr, u"]"_ustr); break; case '{': - rSh.InsertEnclosingChars(u"{", u"}"); + rSh.InsertEnclosingChars(u"{"_ustr, u"}"_ustr); break; case '\"': { diff --git a/sw/source/uibase/inc/wrtsh.hxx b/sw/source/uibase/inc/wrtsh.hxx index 3534e439dd6a..bbcd20adb082 100644 --- a/sw/source/uibase/inc/wrtsh.hxx +++ b/sw/source/uibase/inc/wrtsh.hxx @@ -318,7 +318,7 @@ typedef bool (SwWrtShell::*FNSimpleMove)(); void InsertByWord( const OUString & ); SW_DLLPUBLIC void InsertPageBreak(const OUString *pPageDesc = nullptr, const ::std::optional<sal_uInt16>& rPgNum = std::nullopt); - void InsertEnclosingChars(std::u16string_view sStartStr, std::u16string_view sEndStr); + void InsertEnclosingChars(const OUString& sStartStr, const OUString& sEndStr); SW_DLLPUBLIC void InsertLineBreak(std::optional<SwLineBreakClear> oClear = std::nullopt); void InsertColumnBreak(); SW_DLLPUBLIC void InsertContentControl(SwContentControlType eType); diff --git a/sw/source/uibase/wrtsh/wrtsh1.cxx b/sw/source/uibase/wrtsh/wrtsh1.cxx index 73a5aeaf6b2a..4c007d20436a 100644 --- a/sw/source/uibase/wrtsh/wrtsh1.cxx +++ b/sw/source/uibase/wrtsh/wrtsh1.cxx @@ -989,13 +989,65 @@ void SwWrtShell::InsertPageBreak(const OUString *pPageDesc, const ::std::optiona // Insert enclosing characters // Selections will be overwritten -void SwWrtShell::InsertEnclosingChars(std::u16string_view sStartStr, std::u16string_view sEndStr) +void SwWrtShell::InsertEnclosingChars(const OUString& sStartStr, const OUString& sEndStr) { + if (!lcl_IsAllowed(this) || !CanInsert()) + return; + StartAllAction(); + StartUndo(); + + OUStringBuffer currentText, newText; + bool dotsAdded = false; + const OUString dots = SwResId(STR_LDOTS); for (SwPaM& rPaM : SwWrtShell::GetCursor()->GetRingContainer()) { - const OUString aStr = sStartStr + rPaM.GetText() + sEndStr; - SwViewShell::getIDocumentContentOperations().ReplaceRange(rPaM, aStr, false); + if (*rPaM.GetPoint() == *rPaM.GetMark()) + continue; + if (newText.isEmpty()) + { + OUString pamText = ShortenString(rPaM.GetText(), nUndoStringLength, dots) + .replaceAll(" ", " "); + currentText.append(pamText); + newText.append(sStartStr + pamText + sEndStr); + } + else if (!dotsAdded) + { + dotsAdded = true; + currentText.append(dots); + newText.append(dots); + } + + { + SwPaM aLocalPam(rPaM, nullptr); + aLocalPam.Normalize(); // point is at start now + auto& contentOperations = SwViewShell::getIDocumentContentOperations(); + + // To copy the formatting of the start of the range, insert the start string in two + // phases: insert it after the first selected character; and then move it back + SwPosition posStart = *aLocalPam.GetPoint(); + aLocalPam.GetPoint()->AdjustContent(+1); + contentOperations.InsertString(aLocalPam, sStartStr); + // Now aLocalPam's point is *after* the inserted string + SwPaM insertedPaM(*aLocalPam.GetPoint()); + insertedPaM.SetMark(); + insertedPaM.GetPoint()->AdjustContent(-sStartStr.getLength()); + contentOperations.CopyRange(insertedPaM, posStart, SwCopyFlags::CopyAll); + contentOperations.DeleteRange(insertedPaM); + + // No such problems with end string + aLocalPam.Exchange(); // point is at end now + contentOperations.InsertString(aLocalPam, sEndStr); + } + rPaM.Start()->AdjustContent(-sStartStr.getLength()); // now the selection includes insertion } + + SwRewriter aRewriter; + aRewriter.AddRule(UndoArg1, currentText.makeStringAndClear()); + aRewriter.AddRule(UndoArg2, SwResId(STR_YIELDS)); + aRewriter.AddRule(UndoArg3, newText.makeStringAndClear()); + EndUndo(SwUndoId::UI_REPLACE, &aRewriter); + + EndAllAction(); } // Insert hard page break; commit 3e683f1b5e700a00d3cd7e937e6f77c34a5ae687 Author: Mike Kaganski <[email protected]> AuthorDate: Thu Jun 26 20:00:16 2025 +0500 Commit: Christian Lohmaier <[email protected]> CommitDate: Fri Jul 11 14:36:20 2025 +0200 tdf#167133: Be prepared that frame's object's anchor may be null ... between the time the object was removed from the model, and the layout update. Change-Id: Ieb2f42c18794090cdc8786d5cffd9ae6e2b8f147 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/187071 Reviewed-by: Mike Kaganski <[email protected]> Tested-by: Jenkins Signed-off-by: Xisco Fauli <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/187087 Signed-off-by: Xisco Fauli <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/187101 Signed-off-by: Xisco Fauli <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/187313 Reviewed-by: Ilmari Lauhakangas <[email protected]> Tested-by: Christian Lohmaier <[email protected]> Reviewed-by: Christian Lohmaier <[email protected]> diff --git a/sw/qa/extras/uiwriter/data/text-with-formula-one-paragraph.fodt b/sw/qa/extras/uiwriter/data/text-with-formula-one-paragraph.fodt new file mode 100644 index 000000000000..a422453f0cee --- /dev/null +++ b/sw/qa/extras/uiwriter/data/text-with-formula-one-paragraph.fodt @@ -0,0 +1,32 @@ +<?xml version="1.0" encoding="UTF-8"?> + +<office:document xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0" xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0" xmlns:draw="urn:oasis:names:tc:opendocument:xmlns:drawing:1.0" xmlns:svg="urn:oasis:names:tc:opendocument:xmlns:svg-compatible:1.0" office:version="1.4" office:mimetype="application/vnd.oasis.opendocument.text"> + <office:body> + <office:text> + <text:p>Paragraph one: <draw:frame text:anchor-type="as-char" svg:width="15.54mm" svg:height="5.31mm"> + <draw:object> + <math xmlns="http://www.w3.org/1998/Math/MathML"; display="block"> + <semantics> + <mrow> + <mrow> + <msup> + <mi mathvariant="normal">e</mi> + <mrow> + <mi>i</mi> + <mi>π</mi> + </mrow> + </msup> + <mo stretchy="false">+</mo> + <mn>1</mn> + </mrow> + <mo stretchy="false">=</mo> + <mn>0</mn> + </mrow> + <annotation encoding="StarMath 5.0">{func e}^{i %pi} + 1 = 0</annotation> + </semantics> + </math> + </draw:object> + </draw:frame>.</text:p> + </office:text> + </office:body> +</office:document> \ No newline at end of file diff --git a/sw/qa/extras/uiwriter/uiwriter9.cxx b/sw/qa/extras/uiwriter/uiwriter9.cxx index 6ba09d3aafc4..abd4800b8514 100644 --- a/sw/qa/extras/uiwriter/uiwriter9.cxx +++ b/sw/qa/extras/uiwriter/uiwriter9.cxx @@ -576,6 +576,18 @@ CPPUNIT_TEST_FIXTURE(SwUiWriterTest9, testTdf151710) CPPUNIT_ASSERT_EQUAL(sStartDoubleQuote, xTextDocument->getText()->getString()); } +CPPUNIT_TEST_FIXTURE(SwUiWriterTest9, testTdf167133) +{ + // Given a document with a single paragraph, having a formula object + createSwDoc("text-with-formula-one-paragraph.fodt"); + dispatchCommand(mxComponent, u".uno:SelectAll"_ustr, {}); + SwDoc* pDoc = getSwDoc(); + SwWrtShell* pWrtShell = pDoc->GetDocShell()->GetWrtShell(); + pDoc->getIDocumentContentOperations().ReplaceRange(*pWrtShell->GetCursor(), "", false); + Scheduler::ProcessEventsToIdle(); + // This must not crash! +} + CPPUNIT_TEST_FIXTURE(SwUiWriterTest9, testTdf159054_disableOutlineNumbering) { createSwDoc("tdf159054_disableOutlineNumbering.docx"); diff --git a/sw/source/core/layout/anchoredobject.cxx b/sw/source/core/layout/anchoredobject.cxx index bf0efef7ae8a..cb503fdb61dc 100644 --- a/sw/source/core/layout/anchoredobject.cxx +++ b/sw/source/core/layout/anchoredobject.cxx @@ -725,9 +725,14 @@ SwTextFrame* SwAnchoredObject::FindAnchorCharFrame() if ((rAnch.GetAnchorId() == RndStdIds::FLY_AT_CHAR) || (rAnch.GetAnchorId() == RndStdIds::FLY_AS_CHAR)) { - SwTextFrame* const pFrame(static_cast<SwTextFrame*>(AnchorFrame())); - TextFrameIndex const nOffset(pFrame->MapModelToViewPos(*rAnch.GetContentAnchor())); - pAnchorCharFrame = &pFrame->GetFrameAtOfst(nOffset); + // When the object was already removed from text, but the layout hasn't been + // updated yet, this can be nullptr: + if (const SwPosition* pContentAnchor = rAnch.GetContentAnchor()) + { + SwTextFrame* const pFrame(static_cast<SwTextFrame*>(AnchorFrame())); + TextFrameIndex const nOffset(pFrame->MapModelToViewPos(*pContentAnchor)); + pAnchorCharFrame = &pFrame->GetFrameAtOfst(nOffset); + } } else if (SwFlyFrame* pFlyFrame = DynCastFlyFrame()) { diff --git a/sw/source/core/text/txtfrm.cxx b/sw/source/core/text/txtfrm.cxx index c58055f19232..a96b465d9c7f 100644 --- a/sw/source/core/text/txtfrm.cxx +++ b/sw/source/core/text/txtfrm.cxx @@ -1800,8 +1800,13 @@ void SwTextFrame::HideAndShowObjects() sal_Int32 nHiddenStart; sal_Int32 nHiddenEnd; const SwFormatAnchor& rAnchorFormat = pContact->GetAnchorFormat(); + const SwNode* pNode = rAnchorFormat.GetAnchorNode(); + // When the object was already removed from text, but the layout hasn't been + // updated yet, this can be nullptr: + if (!pNode) + continue; SwScriptInfo::GetBoundsOfHiddenRange( - *rAnchorFormat.GetAnchorNode()->GetTextNode(), + *pNode->GetTextNode(), rAnchorFormat.GetAnchorContentOffset(), nHiddenStart, nHiddenEnd); // Under certain conditions if ( nHiddenStart != COMPLETE_STRING && bShouldBeHidden &&
