From 35a685a006e5571377c01a4e8398ecff024a841e Mon Sep 17 00:00:00 2001 From: Mike Kaganski Date: Thu, 16 Jul 2026 15:13:06 +0500 Subject: [PATCH] tdf#123974 tdf#150910 sc: keep the target's direct cell protection on paste A Ctrl+V changes a cell's content, not whether it is locked. Copying a protected cell onto a cell the user had directly unprotected used to carry the Protected flag along, so the target could no longer be edited, the opposite of what Excel does and what tdf#123974 asks for. Throughout this change "the cell's protection" means the protection set as direct formatting on the cell itself. Protection can also come from a cell style, and conditional formatting sets protection precisely by applying such a style; those two are one and the same mechanism and, as noted at the end, are deliberately left untouched here. The two reports in this area pull in opposite directions: tdf#123974 wants the direct flag dropped on paste, tdf#150910 wants it always retained, and they were noted as contradictory. A rule gated on sheet protection (drop the direct flag only when the sheet is protected and the target was unprotected) was considered and rejected, because its outcome depends on the order of otherwise unrelated steps: directly unprotecting a cell, pasting into it, then protecting the sheet would re-lock it, while protecting the sheet first and pasting after would not. That is impossible to explain to a user. Instead treat a cell's direct protection as a property of the cell that an interactive paste never changes. Snapshot the destination's directly set cell protection before the paste and restore it afterwards, so the pasted attributes overwrite everything except that lock state. Routing both the replicated-single-row fast path and the general path through the same restore also removes the single-row versus multi-row inconsistency of tdf#150910, which shared the same root cause. Only directly applied protection is preserved. Protection that instead comes from a cell style, including a style applied dynamically by conditional formatting, is not touched and keeps following the usual paste (in theory this could be extended to the effective protection if a need shows up). The programmatic copyRange path is left unchanged and still copies attributes verbatim; only an interactive paste opts in. A moved cell (cut and paste) keeps its own protection and is excluded. Signed-off-by: Mike Kaganski Change-Id: I3a1ec141c3ec698858ca8d058e6d9b1e9126402f Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/7069 Reviewed-by: Miklos Vajna Tested-by: Jenkins CPCI (cherry picked from commit 39f30bd4b532f7575957210b644216afa14ac28b) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/208344 Reviewed-by: Xisco Fauli Tested-by: Jenkins --- sc/inc/document.hxx | 3 +- sc/qa/unit/ucalc_copypaste.cxx | 87 ++++++++++++++++++++++++++++++++ sc/source/core/data/document.cxx | 53 ++++++++++++++++++- sc/source/ui/view/viewfun3.cxx | 6 ++- 4 files changed, 146 insertions(+), 3 deletions(-) diff --git a/sc/inc/document.hxx b/sc/inc/document.hxx index adc56cc76fc9..36ac9862a423 100644 --- a/sc/inc/document.hxx +++ b/sc/inc/document.hxx @@ -1900,7 +1900,8 @@ public: ScDocument* pRefUndoDoc, ScDocument* pClipDoc, bool bResetCut = true, bool bAsLink = false, bool bIncludeFiltered = true, bool bSkipEmptyCells = false, - const ScRangeList* pDestRanges = nullptr ); + const ScRangeList* pDestRanges = nullptr, + bool bPreserveDestProtection = false ); void CopyMultiRangeFromClip(const ScAddress& rDestPos, const ScMarkData& rMark, InsertDeleteFlags nInsFlag, ScDocument* pClipDoc, diff --git a/sc/qa/unit/ucalc_copypaste.cxx b/sc/qa/unit/ucalc_copypaste.cxx index b3875d7a2f73..7246bf796b49 100644 --- a/sc/qa/unit/ucalc_copypaste.cxx +++ b/sc/qa/unit/ucalc_copypaste.cxx @@ -31,6 +31,9 @@ #include #include #include +#include +#include +#include #include @@ -408,6 +411,90 @@ CPPUNIT_TEST_FIXTURE(TestCopyPaste, testCopyPaste) m_pDoc->DeleteTab(0); } +CPPUNIT_TEST_FIXTURE(TestCopyPaste, testCopyPasteKeepDestProtection) +{ + // tdf#123974, tdf#150910: an interactive paste changes a cell's content, + // not whether it is locked. Copying a protected cell onto a cell the user + // explicitly unprotected must leave it editable, and this must not depend + // on whether the source spans a single row (the replicated-single-row fast + // path) or several rows (the general path) - the two used to disagree. + m_pDoc->InsertTab(0, u"Sheet1"_ustr); + + // Source column A: two protected cells carrying values. + m_pDoc->SetValue(0, 0, 0, 1.0); + m_pDoc->SetValue(0, 1, 0, 2.0); + m_pDoc->ApplyAttr(0, 0, 0, ScProtectionAttr(true)); + m_pDoc->ApplyAttr(0, 1, 0, ScProtectionAttr(true)); + + // Targets the user explicitly unprotected by direct formatting: C1 (a + // single-row target) and D1:D2 (a multi-row target). + m_pDoc->ApplyAttr(2, 0, 0, ScProtectionAttr(false)); + m_pDoc->ApplyAttr(3, 0, 0, ScProtectionAttr(false)); + m_pDoc->ApplyAttr(3, 1, 0, ScProtectionAttr(false)); + + auto paste = [this](const ScRange& rSrc, const ScRange& rDest, bool bPreserveProtection) { + ScDocument aClipDoc(SCDOCMODE_CLIP); + copyToClip(m_pDoc, rSrc, &aClipDoc); + ScMarkData aMark(m_pDoc->GetSheetLimits()); + aMark.SetMarkArea(rDest); + m_pDoc->CopyFromClip(rDest, aMark, InsertDeleteFlags::ALL, nullptr, &aClipDoc, true, false, + true, false, nullptr, bPreserveProtection); + }; + + // Single-row source exercises the replicated-single-row fast path. + paste(ScRange(0, 0, 0, 0, 0, 0), ScRange(2, 0, 0, 2, 0, 0), /*bPreserveProtection*/ true); + CPPUNIT_ASSERT_EQUAL(1.0, m_pDoc->GetValue(2, 0, 0)); + CPPUNIT_ASSERT_MESSAGE("single-row paste must keep the target unprotected", + !m_pDoc->GetAttr(2, 0, 0, ATTR_PROTECTION).GetProtection()); + + // Multi-row source exercises the general path. + paste(ScRange(0, 0, 0, 0, 1, 0), ScRange(3, 0, 0, 3, 1, 0), /*bPreserveProtection*/ true); + CPPUNIT_ASSERT_EQUAL(1.0, m_pDoc->GetValue(3, 0, 0)); + CPPUNIT_ASSERT_EQUAL(2.0, m_pDoc->GetValue(3, 1, 0)); + CPPUNIT_ASSERT_MESSAGE("multi-row paste must keep the target unprotected (row 1)", + !m_pDoc->GetAttr(3, 0, 0, ATTR_PROTECTION).GetProtection()); + CPPUNIT_ASSERT_MESSAGE("multi-row paste must keep the target unprotected (row 2)", + !m_pDoc->GetAttr(3, 1, 0, ATTR_PROTECTION).GetProtection()); + + // The whole protection item is preserved, not just its Protected bit: an + // unprotected source pasted onto a protected + hidden-formula target keeps + // the target's own settings. + m_pDoc->SetValue(0, 5, 0, 9.0); // A6, unprotected source + m_pDoc->ApplyAttr(0, 5, 0, ScProtectionAttr(false)); + m_pDoc->ApplyAttr(2, 5, 0, ScProtectionAttr(true, /*bHideFormula*/ true)); // C6 target + paste(ScRange(0, 5, 0, 0, 5, 0), ScRange(2, 5, 0, 2, 5, 0), /*bPreserveProtection*/ true); + const ScProtectionAttr& rC6 = m_pDoc->GetAttr(2, 5, 0, ATTR_PROTECTION); + CPPUNIT_ASSERT_MESSAGE("paste must keep the target protected", rC6.GetProtection()); + CPPUNIT_ASSERT_MESSAGE("paste must keep the target's hide-formula flag", rC6.GetHideFormula()); + + // Without the flag (the programmatic copyRange path) attributes are copied + // verbatim, so the source's protection is carried over as before. + m_pDoc->ApplyAttr(4, 5, 0, ScProtectionAttr(true)); // E6 target, protected + paste(ScRange(0, 5, 0, 0, 5, 0), ScRange(4, 5, 0, 4, 5, 0), /*bPreserveProtection*/ false); + CPPUNIT_ASSERT_MESSAGE("paste without the flag must copy the source's state", + !m_pDoc->GetAttr(4, 5, 0, ATTR_PROTECTION).GetProtection()); + + // Protection that comes from a cell style, not from direct formatting, is + // not preserved: only direct protection is. Give G6 an unprotected cell + // style (no direct protection) and paste the protected A1 onto it - it must + // end up protected, following the paste like any other styled attribute. + ScStyleSheet& rUnprotectedStyle = static_cast( + m_pDoc->GetStyleSheetPool()->Make(u"Unprotected"_ustr, SfxStyleFamily::Para)); + rUnprotectedStyle.GetItemSet().Put(ScProtectionAttr(false)); + m_pDoc->ApplyStyle(6, 5, 0, rUnprotectedStyle); // G6 + CPPUNIT_ASSERT_MESSAGE("precondition: the style leaves G6 unprotected", + !m_pDoc->GetAttr(6, 5, 0, ATTR_PROTECTION).GetProtection()); + CPPUNIT_ASSERT_MESSAGE( + "precondition: G6 has no direct protection", + SfxItemState::SET + != m_pDoc->GetPattern(6, 5, 0)->GetItemSet().GetItemState(ATTR_PROTECTION, false)); + paste(ScRange(0, 0, 0, 0, 0, 0), ScRange(6, 5, 0, 6, 5, 0), /*bPreserveProtection*/ true); + CPPUNIT_ASSERT_MESSAGE("style-based protection is not preserved; it follows the paste", + m_pDoc->GetAttr(6, 5, 0, ATTR_PROTECTION).GetProtection()); + + m_pDoc->DeleteTab(0); +} + CPPUNIT_TEST_FIXTURE(TestCopyPaste, testCopyPasteAsLink) { sc::AutoCalcSwitch aACSwitch(*m_pDoc, true); // Turn on auto calc. diff --git a/sc/source/core/data/document.cxx b/sc/source/core/data/document.cxx index 34332513da13..7369a1fb9959 100644 --- a/sc/source/core/data/document.cxx +++ b/sc/source/core/data/document.cxx @@ -3048,7 +3048,7 @@ void ScDocument::CopyFromClip( const ScRange& rDestRange, const ScMarkData& rMark, InsertDeleteFlags nInsFlag, ScDocument* pRefUndoDoc, ScDocument* pClipDoc, bool bResetCut, bool bAsLink, bool bIncludeFiltered, bool bSkipEmptyCells, - const ScRangeList * pDestRanges ) + const ScRangeList * pDestRanges, bool bPreserveDestProtection ) { if (bIsClip) return; @@ -3141,6 +3141,48 @@ void ScDocument::CopyFromClip( sc::ColumnSpanSet aBroadcastSpans; + // An interactive paste treats a cell's own directly applied protection as + // a property of the cell, not of the pasted data: copying a protected cell + // onto a cell the user directly unprotected must leave it editable (and + // vice versa). Note the directly set protection over the destination now, + // and put it back once the pasted attributes have overwritten it below. + // Only direct formatting is handled here; protection that instead comes + // from a cell style (including a style that conditional formatting applies) + // is left to follow the usual paste. A moved cell (cut and paste) keeps its + // own protection, so it is excluded. + struct KeptProtection + { + SCTAB nTab; + SCCOL nCol; + SCROW nRow1; + SCROW nRow2; + ScProtectionAttr aProtection; + }; + std::vector aKeptProtection; + const bool bKeepDestProtection = bPreserveDestProtection + && (nInsFlag & InsertDeleteFlags::ATTRIB) + && !pClipDoc->GetClipParam().mbCutMode; + if (bKeepDestProtection) + { + for (const SCTAB nTab : rMark) + { + for (const ScRange& rRange : *pDestRanges) + { + ScDocAttrIterator aIter(*this, nTab, rRange.aStart.Col(), rRange.aStart.Row(), + rRange.aEnd.Col(), rRange.aEnd.Row()); + SCCOL nAttrCol; + SCROW nAttrRow1, nAttrRow2; + while (const ScPatternAttr* pPattern = aIter.GetNext(nAttrCol, nAttrRow1, nAttrRow2)) + { + if (const ScProtectionAttr* pProtect + = pPattern->GetItemSet().GetItemIfSet(ATTR_PROTECTION, false)) + aKeptProtection.push_back( + { nTab, nAttrCol, nAttrRow1, nAttrRow2, *pProtect }); + } + } + } + } + SCCOL nClipStartCol = aClipRange.aStart.Col(); SCROW nClipStartRow = aClipRange.aStart.Row(); SCROW nClipEndRow = aClipRange.aEnd.Row(); @@ -3249,6 +3291,15 @@ void ScDocument::CopyFromClip( bInsertingFromOtherDoc = false; + // Put back the destination's own direct protection that the paste above + // overwrote (see the note where aKeptProtection is filled). + for (const KeptProtection& rKept : aKeptProtection) + { + ScPatternAttr aPattern(getCellAttributeHelper()); + aPattern.ItemSetPut(rKept.aProtection); + ApplyPatternAreaTab(rKept.nCol, rKept.nRow1, rKept.nCol, rKept.nRow2, rKept.nTab, aPattern); + } + if (nInsFlag & InsertDeleteFlags::CONTENTS) { for (SCTAB nTab : rMark) diff --git a/sc/source/ui/view/viewfun3.cxx b/sc/source/ui/view/viewfun3.cxx index e37dc3e1a70a..c33e57befaa5 100644 --- a/sc/source/ui/view/viewfun3.cxx +++ b/sc/source/ui/view/viewfun3.cxx @@ -1512,9 +1512,13 @@ bool ScViewFunc::PasteFromClip( InsertDeleteFlags nFlags, ScDocument* pClipDoc, if (!bAsLink) { // copy normally (original range) + // keep the destination cells' own directly applied protection - an + // interactive paste changes their content, not whether the user has + // directly locked them (tdf#123974) rDoc.CopyFromClip( aUserRange, aFilteredMark, nNoObjFlags, pRefUndoDoc.get(), pClipDoc, true, false, bIncludeFiltered, - bSkipEmptyCells, (bMarkIsFiltered ? &aRangeList : nullptr) ); + bSkipEmptyCells, (bMarkIsFiltered ? &aRangeList : nullptr), + /*bPreserveDestProtection*/true ); // adapt refs manually in case of transpose if ( bTranspose && bCutMode && (nFlags & InsertDeleteFlags::CONTENTS) )