sw/inc/PostItMgr.hxx | 2
sw/qa/extras/tiledrendering/data/comments-on-load.docx |binary
sw/qa/extras/tiledrendering/tiledrendering2.cxx | 41 +++++++++++++++++
sw/qa/inc/swtestviewcallback.hxx | 1
sw/qa/unit/swtestviewcallback.cxx | 1
sw/source/uibase/docvw/PostItMgr.cxx | 27 +++++++++--
6 files changed, 67 insertions(+), 5 deletions(-)
New commits:
commit d0fbbb715801e24f33f6aaad6166a2416f1bf270
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Fri Apr 11 08:12:35 2025 +0200
Commit: Caolán McNamara <caolan.mcnam...@collabora.com>
CommitDate: Fri Apr 11 10:01:59 2025 +0200
cool#11592 sw lok: fix sometimes lost comments on load
Load a document with the LOK API, execute
getCommandValues(".uno:ViewAnnotations"), then listen for comment
changes. If the document is complex enough, some comments won't be
included anyhwere: neither in the initial comment list, nor as a
notification later.
What happens is that SwXTextDocument::getPostIts() iterates over the
postits of the postit manager, but in case the comment is not yet laid
out (pWin is nullptr), then we just ignore that comment when creating
the initial comment list. But then SwPostItMgr::LayoutPostIts() won't
emit notifications about these comments, either -- assuming these
comments are not new. So if the LOK client is fast enough to execute
getCommandValues() and the core is slow enough with the layout of the
comments, then some initial comments in the documents are lost (they are
in the document, but they are not visible).
Fix the problem by extending SwPostItMgr::GetOrCreateAnnotationWindow()
to give info when it created the annotation window, and then in
SwPostItMgr::LayoutPostIts() notify about both freshly laid out comments
and about explicitly inserted comments.
Adding a bit of debug output confirms that now comments show up even if
we hit the unlucky "no annotation window yet" case when testing
manually:
debug:31555:31497: SwXTextDocument::getPostIts: pWin is 0
debug:31555:31497: SwPostItMgr::LayoutPostIts: lok notify, type is
add
The first is the state right after load, the second is called by
SwPostItMgr::CalcHdl() as a user event ("on idle");
Change-Id: I32082d0014454bd492d2ae87fea42e28b639e8e5
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/183999
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/sw/inc/PostItMgr.hxx b/sw/inc/PostItMgr.hxx
index 8c25f381641b..3f52ab5867ff 100644
--- a/sw/inc/PostItMgr.hxx
+++ b/sw/inc/PostItMgr.hxx
@@ -131,7 +131,7 @@ class SAL_DLLPUBLIC_RTTI SwPostItMgr final : public
SfxListener,
SwSidebarItem* InsertItem( SfxBroadcaster* pItem, bool
bCheckExistence, bool bFocus);
void RemoveItem( SfxBroadcaster* pBroadcast );
- VclPtr<sw::annotation::SwAnnotationWin>
GetOrCreateAnnotationWindow(SwSidebarItem& rItem);
+ VclPtr<sw::annotation::SwAnnotationWin>
GetOrCreateAnnotationWindow(SwSidebarItem& rItem, bool& rCreated);
public:
SwPostItMgr(SwView* aDoc);
diff --git a/sw/qa/extras/tiledrendering/data/comments-on-load.docx
b/sw/qa/extras/tiledrendering/data/comments-on-load.docx
new file mode 100644
index 000000000000..858817730ff2
Binary files /dev/null and
b/sw/qa/extras/tiledrendering/data/comments-on-load.docx differ
diff --git a/sw/qa/extras/tiledrendering/tiledrendering2.cxx
b/sw/qa/extras/tiledrendering/tiledrendering2.cxx
index bb2c021ecd1f..7e9b8380478f 100644
--- a/sw/qa/extras/tiledrendering/tiledrendering2.cxx
+++ b/sw/qa/extras/tiledrendering/tiledrendering2.cxx
@@ -9,6 +9,8 @@
#include <swtiledrenderingtest.hxx>
+#include <boost/property_tree/json_parser.hpp>
+
#include <com/sun/star/util/URLTransformer.hpp>
#include <editeng/editids.hrc>
@@ -24,6 +26,7 @@
#include <sfx2/dispatch.hxx>
#include <sfx2/lokhelper.hxx>
#include <comphelper/lok.hxx>
+#include <tools/json_writer.hxx>
#include <unotxdoc.hxx>
#include <view.hxx>
@@ -711,6 +714,44 @@ CPPUNIT_TEST_FIXTURE(SwTiledRenderingTest,
testTrackChangesInsertUndo)
// record mode to "all views", which is unexpected.
CPPUNIT_ASSERT(dynamic_cast<SfxBoolItem*>(pItem.get())->GetValue());
}
+
+CPPUNIT_TEST_FIXTURE(SwTiledRenderingTest, testCommentsOnLoad)
+{
+ // Given a document of 3 pages, with a small enough visible area that
document load doesn't lay
+ // out the entire document:
+ awt::Rectangle aVisibleArea{ 0, 0, 12240, 15840 };
+ comphelper::LibreOfficeKit::setInitialClientVisibleArea(aVisibleArea);
+ comphelper::ScopeGuard g([] {
comphelper::LibreOfficeKit::setInitialClientVisibleArea({}); });
+ OUString aURL = createFileURL(u"comments-on-load.docx");
+ loadFromURL(aURL);
+ SwXTextDocument* pXTextDocument = getSwTextDoc();
+ SwTestViewCallback aView;
+ tools::JsonWriter aWriter;
+
+ // When getting the list of comments from the document + listening for
notifications from idle
+ // layout:
+ pXTextDocument->getPostIts(aWriter);
+ Scheduler::ProcessEventsToIdle();
+
+ // Then make sure that:
+ // 1) We test the interesting scenario, so getPostIts() reports 0 comments
and
+ // 2) A callback is emitted to notify about the comment on the last page
once user events are
+ // processed.
+ OString aPostIts = aWriter.finishAndGetAsOString();
+ std::stringstream aStream(aPostIts.getStr());
+ boost::property_tree::ptree aTree;
+ boost::property_tree::read_json(aStream, aTree);
+ size_t nCommentCount = aTree.get_child("comments").size();
+ CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(0), nCommentCount);
+ CPPUNIT_ASSERT_EQUAL(1, aView.m_nCommentCallbackCount);
+ // Without the accompanying fix in place, this test would have failed with:
+ // uncaught exception of type std::exception (or derived).
+ // - No such node (action)
+ // i.e. there was no notification about the comment that was positioned by
the user event,
+ // seemingly the comment was load on load (it was there, but not visible).
+ auto aAction =
aView.m_aComment.get_child("action").get_value<std::string>();
+ CPPUNIT_ASSERT_EQUAL(std::string("Add"), aAction);
+}
}
CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/sw/qa/inc/swtestviewcallback.hxx b/sw/qa/inc/swtestviewcallback.hxx
index f31467112571..913f9910b209 100644
--- a/sw/qa/inc/swtestviewcallback.hxx
+++ b/sw/qa/inc/swtestviewcallback.hxx
@@ -51,6 +51,7 @@ public:
boost::property_tree::ptree m_aRedlineTableModified;
/// Post-it / annotation payload.
boost::property_tree::ptree m_aComment;
+ int m_nCommentCallbackCount = 0;
std::vector<OString> m_aStateChanges;
TestLokCallbackWrapper m_callbackWrapper;
OString m_aExportFile;
diff --git a/sw/qa/unit/swtestviewcallback.cxx
b/sw/qa/unit/swtestviewcallback.cxx
index adc83737e3de..ec23e224c539 100644
--- a/sw/qa/unit/swtestviewcallback.cxx
+++ b/sw/qa/unit/swtestviewcallback.cxx
@@ -183,6 +183,7 @@ void SwTestViewCallback::callbackImpl(int nType, const
char* pPayload)
break;
case LOK_CALLBACK_COMMENT:
{
+ ++m_nCommentCallbackCount;
m_aComment.clear();
std::stringstream aStream(pPayload);
boost::property_tree::read_json(aStream, m_aComment);
diff --git a/sw/source/uibase/docvw/PostItMgr.cxx
b/sw/source/uibase/docvw/PostItMgr.cxx
index 116cc7961d9d..4389840dff8b 100644
--- a/sw/source/uibase/docvw/PostItMgr.cxx
+++ b/sw/source/uibase/docvw/PostItMgr.cxx
@@ -888,7 +888,7 @@ void SwPostItMgr::PreparePageContainer()
}
}
-VclPtr<SwAnnotationWin>
SwPostItMgr::GetOrCreateAnnotationWindow(SwSidebarItem& rItem)
+VclPtr<SwAnnotationWin>
SwPostItMgr::GetOrCreateAnnotationWindow(SwSidebarItem& rItem, bool& rCreated)
{
VclPtr<SwAnnotationWin> pPostIt = rItem.mpPostIt;
if (!pPostIt)
@@ -906,6 +906,8 @@ VclPtr<SwAnnotationWin>
SwPostItMgr::GetOrCreateAnnotationWindow(SwSidebarItem&
}
mpAnswer.reset();
}
+
+ rCreated = true;
}
return rItem.mpPostIt;
}
@@ -920,6 +922,7 @@ void SwPostItMgr::LayoutPostIts()
if (bEnableMapMode)
mpEditWin->EnableMapMode();
+ std::set<VclPtr<SwAnnotationWin>> aCreatedPostIts;
if ( !mvPostItFields.empty() && !mbWaitingForCalcRects )
{
mbLayouting = true;
@@ -941,7 +944,14 @@ void SwPostItMgr::LayoutPostIts()
{
if (pItem->mbShow)
{
- VclPtr<SwAnnotationWin> pPostIt =
GetOrCreateAnnotationWindow(*pItem);
+ bool bCreated = false;
+ VclPtr<SwAnnotationWin> pPostIt =
GetOrCreateAnnotationWindow(*pItem, bCreated);
+ if (bCreated)
+ {
+ // The annotation window was created for a
previously existing, but not
+ // laid out comment.
+ aCreatedPostIts.insert(pPostIt);
+ }
pPostIt->SetChangeTracking(
pItem->mLayoutStatus,
@@ -1102,7 +1112,10 @@ void SwPostItMgr::LayoutPostIts()
if (bLoKitActive && !bTiledAnnotations)
{
if (visiblePostIt->GetSidebarItem().mbPendingLayout &&
visiblePostIt->GetSidebarItem().mLayoutStatus != SwPostItHelper::DELETED)
- lcl_CommentNotification(mpView,
CommentNotificationType::Add, &visiblePostIt->GetSidebarItem(), 0);
+ {
+ // Notify about a just inserted comment.
+ aCreatedPostIts.insert(visiblePostIt);
+ }
else if (visiblePostIt->IsAnchorRectChanged())
{
lcl_CommentNotification(mpView,
CommentNotificationType::Modify, &visiblePostIt->GetSidebarItem(), 0);
@@ -1158,6 +1171,12 @@ void SwPostItMgr::LayoutPostIts()
mbLayouting = false;
}
+ // Now that comments are laid out, notify about freshly laid out or just
inserted comments.
+ for (const auto& pPostIt : aCreatedPostIts)
+ {
+ lcl_CommentNotification(mpView, CommentNotificationType::Add,
&pPostIt->GetSidebarItem(), 0);
+ }
+
if (bEnableMapMode)
mpEditWin->EnableMapMode(false);
}
@@ -1939,7 +1958,7 @@ SwPostItField* SwPostItMgr::GetLatestPostItField()
sw::annotation::SwAnnotationWin*
SwPostItMgr::GetOrCreateAnnotationWindowForLatestPostItField()
{
- return GetOrCreateAnnotationWindow(*mvPostItFields.back());
+ return GetOrCreateAnnotationWindow(*mvPostItFields.back(),
o3tl::temporary(bool()));
}
SwAnnotationWin* SwPostItMgr::GetNextPostIt( sal_uInt16 aDirection,
