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 ddc3cf99a47cb76e1386783df782e4852a19307a
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Fri Apr 11 08:12:35 2025 +0200
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Fri Apr 11 11:20:09 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/+/184014
Tested-by: Jenkins
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
diff --git a/sw/inc/PostItMgr.hxx b/sw/inc/PostItMgr.hxx
index a7abeb9514f3..8d75d8bd7920 100644
--- a/sw/inc/PostItMgr.hxx
+++ b/sw/inc/PostItMgr.hxx
@@ -131,7 +131,7 @@ class SAL_DLLPUBLIC_RTTI SwPostItMgr final : public
SfxListener,
SwAnnotationItem* InsertItem( SfxBroadcaster* pItem, bool
bCheckExistence, bool bFocus);
void RemoveItem( SfxBroadcaster* pBroadcast );
- VclPtr<sw::annotation::SwAnnotationWin>
GetOrCreateAnnotationWindow(SwAnnotationItem& rItem);
+ VclPtr<sw::annotation::SwAnnotationWin>
GetOrCreateAnnotationWindow(SwAnnotationItem& 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 00ffe811b821..e94293d66246 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 <comphelper/lok.hxx>
#include <comphelper/scopeguard.hxx>
#include <sfx2/dispatch.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 678663e426ab..2e0b35ac128e 100644
--- a/sw/source/uibase/docvw/PostItMgr.cxx
+++ b/sw/source/uibase/docvw/PostItMgr.cxx
@@ -890,7 +890,7 @@ void SwPostItMgr::PreparePageContainer()
}
}
-VclPtr<SwAnnotationWin>
SwPostItMgr::GetOrCreateAnnotationWindow(SwAnnotationItem& rItem)
+VclPtr<SwAnnotationWin>
SwPostItMgr::GetOrCreateAnnotationWindow(SwAnnotationItem& rItem, bool&
rCreated)
{
VclPtr<SwAnnotationWin> pPostIt = rItem.mpPostIt;
if (!pPostIt)
@@ -908,6 +908,8 @@ VclPtr<SwAnnotationWin>
SwPostItMgr::GetOrCreateAnnotationWindow(SwAnnotationIte
}
mpAnswer.reset();
}
+
+ rCreated = true;
}
return rItem.mpPostIt;
}
@@ -922,6 +924,7 @@ void SwPostItMgr::LayoutPostIts()
if (bEnableMapMode)
mpEditWin->EnableMapMode();
+ std::set<VclPtr<SwAnnotationWin>> aCreatedPostIts;
if ( !mvPostItFields.empty() && !mbWaitingForCalcRects )
{
mbLayouting = true;
@@ -943,7 +946,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,
@@ -1104,7 +1114,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);
@@ -1160,6 +1173,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);
}
@@ -1981,7 +2000,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,
