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>
diff --git a/sw/qa/extras/uiwriter/uiwriter.cxx b/sw/qa/extras/uiwriter/uiwriter.cxx
index c39659f..dae657a 100644
--- a/sw/qa/extras/uiwriter/uiwriter.cxx
+++ b/sw/qa/extras/uiwriter/uiwriter.cxx
@@ -2225,7 +2225,9 @@
// 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
new file mode 100644
index 0000000..c9f9942
--- /dev/null
+++ b/sw/qa/uitest/data/tdf46561.odt
Binary files differ
diff --git a/sw/qa/uitest/writer_tests7/tdf46561.py b/sw/qa/uitest/writer_tests7/tdf46561.py
new file mode 100644
index 0000000..2351365
--- /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 1c40970..9d10e4b 100644
--- a/sw/source/core/doc/docdesc.cxx
+++ b/sw/source/core/doc/docdesc.cxx
@@ -388,8 +388,35 @@
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 @@
// 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 @@
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 @@
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 @@
}
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 97c714e..7013bf8 100644
--- a/sw/source/core/layout/pagedesc.cxx
+++ b/sw/source/core/layout/pagedesc.cxx
@@ -430,33 +430,44 @@
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
