ODFstudio/odfstudio
2
0
Fork 0
Code Issues Packages Activity

tdf#123974 tdf#150910 sc: keep the target's direct cell protection on paste

Browse Source 
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 <mike.kaganski@collabora.com>
Change-Id: I3a1ec141c3ec698858ca8d058e6d9b1e9126402f
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/7069
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins CPCI <releng@collaboraoffice.com>
(cherry picked from commit 39f30bd4b532f7575957210b644216afa14ac28b)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/208344
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
Tested-by: Jenkins
This commit is contained in:
Mike Kaganski
2026-07-22 14:01:57 +02:00
committed by Xisco Fauli
parent d3cf169ea1
commit 35a685a006
4 changed files with 146 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
sc/inc/document.hxx
+2 -1
View File
@@ -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,
sc/qa/unit/ucalc_copypaste.cxx
+87
View File
@@ -31,6 +31,9 @@
#include <refundo.hxx>
#include <scitems.hxx>
#include <scopetools.hxx>
#include <stlpool.hxx>
#include <stlsheet.hxx>
#include <svl/style.hxx>
#include <sfx2/docfile.hxx>
@@ -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<ScStyleSheet&>(
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.
sc/source/core/data/document.cxx
+52 -1
View File
@@ -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<KeptProtection> 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)
sc/source/ui/view/viewfun3.cxx
+5 -1
View File
@@ -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) )