diff options
author | Daniel Arato (NISZ) <arato.daniel@nisz.hu> | 2021-02-22 16:59:38 +0100 |
---|---|---|
committer | Balazs Varga <varga.balazs3@nisz.hu> | 2021-03-17 13:10:27 +0100 |
commit | 8d8486f43c1a8a51157bfc3e0b87090b05a9229e (patch) | |
tree | 1321f23e4480341338f979f08fce6cdf46030f85 | |
parent | 262c07d2b733543dd895b92325b36ce093226f26 (diff) |
tdf#46561 sw: fix lost undo stack setting header/footer
Changing 'shared' setting of left vs right or
first vs non-first page headers or footers removed
the whole undo stack.
Note: style changes before a 'shared' change
can still not be undone.
Co-authored-by: Attila Bakos (NISZ)
Change-Id: I6875bd0581869ffeb853911347dbc9f8c666214b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/111635
Reviewed-by: László Németh <nemeth@numbertext.org>
Reviewed-by: Balazs Varga <varga.balazs3@nisz.hu>
Tested-by: Balazs Varga <varga.balazs3@nisz.hu>
-rw-r--r-- | sw/qa/extras/uiwriter/uiwriter.cxx | 4 | ||||
-rw-r--r-- | sw/qa/uitest/data/tdf46561.odt | bin | 0 -> 8798 bytes | |||
-rw-r--r-- | sw/qa/uitest/writer_tests7/tdf46561.py | 105 | ||||
-rw-r--r-- | sw/source/core/doc/docdesc.cxx | 87 | ||||
-rw-r--r-- | sw/source/core/layout/pagedesc.cxx | 27 |
5 files changed, 156 insertions, 67 deletions
diff --git a/sw/qa/extras/uiwriter/uiwriter.cxx b/sw/qa/extras/uiwriter/uiwriter.cxx index c39659fa9bab..dae657a35088 100644 --- a/sw/qa/extras/uiwriter/uiwriter.cxx +++ b/sw/qa/extras/uiwriter/uiwriter.cxx @@ -2225,7 +2225,9 @@ void SwUiWriterTest::testTextCursorInvalidation() // this does not actually delete the header: xPageStyle->setPropertyValue("HeaderIsOn", uno::makeAny(false)); pWrtShell->ChangeHeaderOrFooter(u"Default Page Style", true, false, false); // must be disposed after deleting header - CPPUNIT_ASSERT_THROW(xCursor->goRight(1, false), uno::RuntimeException); + // cursor ends up in body + // UPDATE: this behaviour has been corrected as a side effect of the fix to tdf#46561: + //CPPUNIT_ASSERT_THROW(xCursor->goRight(1, false), uno::RuntimeException); } void SwUiWriterTest::testTdf68183() diff --git a/sw/qa/uitest/data/tdf46561.odt b/sw/qa/uitest/data/tdf46561.odt Binary files differnew file mode 100644 index 000000000000..c9f9942521d0 --- /dev/null +++ b/sw/qa/uitest/data/tdf46561.odt diff --git a/sw/qa/uitest/writer_tests7/tdf46561.py b/sw/qa/uitest/writer_tests7/tdf46561.py new file mode 100644 index 000000000000..235136524903 --- /dev/null +++ b/sw/qa/uitest/writer_tests7/tdf46561.py @@ -0,0 +1,105 @@ +# -*- tab-width: 4; indent-tabs-mode: nil; py-indent-offset: 4 -*- +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +from uitest.framework import UITestCase +from libreoffice.uno.propertyvalue import mkPropertyValues +from uitest.uihelper.common import select_pos +from uitest.uihelper.common import type_text +import importlib +import org.libreoffice.unotest +import pathlib + +def get_url_for_data_file(file_name): + return pathlib.Path(org.libreoffice.unotest.makeCopyFromTDOC(file_name)).as_uri() + +class tdf46561(UITestCase): + def check_header_texts(self, master="", first="", left="", right=""): + # Get the current header style and its text contents + xPageStyle = self.document.getStyleFamilies().getByIndex(2) + xHeaderText = xPageStyle.getByIndex(0).HeaderText.String + xHeaderTextFirst = xPageStyle.getByIndex(0).HeaderTextFirst.String + xHeaderTextLeft = xPageStyle.getByIndex(0).HeaderTextLeft.String + xHeaderTextRight = xPageStyle.getByIndex(0).HeaderTextRight.String + + # Check the current values + self.assertEqual(master, xHeaderText) + self.assertEqual(first, xHeaderTextFirst) + self.assertEqual(left, xHeaderTextLeft) + self.assertEqual(right, xHeaderTextRight) + + def test_tdf46561(self): + self.ui_test.load_file(get_url_for_data_file("tdf46561.odt")) + self.document = self.ui_test.get_component() + self.check_header_texts(master="right", first="1st", left="left", right="right") + + xWriterDoc = self.xUITest.getTopFocusWindow() + xWriterEdit = xWriterDoc.getChild("writer_edit") + xWriterEdit.executeAction("GOTO", mkPropertyValues({"PAGE": "2"})) + self.xUITest.executeCommand(".uno:JumpToHeader") + + # Switch "same left and right page headers" on and off a few times + for _ in range(4): + self.ui_test.execute_dialog_through_command(".uno:PageDialog") + PageDialog = self.xUITest.getTopFocusWindow(); + + xTabs = PageDialog.getChild("tabcontrol") + select_pos(xTabs, "4") + + Button = xTabs.getChild('checkSameLR') + Button.executeAction("CLICK",tuple()) + ok = PageDialog.getChild("ok") + self.ui_test.close_dialog_through_button(ok) + + # We should be back to the starting state after 2*k on/off changes + self.check_header_texts(master="right", first="1st", left="left", right="right") + + # Enter some additional text in the left page header + type_text(xWriterEdit, "XXXX") + self.check_header_texts(master="right", first="1st", left="XXXXleft", right="right") + + # Now go back one change (before entering "XXXX") + self.xUITest.executeCommand(".uno:Undo") + self.check_header_texts(master="right", first="1st", left="left", right="right") + + # Undo the fourth change + self.xUITest.executeCommand(".uno:Undo") + self.check_header_texts(master="right", first="1st", left="right", right="right") + + # Undo the third change + self.xUITest.executeCommand(".uno:Undo") + self.check_header_texts(master="right", first="1st", left="left", right="right") + + # Undo the second change + self.xUITest.executeCommand(".uno:Undo") + self.check_header_texts(master="right", first="1st", left="right", right="right") + + # Undo the first change + self.xUITest.executeCommand(".uno:Undo") + self.check_header_texts(master="right", first="1st", left="left", right="right") + + # Redo the first change + self.xUITest.executeCommand(".uno:Redo") + self.check_header_texts(master="right", first="1st", left="right", right="right") + + # Redo the second change + self.xUITest.executeCommand(".uno:Redo") + self.check_header_texts(master="right", first="1st", left="left", right="right") + + # Redo the third change + self.xUITest.executeCommand(".uno:Redo") + self.check_header_texts(master="right", first="1st", left="right", right="right") + + # Redo the fourth change + self.xUITest.executeCommand(".uno:Redo") + self.check_header_texts(master="right", first="1st", left="left", right="right") + + # Redo the final change + self.xUITest.executeCommand(".uno:Redo") + self.check_header_texts(master="right", first="1st", left="XXXXleft", right="right") + + self.ui_test.close_doc() + +# vim: set shiftwidth=4 softtabstop=4 expandtab: diff --git a/sw/source/core/doc/docdesc.cxx b/sw/source/core/doc/docdesc.cxx index 1c40970c4ecd..9d10e4b45214 100644 --- a/sw/source/core/doc/docdesc.cxx +++ b/sw/source/core/doc/docdesc.cxx @@ -388,8 +388,35 @@ void SwDoc::ChgPageDesc( size_t i, const SwPageDesc &rChged ) if (GetIDocumentUndoRedo().DoesUndo()) { - GetIDocumentUndoRedo().AppendUndo( - std::make_unique<SwUndoPageDesc>(rDesc, rChged, this)); + // Stash header formats as needed. + const SwFormatHeader& rLeftHead = rChged.GetLeft().GetHeader(); + const SwFormatHeader& rFirstMasterHead = rChged.GetFirstMaster().GetHeader(); + const SwFormatHeader& rFirstLeftHead = rChged.GetFirstLeft().GetHeader(); + const bool bStashLeftHead = !rDesc.IsHeaderShared() && rChged.IsHeaderShared(); + const bool bStashFirstMasterHead = !rDesc.IsFirstShared() && rChged.IsFirstShared(); + const bool bStashFirstLeftHead = (!rDesc.IsHeaderShared() && rChged.IsHeaderShared()) || (!rDesc.IsFirstShared() && rChged.IsFirstShared()); + if (bStashLeftHead && rLeftHead.GetRegisteredIn()) + rDesc.StashFrameFormat(rChged.GetLeft(), true, true, false); + if (bStashFirstMasterHead && rFirstMasterHead.GetRegisteredIn()) + rDesc.StashFrameFormat(rChged.GetFirstMaster(), true, false, true); + if (bStashFirstLeftHead && rFirstLeftHead.GetRegisteredIn()) + rDesc.StashFrameFormat(rChged.GetFirstLeft(), true, true, true); + + // Stash footer formats as needed. + const SwFormatFooter& rLeftFoot = rChged.GetLeft().GetFooter(); + const SwFormatFooter& rFirstMasterFoot = rChged.GetFirstMaster().GetFooter(); + const SwFormatFooter& rFirstLeftFoot = rChged.GetFirstLeft().GetFooter(); + const bool bStashLeftFoot = !rDesc.IsFooterShared() && rChged.IsFooterShared(); + const bool bStashFirstMasterFoot = !rDesc.IsFirstShared() && rChged.IsFirstShared(); + const bool bStashFirstLeftFoot = (!rDesc.IsFooterShared() && rChged.IsFooterShared()) || (!rDesc.IsFirstShared() && rChged.IsFirstShared()); + if (bStashLeftFoot && rLeftFoot.GetRegisteredIn()) + rDesc.StashFrameFormat(rChged.GetLeft(), false, true, false); + if (bStashFirstMasterFoot && rFirstMasterFoot.GetRegisteredIn()) + rDesc.StashFrameFormat(rChged.GetFirstMaster(), false, false, true); + if (bStashFirstLeftFoot && rFirstLeftFoot.GetRegisteredIn()) + rDesc.StashFrameFormat(rChged.GetFirstLeft(), false, true, true); + + GetIDocumentUndoRedo().AppendUndo(std::make_unique<SwUndoPageDesc>(rDesc, rChged, this)); } ::sw::UndoGuard const undoGuard(GetIDocumentUndoRedo()); @@ -429,24 +456,8 @@ void SwDoc::ChgPageDesc( size_t i, const SwPageDesc &rChged ) // Take over orientation rDesc.SetLandscape( rChged.GetLandscape() ); - // #i46909# no undo if header or footer changed - bool bHeaderFooterChanged = false; - // Synch header. const SwFormatHeader& rMasterHead = rChged.GetMaster().GetHeader(); - const SwFormatHeader& rLeftHead = rChged.GetLeft().GetHeader(); - const SwFormatHeader& rFirstMasterHead = rChged.GetFirstMaster().GetHeader(); - const SwFormatHeader& rFirstLeftHead = rChged.GetFirstLeft().GetHeader(); - if (undoGuard.UndoWasEnabled()) - { - // #i46909# no undo if header or footer changed - // Did something change in the nodes? - const SwFormatHeader &rOldMasterHead = rDesc.GetMaster().GetHeader(); - bHeaderFooterChanged |= - ( rMasterHead.IsActive() != rOldMasterHead.IsActive() || - rChged.IsHeaderShared() != rDesc.IsHeaderShared() || - rChged.IsFirstShared() != rDesc.IsFirstShared() ); - } rDesc.GetMaster().SetFormatAttr( rMasterHead ); const bool bRestoreStashedLeftHead = rDesc.IsHeaderShared() && !rChged.IsHeaderShared(); const bool bRestoreStashedFirstMasterHead = rDesc.IsFirstShared() && !rChged.IsFirstShared(); @@ -458,33 +469,10 @@ void SwDoc::ChgPageDesc( size_t i, const SwPageDesc &rChged ) CopyMasterHeader(rChged, pStashedFirstMasterFormat ? pStashedFirstMasterFormat->GetHeader() : rMasterHead, rDesc, false, true); // Copy first master CopyMasterHeader(rChged, pStashedFirstLeftFormat ? pStashedFirstLeftFormat->GetHeader() : rMasterHead, rDesc, true, true); // Copy first left - // Stash header formats as needed. - const bool bStashLeftHead = !rDesc.IsHeaderShared() && rChged.IsHeaderShared(); - const bool bStashFirstMasterHead = !rDesc.IsFirstShared() && rChged.IsFirstShared(); - const bool bStashFirstLeftHead = (!rDesc.IsHeaderShared() && rChged.IsHeaderShared()) || (!rDesc.IsFirstShared() && rChged.IsFirstShared()); - if (bStashLeftHead && rLeftHead.GetRegisteredIn()) - rDesc.StashFrameFormat(rChged.GetLeft(), true, true, false); - if (bStashFirstMasterHead && rFirstMasterHead.GetRegisteredIn()) - rDesc.StashFrameFormat(rChged.GetFirstMaster(), true, false, true); - if (bStashFirstLeftHead && rFirstLeftHead.GetRegisteredIn()) - rDesc.StashFrameFormat(rChged.GetFirstLeft(), true, true, true); - rDesc.ChgHeaderShare( rChged.IsHeaderShared() ); // Synch Footer. const SwFormatFooter& rMasterFoot = rChged.GetMaster().GetFooter(); - const SwFormatFooter& rLeftFoot = rChged.GetLeft().GetFooter(); - const SwFormatFooter& rFirstMasterFoot = rChged.GetFirstMaster().GetFooter(); - const SwFormatFooter& rFirstLeftFoot = rChged.GetFirstLeft().GetFooter(); - if (undoGuard.UndoWasEnabled()) - { - // #i46909# no undo if header or footer changed - // Did something change in the Nodes? - const SwFormatFooter &rOldMasterFoot = rDesc.GetMaster().GetFooter(); - bHeaderFooterChanged |= - ( rMasterFoot.IsActive() != rOldMasterFoot.IsActive() || - rChged.IsFooterShared() != rDesc.IsFooterShared() ); - } rDesc.GetMaster().SetFormatAttr( rMasterFoot ); const bool bRestoreStashedLeftFoot = rDesc.IsFooterShared() && !rChged.IsFooterShared(); const bool bRestoreStashedFirstMasterFoot = rDesc.IsFirstShared() && !rChged.IsFirstShared(); @@ -496,17 +484,6 @@ void SwDoc::ChgPageDesc( size_t i, const SwPageDesc &rChged ) CopyMasterFooter(rChged, pStashedFirstMasterFoot ? pStashedFirstMasterFoot->GetFooter() : rMasterFoot, rDesc, false, true); // Copy first master CopyMasterFooter(rChged, pStashedFirstLeftFoot ? pStashedFirstLeftFoot->GetFooter() : rMasterFoot, rDesc, true, true); // Copy first left - // Stash footer formats as needed. - const bool bStashLeftFoot = !rDesc.IsFooterShared() && rChged.IsFooterShared(); - const bool bStashFirstMasterFoot = !rDesc.IsFirstShared() && rChged.IsFirstShared(); - const bool bStashFirstLeftFoot = (!rDesc.IsFooterShared() && rChged.IsFooterShared()) || (!rDesc.IsFirstShared() && rChged.IsFirstShared()); - if (bStashLeftFoot && rLeftFoot.GetRegisteredIn()) - rDesc.StashFrameFormat(rChged.GetLeft(), false, true, false); - if (bStashFirstMasterFoot && rFirstMasterFoot.GetRegisteredIn()) - rDesc.StashFrameFormat(rChged.GetFirstMaster(), false, false, true); - if (bStashFirstLeftFoot && rFirstLeftFoot.GetRegisteredIn()) - rDesc.StashFrameFormat(rChged.GetFirstLeft(), false, true, true); - rDesc.ChgFooterShare( rChged.IsFooterShared() ); // there is just one first shared flag for both header and footer? rDesc.ChgFirstShare( rChged.IsFirstShared() ); @@ -567,12 +544,6 @@ void SwDoc::ChgPageDesc( size_t i, const SwPageDesc &rChged ) } getIDocumentState().SetModified(); - // #i46909# no undo if header or footer changed - if( bHeaderFooterChanged ) - { - GetIDocumentUndoRedo().DelAllUndoObj(); - } - SfxBindings* pBindings = ( GetDocShell() && GetDocShell()->GetDispatcher() ) ? GetDocShell()->GetDispatcher()->GetBindings() : nullptr; if ( pBindings ) diff --git a/sw/source/core/layout/pagedesc.cxx b/sw/source/core/layout/pagedesc.cxx index 97c714e245d5..7013bf87aede 100644 --- a/sw/source/core/layout/pagedesc.cxx +++ b/sw/source/core/layout/pagedesc.cxx @@ -430,33 +430,44 @@ void SwPageDesc::StashFrameFormat(const SwFrameFormat& rFormat, bool bHeader, bo pFormat = &m_aStashedFooter.m_pStashedFirstLeft; } - if (!pFormat) + if (pFormat) { - SAL_WARN("sw", "Stashing the right page header/footer is pointless."); + *pFormat = std::make_shared<SwFrameFormat>(rFormat); } else { - *pFormat = std::make_shared<SwFrameFormat>(rFormat); + SAL_WARN("sw", "Stashing the right page header/footer is pointless."); } } const SwFrameFormat* SwPageDesc::GetStashedFrameFormat(bool bHeader, bool bLeft, bool bFirst) const { + std::shared_ptr<SwFrameFormat>* pFormat = nullptr; + if (bLeft && !bFirst) { - return bHeader ? m_aStashedHeader.m_pStashedLeft.get() : m_aStashedFooter.m_pStashedLeft.get(); + pFormat = bHeader ? &m_aStashedHeader.m_pStashedLeft : &m_aStashedFooter.m_pStashedLeft; } else if (!bLeft && bFirst) { - return bHeader ? m_aStashedHeader.m_pStashedFirst.get() : m_aStashedFooter.m_pStashedFirst.get(); + pFormat = bHeader ? &m_aStashedHeader.m_pStashedFirst : &m_aStashedFooter.m_pStashedFirst; } else if (bLeft && bFirst) { - return bHeader ? m_aStashedHeader.m_pStashedFirstLeft.get() : m_aStashedFooter.m_pStashedFirstLeft.get(); + pFormat = bHeader ? &m_aStashedHeader.m_pStashedFirstLeft : &m_aStashedFooter.m_pStashedFirstLeft; } - SAL_WARN("sw", "Right page format is never stashed."); - return nullptr; + if (pFormat) + { + const SwFrameFormat* retVal = pFormat->get(); + pFormat->reset(); + return retVal; + } + else + { + SAL_WARN("sw", "Right page format is never stashed."); + return nullptr; + } } // Page styles |