include/sfx2/objsh.hxx | 2
sc/source/ui/docshell/docsh.cxx | 2
sc/source/ui/inc/docsh.hxx | 2
sfx2/source/doc/objxtor.cxx | 2
sw/inc/docsh.hxx | 2
sw/qa/extras/tiledrendering/tiledrendering.cxx | 95 ++++++++++++++++++++++++-
sw/source/uibase/app/docsh.cxx | 22 +++++
sw/source/uibase/uiview/viewstat.cxx | 10 ++
8 files changed, 127 insertions(+), 10 deletions(-)
New commits:
commit d17f6bef972a858165dd6773c74f828de3176045
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Wed Mar 5 08:22:14 2025 +0100
Commit: Caolán McNamara <caolan.mcnam...@collabora.com>
CommitDate: Wed Mar 5 09:41:41 2025 +0100
cool#11226 sw per-view redline on: state for the per-view and per-doc
commands
The UI for recording is one toolbar button (toggle) with a dropdown that
has 2 radio buttons. The expectation is that once the toggle is on, one
of the radio buttons report enabled state. So this is a tri-state: the
record status is off, per-view or per-doc on the UI. At a code level, we
have a per-doc flag and we have a boolean in each view.
The requirements:
1) Compatibility: if the dropdown menu is not used, then clicking on the
old toggle should enable recording for all views & once it's enabled
(either per-view or per-doc), clicking on it should disable the
recording.
2) If recording is on, the exactly one of per-view or per-doc radio
button in the submenu should report enabled status.
This leads to a not entirely symmetric behavior if you compare the
enable, status and disable parts of this commands, and this is wanted to
result in the least amount of surprise.
Fix the problem by:
1) Enable: the toolbar button enables recording for all views (to stay
compatible), the dropdown menu allows choosing between this view vs
all views.
2) Status: the toolbar button reports enabled status if recording is on
for this view for any reason; the "this view" command reports enabled
status if recording is on in this view but not in all views; the "all
views" command reports enabled status if recording is on in this view
and no other view has it disabled.
3) Disable is almost the opposite of enabled, but the toolbar toggle can
always disable recording, so a per-view -> off -> all-views
transition is possible by clicking on the toggle twice.
This required a single change to testTrackChangesPerViewEnableOne,
because now going back to the no-record state is doable by clicking on
the enabled toolbar button, i.e. by dispatching the .uno:TrackChanges
command; dispatching .uno:TrackChangesInThisView always changes to the
per-view recording.
Change-Id: Iacc984f832b4c08e0e100a67774e1e559729d82a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/182517
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
Tested-by: Caolán McNamara <caolan.mcnam...@collabora.com>
Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>
diff --git a/include/sfx2/objsh.hxx b/include/sfx2/objsh.hxx
index 4b245a63a057..4206e26dc8bb 100644
--- a/include/sfx2/objsh.hxx
+++ b/include/sfx2/objsh.hxx
@@ -688,7 +688,7 @@ public:
// change recording and respective passwword protection for Writer and Calc
// slots available for Writer: FN_REDLINE_ON, FN_REDLINE_ON
// slots used for Calc: FID_CHG_RECORD, SID_CHG_PROTECT
- virtual bool IsChangeRecording(SfxViewShell* pViewShell = nullptr)
const;
+ 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 SetProtectionPassword( const OUString &rPassword );
diff --git a/sc/source/ui/docshell/docsh.cxx b/sc/source/ui/docshell/docsh.cxx
index 3720fa2ee685..3e13572220dc 100644
--- a/sc/source/ui/docshell/docsh.cxx
+++ b/sc/source/ui/docshell/docsh.cxx
@@ -3368,7 +3368,7 @@ void ScDocShellModificator::SetDocumentModified()
}
}
-bool ScDocShell::IsChangeRecording(SfxViewShell* /*pViewShell*/) const
+bool ScDocShell::IsChangeRecording(SfxViewShell* /*pViewShell*/, bool
/*bRecordAllViews*/) const
{
ScChangeTrack* pChangeTrack = m_pDocument->GetChangeTrack();
return pChangeTrack != nullptr;
diff --git a/sc/source/ui/inc/docsh.hxx b/sc/source/ui/inc/docsh.hxx
index 127700949474..515ee1e4fb0f 100644
--- a/sc/source/ui/inc/docsh.hxx
+++ b/sc/source/ui/inc/docsh.hxx
@@ -425,7 +425,7 @@ public:
// password protection for Calc (derived from SfxObjectShell)
// see also: FID_CHG_RECORD, SID_CHG_PROTECT
- virtual bool IsChangeRecording(SfxViewShell* pViewShell = nullptr)
const override;
+ 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 SetProtectionPassword( const OUString &rPassword )
override;
diff --git a/sfx2/source/doc/objxtor.cxx b/sfx2/source/doc/objxtor.cxx
index ec1441aad050..6be353114b0b 100644
--- a/sfx2/source/doc/objxtor.cxx
+++ b/sfx2/source/doc/objxtor.cxx
@@ -1092,7 +1092,7 @@ void SfxObjectShell::SetInitialized_Impl( const bool
i_fromInitNew )
}
-bool SfxObjectShell::IsChangeRecording(SfxViewShell* /*pViewShell*/) const
+bool SfxObjectShell::IsChangeRecording(SfxViewShell* /*pViewShell*/, bool
/*bRecordAllViews*/) const
{
// currently this function needs to be overwritten by Writer and Calc only
SAL_WARN( "sfx.doc", "function not implemented" );
diff --git a/sw/inc/docsh.hxx b/sw/inc/docsh.hxx
index 5140bd9daa99..bbebb8c2c386 100644
--- a/sw/inc/docsh.hxx
+++ b/sw/inc/docsh.hxx
@@ -321,7 +321,7 @@ public:
/** passwword protection for Writer (derived from SfxObjectShell)
see also: FN_REDLINE_ON, FN_REDLINE_ON */
- virtual bool IsChangeRecording(SfxViewShell* pViewShell = nullptr)
const override;
+ 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 SetProtectionPassword( const OUString &rPassword )
override;
diff --git a/sw/qa/extras/tiledrendering/tiledrendering.cxx
b/sw/qa/extras/tiledrendering/tiledrendering.cxx
index 030bb6a589a8..1046c9ea27fe 100644
--- a/sw/qa/extras/tiledrendering/tiledrendering.cxx
+++ b/sw/qa/extras/tiledrendering/tiledrendering.cxx
@@ -4860,7 +4860,7 @@ CPPUNIT_TEST_FIXTURE(SwTiledRenderingTest,
testTrackChangesPerViewEnableOne)
CPPUNIT_ASSERT(aRecord2.empty());
// And given a reset state (both view1 and view2 recording is disabled):
- comphelper::dispatchCommand(".uno:TrackChangesInThisView", {});
+ comphelper::dispatchCommand(".uno:TrackChanges", {});
// When recording changes in view2:
SfxLokHelper::setView(nView2);
@@ -5004,6 +5004,99 @@ CPPUNIT_TEST_FIXTURE(SwTiledRenderingTest,
testTrackChangesPerDocInsert)
CPPUNIT_ASSERT_EQUAL(static_cast<SwRedlineTable::size_type>(2),
pWrtShell2->GetRedlineCount());
}
+CPPUNIT_TEST_FIXTURE(SwTiledRenderingTest, testTrackChangesStates)
+{
+ // Given a document with 2 views:
+ SwXTextDocument* pXTextDocument = createDoc();
+ CPPUNIT_ASSERT(pXTextDocument);
+ ViewCallback aView1;
+ int nView1 = SfxLokHelper::getView();
+ SwView* pView1 = pXTextDocument->GetDocShell()->GetView();
+ SfxLokHelper::createView();
+ ViewCallback aView2;
+ SwView* pView2 = pXTextDocument->GetDocShell()->GetView();
+ SfxLokHelper::setView(nView1);
+
+ // When enabling recording in view1 only:
+
pView1->GetViewFrame().GetDispatcher()->Execute(FN_TRACK_CHANGES_IN_THIS_VIEW,
+ SfxCallMode::SYNCHRON);
+
+ // Then make sure the "is record", "is record for this view on" and "is
record for all views on"
+ // statuses are correct:
+ std::unique_ptr<SfxPoolItem> pItem;
+ pView1->GetViewFrame().GetBindings().QueryState(FN_REDLINE_ON, pItem);
+ CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt16>(FN_REDLINE_ON),
pItem->Which());
+ // Without the accompanying fix in place, this test would have failed,
enabling recording for
+ // this view didn't enable the toolbar button in the same view, which
looked confusing.
+ CPPUNIT_ASSERT(dynamic_cast<SfxBoolItem*>(pItem.get())->GetValue());
+
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());
+ CPPUNIT_ASSERT(dynamic_cast<SfxBoolItem*>(pItem.get())->GetValue());
+
pView1->GetViewFrame().GetBindings().QueryState(FN_TRACK_CHANGES_IN_ALL_VIEWS,
pItem);
+
CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt16>(FN_TRACK_CHANGES_IN_ALL_VIEWS),
pItem->Which());
+ CPPUNIT_ASSERT(!dynamic_cast<SfxBoolItem*>(pItem.get())->GetValue());
+ pView2->GetViewFrame().GetBindings().QueryState(FN_REDLINE_ON, pItem);
+ CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt16>(FN_REDLINE_ON),
pItem->Which());
+ CPPUNIT_ASSERT(!dynamic_cast<SfxBoolItem*>(pItem.get())->GetValue());
+
pView2->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());
+ CPPUNIT_ASSERT(!dynamic_cast<SfxBoolItem*>(pItem.get())->GetValue());
+
pView2->GetViewFrame().GetBindings().QueryState(FN_TRACK_CHANGES_IN_ALL_VIEWS,
pItem);
+
CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt16>(FN_TRACK_CHANGES_IN_ALL_VIEWS),
pItem->Which());
+ CPPUNIT_ASSERT(!dynamic_cast<SfxBoolItem*>(pItem.get())->GetValue());
+
+ // When disabling recording:
+ SfxBoolItem aOn(FN_REDLINE_ON, false);
+ pView1->GetViewFrame().GetDispatcher()->ExecuteList(FN_REDLINE_ON,
SfxCallMode::SYNCHRON,
+ { &aOn });
+
+ // Then make sure the "is record", "is record for this view on" and "is
record for all views on"
+ // statuses are correct:
+ pView1->GetViewFrame().GetBindings().QueryState(FN_REDLINE_ON, pItem);
+ CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt16>(FN_REDLINE_ON),
pItem->Which());
+ CPPUNIT_ASSERT(!dynamic_cast<SfxBoolItem*>(pItem.get())->GetValue());
+
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());
+ CPPUNIT_ASSERT(!dynamic_cast<SfxBoolItem*>(pItem.get())->GetValue());
+
pView1->GetViewFrame().GetBindings().QueryState(FN_TRACK_CHANGES_IN_ALL_VIEWS,
pItem);
+
CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt16>(FN_TRACK_CHANGES_IN_ALL_VIEWS),
pItem->Which());
+ CPPUNIT_ASSERT(!dynamic_cast<SfxBoolItem*>(pItem.get())->GetValue());
+ pView2->GetViewFrame().GetBindings().QueryState(FN_REDLINE_ON, pItem);
+ CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt16>(FN_REDLINE_ON),
pItem->Which());
+ CPPUNIT_ASSERT(!dynamic_cast<SfxBoolItem*>(pItem.get())->GetValue());
+
pView2->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());
+ CPPUNIT_ASSERT(!dynamic_cast<SfxBoolItem*>(pItem.get())->GetValue());
+
pView2->GetViewFrame().GetBindings().QueryState(FN_TRACK_CHANGES_IN_ALL_VIEWS,
pItem);
+
CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt16>(FN_TRACK_CHANGES_IN_ALL_VIEWS),
pItem->Which());
+ CPPUNIT_ASSERT(!dynamic_cast<SfxBoolItem*>(pItem.get())->GetValue());
+
+ // When enabling recording in all views:
+
pView1->GetViewFrame().GetDispatcher()->Execute(FN_TRACK_CHANGES_IN_ALL_VIEWS,
+ SfxCallMode::SYNCHRON);
+
+ // Then make sure the "is record", "is record for this view on" and "is
record for all views on"
+ // statuses are correct:
+ pView1->GetViewFrame().GetBindings().QueryState(FN_REDLINE_ON, pItem);
+ CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt16>(FN_REDLINE_ON),
pItem->Which());
+ CPPUNIT_ASSERT(dynamic_cast<SfxBoolItem*>(pItem.get())->GetValue());
+
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());
+ CPPUNIT_ASSERT(!dynamic_cast<SfxBoolItem*>(pItem.get())->GetValue());
+
pView1->GetViewFrame().GetBindings().QueryState(FN_TRACK_CHANGES_IN_ALL_VIEWS,
pItem);
+
CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt16>(FN_TRACK_CHANGES_IN_ALL_VIEWS),
pItem->Which());
+ CPPUNIT_ASSERT(dynamic_cast<SfxBoolItem*>(pItem.get())->GetValue());
+ pView2->GetViewFrame().GetBindings().QueryState(FN_REDLINE_ON, pItem);
+ CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt16>(FN_REDLINE_ON),
pItem->Which());
+ CPPUNIT_ASSERT(dynamic_cast<SfxBoolItem*>(pItem.get())->GetValue());
+
pView2->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());
+ CPPUNIT_ASSERT(!dynamic_cast<SfxBoolItem*>(pItem.get())->GetValue());
+
pView2->GetViewFrame().GetBindings().QueryState(FN_TRACK_CHANGES_IN_ALL_VIEWS,
pItem);
+
CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt16>(FN_TRACK_CHANGES_IN_ALL_VIEWS),
pItem->Which());
+ CPPUNIT_ASSERT(dynamic_cast<SfxBoolItem*>(pItem.get())->GetValue());
+}
+
CPPUNIT_PLUGIN_IMPLEMENT();
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/uibase/app/docsh.cxx b/sw/source/uibase/app/docsh.cxx
index f71093addeb4..5f20ca5b1825 100644
--- a/sw/source/uibase/app/docsh.cxx
+++ b/sw/source/uibase/app/docsh.cxx
@@ -1346,7 +1346,7 @@ const ::sfx2::IXmlIdRegistry*
SwDocShell::GetXmlIdRegistry() const
return m_xDoc ? &m_xDoc->GetXmlIdRegistry() : nullptr;
}
-bool SwDocShell::IsChangeRecording(SfxViewShell* pViewShell) const
+bool SwDocShell::IsChangeRecording(SfxViewShell* pViewShell, bool
bRecordAllViews) const
{
SwWrtShell* pWrtShell = nullptr;
auto pView = dynamic_cast<SwView*>(pViewShell);
@@ -1361,7 +1361,25 @@ bool SwDocShell::IsChangeRecording(SfxViewShell*
pViewShell) const
if (!pWrtShell)
return false;
- return bool(pWrtShell->GetRedlineFlags() & RedlineFlags::On);
+
+ auto bOn = bool(pWrtShell->GetRedlineFlags() & RedlineFlags::On);
+ if (bOn)
+ {
+ if (bRecordAllViews)
+ {
+ for (SwViewShell& rSh : pWrtShell->GetRingContainer())
+ {
+ if (!rSh.GetViewOptions()->IsRedlineRecordingOn())
+ {
+ return false;
+ }
+ }
+ }
+
+ return true;
+ }
+
+ return false;
}
bool SwDocShell::HasChangeRecordProtection() const
diff --git a/sw/source/uibase/uiview/viewstat.cxx
b/sw/source/uibase/uiview/viewstat.cxx
index fd568e6d7a1e..84d6b1ece4e6 100644
--- a/sw/source/uibase/uiview/viewstat.cxx
+++ b/sw/source/uibase/uiview/viewstat.cxx
@@ -325,7 +325,8 @@ void SwView::GetState(SfxItemSet &rSet)
}
break;
case FN_REDLINE_ON:
- rSet.Put( SfxBoolItem( nWhich,
GetDocShell()->IsChangeRecording(this) ) );
+ // Enabled at least in this view.
+ rSet.Put( SfxBoolItem( nWhich,
GetDocShell()->IsChangeRecording(this, /*bRecordAllViews=*/false) ) );
// When the view is new (e.g. on load), show the Hidden Track
Changes infobar
// if Show Changes is disabled, but recording of changes is
enabled
// or hidden tracked changes are there already in the document.
@@ -333,7 +334,7 @@ void SwView::GetState(SfxItemSet &rSet)
// enabled, see in sfx2.
if ( m_bForceChangesToolbar &&
m_pWrtShell->GetLayout()->IsHideRedlines() )
{
- bool isRecording = GetDocShell()->IsChangeRecording(this);
+ bool isRecording = GetDocShell()->IsChangeRecording(this,
/*bRecordAllViews=*/false);
bool hasRecorded =
m_pWrtShell->GetDoc()->getIDocumentRedlineAccess().GetRedlineTable().size();
if ( isRecording || hasRecorded )
@@ -352,10 +353,15 @@ void SwView::GetState(SfxItemSet &rSet)
break;
case FN_TRACK_CHANGES_IN_THIS_VIEW:
{
+ // Enabled in this view, but not in all views.
+ bool bOn = GetDocShell()->IsChangeRecording(this,
/*bRecordAllViews=*/false) && !GetDocShell()->IsChangeRecording(this);
+ rSet.Put(SfxBoolItem(nWhich, bOn));
}
break;
case FN_TRACK_CHANGES_IN_ALL_VIEWS:
{
+ // Enabled in all views.
+ rSet.Put(SfxBoolItem(nWhich,
GetDocShell()->IsChangeRecording(this)));
}
break;
case FN_REDLINE_PROTECT :
