mir/libreoffice
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

cool#11226 sw per-view redline on: fix bad record mode change on undo

Browse Source 
Have 2 Writer views, enable recording for "this view", type a few
characters, undo. The recording is now enabled for all views, not just
this view.

The problem is that sw::DocumentRedlineManager::SetRedlineFlags_intern()
only gets flags, so in case UndoRedoRedlineGuard sets new temporary
flags & then restores those flags later, then that guard assumes that we
return to the old state, but it implicitly calls
SetRedlineFlags_intern() with 'bRecordAllViews = true', so the record
mode changes from "this view" to "all views" at the end.

Fix the problem by changing the RecordAllViews boolean to an enum and
introducing a "view agnostic" mode: here we don't care too much about
views, since these calls are typically just temporary, but what we care
is that 1) if they enable recording, then IsRedlineOn() reports true and
2) if they set new flags & later restore the old flags, the state is
unchanged.

Alternatives considered:
- It would be possible to just touch the model in the "view agnostic"
  case, but then IsRedlineOn() would still get the recording bool from
  the view, leading to a stack overflow: lcl_DoWithBreaks() ->
  sw::DocumentContentOperationsManager::DeleteAndJoinWithRedlineImpl() ->
  sw::DocumentRedlineManager::AppendRedline() ->
  sw::DocumentContentOperationsManager::DeleteAndJoin() ->
  lcl_DoWithBreaks() again.
- It would be possible to fix this on the undo side (to query the record
  mode and restore that mode explicitly when restoring flags), but we
  have many other use-cases for setting & restoring flags: e.g.
  autocorrect or autoformat.

Change-Id: I75697c6b5b3767ad8db899cda080be312eb6c821
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/183792
Tested-by: Jenkins
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
This commit is contained in:
Miklos Vajna 2025-04-07 09:12:57 +02:00
parent eee10ae0c8
commit 94e683b5e1
16 changed files with 105 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
include/sfx2/objsh.hxx
View File

@ -44,6 +44,7 @@
#include <o3tl/typed_flags_set.hxx>
#include <functional>
#include <sfx2/AccessibilityIssue.hxx>
#include <sfx2/redlinerecordingmode.hxx>
#include <unotools/ucbstreamhelper.hxx>
@ -696,7 +697,7 @@ public:
// slots used for Calc: FID_CHG_RECORD, SID_CHG_PROTECT
virtual bool IsChangeRecording(SfxViewShell* pViewShell = nullptr, bool bRecordAllViews = true) const;
virtual bool HasChangeRecordProtection() const;
virtual void SetChangeRecording( bool bActivate, bool bLockAllViews = false, bool bRecordAllViews = true );
virtual void SetChangeRecording( bool bActivate, bool bLockAllViews = false, SfxRedlineRecordingMode eRedlineRecordingMode = SfxRedlineRecordingMode::ViewAgnostic );
virtual void SetProtectionPassword( const OUString &rPassword );
virtual bool GetProtectionHash( /*out*/ css::uno::Sequence< sal_Int8 > &rPasswordHash );

24
include/sfx2/redlinerecordingmode.hxx Normal file
View File

@ -0,0 +1,24 @@
/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
/*
* This file is part of the LibreOffice project.
*
* 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/.
*
*/
#pragma once
/// Decides how new redline flags affect the views
enum class SfxRedlineRecordingMode
{
/// Model and current view only: to set new flags & restore old ones later
ViewAgnostic,
/// Have recording on in this view
ThisView,
/// Have recording on in all views
AllViews,
};
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */

2
sc/source/ui/docshell/docsh.cxx
View File

@ -3409,7 +3409,7 @@ bool ScDocShell::HasChangeRecordProtection() const
return bRes;
}
void ScDocShell::SetChangeRecording( bool bActivate, bool /*bLockAllViews*/, bool /*bRecordAllViews*/ )
void ScDocShell::SetChangeRecording( bool bActivate, bool /*bLockAllViews*/, SfxRedlineRecordingMode /*eRedlineRecordingMode*/)
{
bool bOldChangeRecording = IsChangeRecording();

2
sc/source/ui/inc/docsh.hxx
View File

@ -427,7 +427,7 @@ public:
// see also: FID_CHG_RECORD, SID_CHG_PROTECT
virtual bool IsChangeRecording(SfxViewShell* pViewShell = nullptr, bool bRecordAllViews = true) const override;
virtual bool HasChangeRecordProtection() const override;
virtual void SetChangeRecording( bool bActivate, bool bLockAllViews = false, bool bRecordAllViews = true ) override;
virtual void SetChangeRecording( bool bActivate, bool bLockAllViews = false, SfxRedlineRecordingMode eRedlineRecordingMode = SfxRedlineRecordingMode::ViewAgnostic) override;
virtual void SetProtectionPassword( const OUString &rPassword ) override;
virtual bool GetProtectionHash( /*out*/ css::uno::Sequence< sal_Int8 > &rPasswordHash ) override;

2
sfx2/source/doc/objxtor.cxx
View File

@ -1140,7 +1140,7 @@ bool SfxObjectShell::HasChangeRecordProtection() const
}
void SfxObjectShell::SetChangeRecording( bool /*bActivate*/, bool /*bLockAllViews*/, bool /*bRecordAllViews*/ )
void SfxObjectShell::SetChangeRecording( bool /*bActivate*/, bool /*bLockAllViews*/, SfxRedlineRecordingMode /*eRedlineRecordingMode*/)
{
// currently this function needs to be overwritten by Writer and Calc only
SAL_WARN( "sfx.doc", "function not implemented" );

5
sw/inc/IDocumentRedlineAccess.hxx
View File

@ -28,6 +28,7 @@
#include <com/sun/star/uno/Sequence.h>
#include <o3tl/typed_flags_set.hxx>
#include <svx/ctredlin.hxx>
#include <sfx2/redlinerecordingmode.hxx>
#include "docary.hxx"
@ -107,14 +108,14 @@ public:
@param eMode
[in] the new redline mode.
*/
virtual void SetRedlineFlags_intern(/*[in]*/RedlineFlags eMode, bool bRecordAllViews = true, bool bRecordModeChange = false) = 0;
virtual void SetRedlineFlags_intern(/*[in]*/RedlineFlags eMode, SfxRedlineRecordingMode eRedlineRecordingMode = SfxRedlineRecordingMode::ViewAgnostic, bool bRecordModeChange = false) = 0;
/** Set a new redline mode.
@param eMode
[in] the new redline mode.
*/
virtual void SetRedlineFlags(/*[in]*/RedlineFlags eMode, bool bRecordAllViews = true, bool bRecordModeChange = false) = 0;
virtual void SetRedlineFlags(/*[in]*/RedlineFlags eMode, SfxRedlineRecordingMode eRedlineRecordingMode = SfxRedlineRecordingMode::ViewAgnostic, bool bRecordModeChange = false) = 0;
/** Query if redlining is on.

2
sw/inc/docsh.hxx
View File

@ -323,7 +323,7 @@ public:
see also: FN_REDLINE_ON, FN_REDLINE_ON */
virtual bool IsChangeRecording(SfxViewShell* pViewShell = nullptr, bool bRecordAllViews = true) const override;
virtual bool HasChangeRecordProtection() const override;
virtual void SetChangeRecording( bool bActivate, bool bLockAllViews = false, bool bRecordAllViews = true ) override;
virtual void SetChangeRecording( bool bActivate, bool bLockAllViews = false, SfxRedlineRecordingMode eRedlineRecordingMode = SfxRedlineRecordingMode::ViewAgnostic) override;
virtual void SetProtectionPassword( const OUString &rPassword ) override;
virtual bool GetProtectionHash( /*out*/ css::uno::Sequence< sal_Int8 > &rPasswordHash ) override;

4
sw/inc/editsh.hxx
View File

@ -19,6 +19,8 @@
#ifndef INCLUDED_SW_INC_EDITSH_HXX
#define INCLUDED_SW_INC_EDITSH_HXX
#include <sfx2/redlinerecordingmode.hxx>
#include "crsrsh.hxx"
#include "charfmt.hxx"
@ -953,7 +955,7 @@ public:
/// For Redlining.
SW_DLLPUBLIC RedlineFlags GetRedlineFlags() const;
SW_DLLPUBLIC void SetRedlineFlags( RedlineFlags eMode, bool bRecordAllViews = true );
SW_DLLPUBLIC void SetRedlineFlags( RedlineFlags eMode, SfxRedlineRecordingMode eRedlineRecordingMode = SfxRedlineRecordingMode::ViewAgnostic );
bool IsRedlineOn() const;
SW_DLLPUBLIC SwRedlineTable::size_type GetRedlineCount() const;
const SwRangeRedline& GetRedline( SwRedlineTable::size_type nPos ) const;

27
sw/qa/extras/tiledrendering/tiledrendering2.cxx
View File

@ -684,6 +684,33 @@ CPPUNIT_TEST_FIXTURE(SwTiledRenderingTest, testControlCodesCursor)
// cursor when showing formatting marks.
CPPUNIT_ASSERT(!aView1.m_bCursorVisible);
}
CPPUNIT_TEST_FIXTURE(SwTiledRenderingTest, testTrackChangesInsertUndo)
{
// Given a document with 2 views, first view enables redline record for that view only:
SwXTextDocument* pXTextDocument = createDoc();
CPPUNIT_ASSERT(pXTextDocument);
SwTestViewCallback aView1;
int nView1 = SfxLokHelper::getView();
SwView* pView1 = pXTextDocument->GetDocShell()->GetView();
SwWrtShell* pWrtShell1 = pXTextDocument->GetDocShell()->GetWrtShell();
SfxLokHelper::createView();
SwTestViewCallback aView2;
SfxLokHelper::setView(nView1);
comphelper::dispatchCommand(".uno:TrackChangesInThisView", {});
// When typing and undoing:
pWrtShell1->Insert(u"A"_ustr);
pWrtShell1->Undo();
// Then make sure the record mode doesn't change:
std::unique_ptr<SfxPoolItem> pItem;
pView1->GetViewFrame().GetBindings().QueryState(FN_TRACK_CHANGES_IN_THIS_VIEW, pItem);
CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt16>(FN_TRACK_CHANGES_IN_THIS_VIEW), pItem->Which());
// Without the accompanying fix in place, this test would have failed, undo changed "this view"
// record mode to "all views", which is unexpected.
CPPUNIT_ASSERT(dynamic_cast<SfxBoolItem*>(pItem.get())->GetValue());
}
}
CPPUNIT_PLUGIN_IMPLEMENT();

21
sw/source/core/doc/DocumentRedlineManager.cxx
View File

@ -1171,7 +1171,7 @@ RedlineFlags DocumentRedlineManager::GetRedlineFlags(const SwViewShell* pViewShe
return eRedlineFlags;
}
void DocumentRedlineManager::SetRedlineFlags( RedlineFlags eMode, bool bRecordAllViews, bool bRecordModeChange )
void DocumentRedlineManager::SetRedlineFlags( RedlineFlags eMode, SfxRedlineRecordingMode eRedlineRecordingMode, bool bRecordModeChange )
{
if( GetRedlineFlags() == eMode && !bRecordModeChange )
return;
@ -1244,7 +1244,7 @@ void DocumentRedlineManager::SetRedlineFlags( RedlineFlags eMode, bool bRecordAl
m_rDoc.SetInXMLImport( bSaveInXMLImportFlag );
}
SetRedlineFlags_intern(eMode, bRecordAllViews, bRecordModeChange);
SetRedlineFlags_intern(eMode, eRedlineRecordingMode, bRecordModeChange);
m_rDoc.getIDocumentState().SetModified();
// #TODO - add 'SwExtraRedlineTable' also ?
@ -1260,12 +1260,25 @@ bool DocumentRedlineManager::IsIgnoreRedline() const
return bool(RedlineFlags::Ignore & GetRedlineFlags());
}
void DocumentRedlineManager::SetRedlineFlags_intern(RedlineFlags eMode, bool bRecordAllViews, bool bRecordModeChange)
void DocumentRedlineManager::SetRedlineFlags_intern(RedlineFlags eMode, SfxRedlineRecordingMode eRedlineRecordingMode, bool bRecordModeChange)
{
SwDocShell* pDocShell = m_rDoc.GetDocShell();
SwViewShell* pViewShell = pDocShell ? pDocShell->GetWrtShell() : nullptr;
if (pViewShell)
if (pViewShell && eRedlineRecordingMode == SfxRedlineRecordingMode::ViewAgnostic)
{
// Just set the requested flags on the model and on the current view, so setting flags &
// restoring them result in the same state (no matter if that was this-view or all-views).
auto bRedlineRecordingOn = bool(eMode & RedlineFlags::On);
SwViewOption aOpt(*pViewShell->GetViewOptions());
if (aOpt.IsRedlineRecordingOn() != bRedlineRecordingOn)
{
aOpt.SetRedlineRecordingOn(bRedlineRecordingOn);
pViewShell->ApplyViewOptions(aOpt);
}
}
else if (pViewShell)
{
bool bRecordAllViews = eRedlineRecordingMode == SfxRedlineRecordingMode::AllViews;
// Recording may be per-view, the rest is per-document.
for(SwViewShell& rSh : pViewShell->GetRingContainer())
{

15
sw/source/core/edit/edredln.cxx
View File

@ -34,16 +34,21 @@ RedlineFlags SwEditShell::GetRedlineFlags() const
return GetDoc()->getIDocumentRedlineAccess().GetRedlineFlags(this);
}
void SwEditShell::SetRedlineFlags( RedlineFlags eMode, bool bRecordAllViews )
void SwEditShell::SetRedlineFlags( RedlineFlags eMode, SfxRedlineRecordingMode eRedlineRecordingMode)
{
if (SwDocShell* pDocSh = GetDoc()->GetDocShell())
{
bool bRecordModeChange = bRecordAllViews != pDocSh->IsChangeRecording(nullptr, bRecordAllViews);
bool bRecordModeChange = false;
if (eRedlineRecordingMode != SfxRedlineRecordingMode::ViewAgnostic)
{
bool bRecordAllViews = eRedlineRecordingMode == SfxRedlineRecordingMode::AllViews;
bRecordModeChange = bRecordAllViews != pDocSh->IsChangeRecording(nullptr, bRecordAllViews);
}
if( eMode != GetDoc()->getIDocumentRedlineAccess().GetRedlineFlags() || bRecordModeChange )
{
CurrShell aCurr( this );
StartAllAction();
GetDoc()->getIDocumentRedlineAccess().SetRedlineFlags( eMode, bRecordAllViews, bRecordModeChange );
GetDoc()->getIDocumentRedlineAccess().SetRedlineFlags( eMode, eRedlineRecordingMode, bRecordModeChange );
EndAllAction();
}
}
@ -135,7 +140,7 @@ void SwEditShell::ReinstateRedline(SwRedlineTable::size_type nPos)
if (!IsRedlineOn())
{
RedlineFlags nMode = GetRedlineFlags();
SetRedlineFlags(nMode | RedlineFlags::On, /*bRecordAllViews=*/false);
SetRedlineFlags(nMode | RedlineFlags::On, SfxRedlineRecordingMode::ThisView);
}
SwRangeRedline& rRedline = GetRedline(nPos);
@ -237,7 +242,7 @@ void SwEditShell::ReinstateRedlinesInSelection()
if (!IsRedlineOn())
{
RedlineFlags nMode = GetRedlineFlags();
SetRedlineFlags(nMode | RedlineFlags::On, /*bRecordAllViews=*/false);
SetRedlineFlags(nMode | RedlineFlags::On, SfxRedlineRecordingMode::ThisView);
}
SwPosition aCursorStart(*GetCursor()->Start());

4
sw/source/core/inc/DocumentRedlineManager.hxx
View File

@ -38,9 +38,9 @@ public:
*/
virtual RedlineFlags GetRedlineFlags(const SwViewShell* pViewShell = nullptr) const override;
virtual void SetRedlineFlags_intern(/*[in]*/RedlineFlags eMode, bool bRecordAllViews = true, bool bRecordModeChange = false) override;
virtual void SetRedlineFlags_intern(/*[in]*/RedlineFlags eMode, SfxRedlineRecordingMode eRedlineRecordingMode = SfxRedlineRecordingMode::ViewAgnostic, bool bRecordModeChange = false) override;
virtual void SetRedlineFlags(/*[in]*/RedlineFlags eMode, bool bRecordAllViews = true, bool bRecordModeChange = false) override;
virtual void SetRedlineFlags(/*[in]*/RedlineFlags eMode, SfxRedlineRecordingMode eRedlineRecordingMode = SfxRedlineRecordingMode::ViewAgnostic, bool bRecordModeChange = false) override;
virtual bool IsRedlineOn() const override;

6
sw/source/uibase/app/docsh.cxx
View File

@ -1399,7 +1399,7 @@ bool SwDocShell::HasChangeRecordProtection() const
return m_pWrtShell->getIDocumentRedlineAccess().GetRedlinePassword().hasElements();
}
void SwDocShell::SetChangeRecording( bool bActivate, bool bLockAllViews, bool bRecordAllViews )
void SwDocShell::SetChangeRecording( bool bActivate, bool bLockAllViews, SfxRedlineRecordingMode eRedlineRecordingMode)
{
RedlineFlags nOn = bActivate ? RedlineFlags::On : RedlineFlags::NONE;
RedlineFlags nMode = m_pWrtShell->GetRedlineFlags();
@ -1407,11 +1407,11 @@ void SwDocShell::SetChangeRecording( bool bActivate, bool bLockAllViews, bool bR
{
// tdf#107870: prevent jumping to cursor
auto aViewGuard(LockAllViews());
m_pWrtShell->SetRedlineFlagsAndCheckInsMode( (nMode & ~RedlineFlags::On) | nOn, bRecordAllViews );
m_pWrtShell->SetRedlineFlagsAndCheckInsMode( (nMode & ~RedlineFlags::On) | nOn, eRedlineRecordingMode );
}
else
{
m_pWrtShell->SetRedlineFlagsAndCheckInsMode( (nMode & ~RedlineFlags::On) | nOn, bRecordAllViews );
m_pWrtShell->SetRedlineFlagsAndCheckInsMode( (nMode & ~RedlineFlags::On) | nOn, eRedlineRecordingMode );
}
}

3
sw/source/uibase/inc/wrtsh.hxx
View File

@ -29,6 +29,7 @@
#include <o3tl/typed_flags_set.hxx>
#include <svx/swframetypes.hxx>
#include <vcl/weld.hxx>
#include <sfx2/redlinerecordingmode.hxx>
#include <doc.hxx>
#include <docsh.hxx>
@ -179,7 +180,7 @@ public:
void SetInsMode( bool bOn = true );
void ToggleInsMode() { SetInsMode( !m_bIns ); }
bool IsInsMode() const { return m_bIns; }
SW_DLLPUBLIC void SetRedlineFlagsAndCheckInsMode( RedlineFlags eMode, bool bRecordAllViews = true );
SW_DLLPUBLIC void SetRedlineFlagsAndCheckInsMode( RedlineFlags eMode, SfxRedlineRecordingMode eRedlineRecordingMode = SfxRedlineRecordingMode::ViewAgnostic);
SW_DLLPUBLIC void EnterSelFrameMode(const Point *pStartDrag = nullptr);
SW_DLLPUBLIC void LeaveSelFrameMode();

8
sw/source/uibase/uiview/view2.cxx
View File

@ -807,8 +807,12 @@ void SwView::Execute(SfxRequest &rReq)
}
SwDocShell* pDocShell = GetDocShell();
bool bRecordAllViews = nSlot != FN_TRACK_CHANGES_IN_THIS_VIEW;
pDocShell->SetChangeRecording( oOn.value(), /*bLockAllViews=*/true, bRecordAllViews );
SfxRedlineRecordingMode eRedlineRecordingMode = SfxRedlineRecordingMode::AllViews;
if (nSlot == FN_TRACK_CHANGES_IN_THIS_VIEW)
{
eRedlineRecordingMode = SfxRedlineRecordingMode::ThisView;
}
pDocShell->SetChangeRecording( oOn.value(), /*bLockAllViews=*/true, eRedlineRecordingMode );
// Notify all view shells of this document, as the track changes mode is document-global.
for (SfxViewFrame* pViewFrame = SfxViewFrame::GetFirst(pDocShell); pViewFrame; pViewFrame = SfxViewFrame::GetNext(*pViewFrame, pDocShell))

4
sw/source/uibase/wrtsh/select.cxx
View File

@ -749,9 +749,9 @@ void SwWrtShell::SetInsMode( bool bOn )
}
//Overwrite mode is incompatible with red-lining
void SwWrtShell::SetRedlineFlagsAndCheckInsMode( RedlineFlags eMode, bool bRecordAllViews )
void SwWrtShell::SetRedlineFlagsAndCheckInsMode( RedlineFlags eMode, SfxRedlineRecordingMode eRedlineRecordingMode )
{
SetRedlineFlags( eMode, bRecordAllViews );
SetRedlineFlags( eMode, eRedlineRecordingMode );
if (IsRedlineOn())
SetInsMode();
}