online.git: browser/src cypress_test/integration_tests

0 views
Skip to first unread message

"Miklos Vajna (via github)"

unread,
Mar 2, 2026, 8:48:58 AMMar 2
to collaboraon...@googlegroups.com
browser/src/app/ViewLayoutCompareChanges.ts | 15 ++++
cypress_test/integration_tests/desktop/writer/track_changes_spec.js | 31 ++++++++++
2 files changed, 44 insertions(+), 2 deletions(-)

New commits:
commit 38c139612967cec17d6ccc95057264bc670833be
Author: Miklos Vajna <vmi...@collabora.com>
AuthorDate: Mon Mar 2 10:43:00 2026 +0100
Commit: Caolán McNamara <cao...@gmail.com>
CommitDate: Mon Mar 2 13:48:28 2026 +0000

cool#14716 document compare: fix bad tile position after zoom change

Enter the document compare view in Writer using Review -> View Changes
on the notebookbar, zoom out: the left/right change labels and the tiles
are no longer in sync.

Notice that the multi-page view doesn't have this problem and it has
explicit code at browser/src/app/ViewLayoutMultiPage.ts to listen to a
'zoomend' event.

Fix the problem for compare changes view layout similarly. Extract part
of the scroll handler to a separate refreshView() and also from the
'zoomend' event listener.

This way the tiles are in sync with the left/right change labels after a
zoom change.

Signed-off-by: Miklos Vajna <vmi...@collabora.com>
Change-Id: I22357cb67387f25ab6e31ff00b12e390ee049bda

diff --git a/browser/src/app/ViewLayoutCompareChanges.ts b/browser/src/app/ViewLayoutCompareChanges.ts
index b7b9934ecb..75f2a3e6a6 100644
--- a/browser/src/app/ViewLayoutCompareChanges.ts
+++ b/browser/src/app/ViewLayoutCompareChanges.ts
@@ -27,11 +27,23 @@ class ViewLayoutCompareChanges extends ViewLayoutNewBase {
constructor() {
super();

+ app.map.on('zoomend', this.onZoomEnd.bind(this));
+
this.adjustViewZoomLevel();

app.layoutingService.appendLayoutingTask(() => this.updateViewData());
}

+ /// Refresh the view after scroll or zoom change.
+ private refreshView(): void {
+ this.updateViewData();
+ app.sectionContainer.requestReDraw();
+ }
+
+ private onZoomEnd(): void {
+ this.refreshView();
+ }
+
public adjustViewZoomLevel() {
Util.ensureValue(app.activeDocument);

@@ -164,8 +176,7 @@ class ViewLayoutCompareChanges extends ViewLayoutNewBase {
const scrolled = super.scroll(pX, pY);

if (scrolled) {
- this.updateViewData();
- app.sectionContainer.requestReDraw();
+ this.refreshView();
}

return scrolled;
diff --git a/cypress_test/integration_tests/desktop/writer/track_changes_spec.js b/cypress_test/integration_tests/desktop/writer/track_changes_spec.js
index ae00475fa8..56eaee0de7 100644
--- a/cypress_test/integration_tests/desktop/writer/track_changes_spec.js
+++ b/cypress_test/integration_tests/desktop/writer/track_changes_spec.js
@@ -307,6 +307,37 @@ describe(['tagdesktop', 'tagnextcloud', 'tagproxy'], 'Track Changes', function (
});
});

+ it('Zoom out updates visible area in compare changes mode', function () {
+ // Given a document in compare changes mode:
+ desktopHelper.switchUIToNotebookbar();
+ cy.cGet('#Review-tab-label').click();
+ desktopHelper.getNbIconArrow('TrackChanges', 'Review').click();
+ cy.cGet('#compare-tracked-change').filter(':visible').click();
+ cy.cGet('.compare-changes-labels').should('not.have.css', 'display', 'none');
+
+ // When zooming out:
+ let initialWidth = 0;
+ cy.getFrameWindow().then(function(win) {
+ cy.wrap(null).should(function() {
+ initialWidth = win.app.activeDocument.activeLayout.viewedRectangle.width;
+ expect(initialWidth, 'initial viewedRectangle.width').to.be.greaterThan(0);
+ });
+ });
+ desktopHelper.zoomOut();
+
+ // Then the visible area width (in twips) should increase, since each pixel now covers
+ // more twips at a smaller zoom level:
+ cy.getFrameWindow().then(function(win) {
+ cy.wrap(null).should(function() {
+ let newWidth = win.app.activeDocument.activeLayout.viewedRectangle.width;
+ // Without the accompanying fix in place, this test would have failed, the
+ // visible area was not updated on zoom change: 14689 didn't
+ // increase to 17627
+ expect(newWidth, 'viewedRectangle.width after zoom out').to.be.greaterThan(initialWidth);
+ });
+ });
+ });
+
it.skip('Comment Undo-Redo', function () {
for (var n = 0; n < 2; n++) {
desktopHelper.getCompactIconArrow('DefaultNumbering').click();

"Mike Kaganski (via github)"

unread,
Mar 4, 2026, 8:33:32 AMMar 4
to collaboraon...@googlegroups.com
browser/src/canvas/sections/CommentListSection.ts | 4 +
cypress_test/integration_tests/desktop/calc/annotation_spec.js | 35 +++++++++-
cypress_test/integration_tests/desktop/impress/annotation_spec.js | 25 ++++++-
cypress_test/integration_tests/desktop/writer/annotation_spec.js | 25 ++++++-
4 files changed, 86 insertions(+), 3 deletions(-)

New commits:
commit ff10be2b9b57b832219d450ce972781bcc0603de
Author: Mike Kaganski <mike.k...@collabora.com>
AuthorDate: Tue Mar 3 20:26:03 2026 +0500
Commit: Miklos Vajna <vmi...@collabora.com>
CommitDate: Wed Mar 4 14:32:36 2026 +0100

Send "Clicked_Comment" postMessage when comment is clicked

The comment id is the only message's value.

Signed-off-by: Mike Kaganski <mike.k...@collabora.com>
Change-Id: I16128cf5dae3188c54728581404fb255f2793e4f

diff --git a/browser/src/canvas/sections/CommentListSection.ts b/browser/src/canvas/sections/CommentListSection.ts
index cbef45677a..b6a5df20d1 100644
--- a/browser/src/canvas/sections/CommentListSection.ts
+++ b/browser/src/canvas/sections/CommentListSection.ts
@@ -734,6 +734,10 @@ export class CommentSection extends CanvasSectionObject {

public click (annotation: any): void {
this.select(annotation);
+ app.map.fire('postMessage', {
+ msgId: 'Clicked_Comment',
+ args: { Id: annotation.sectionProperties.data.id }
+ });
}

public save (annotation: any): void {
diff --git a/cypress_test/integration_tests/desktop/calc/annotation_spec.js b/cypress_test/integration_tests/desktop/calc/annotation_spec.js
index 70b9dcc3bf..635524b0e2 100644
--- a/cypress_test/integration_tests/desktop/calc/annotation_spec.js
+++ b/cypress_test/integration_tests/desktop/calc/annotation_spec.js
@@ -1,4 +1,4 @@
-/* global describe it require cy beforeEach */
+/* global describe it require cy beforeEach expect */

var helper = require('../../common/helper');
var desktopHelper = require('../../common/desktop_helper');
@@ -58,6 +58,39 @@ describe(['tagdesktop'], 'Annotation Tests', function() {
});
});

+ it('Click on comment emits Clicked_Comment postMessage', function() {
+ desktopHelper.insertComment();
+
+ // This will record usage of window.postMessage (called from
+ // _postMessage in browser/src/map/handler/Map.WOPI.js
+ cy.getFrameWindow().then(win => {
+ cy.stub(win.parent, 'postMessage').as('postMessage');
+ });
+
+ // <div id="comment-container-1" ...> is the topmost element of the comment.
+ // Override its hidden attributes; then 'mouseover' should make the comment visible.
+ cy.cGet('#comment-container-1').should('exist');
+ cy.cGet('#comment-container-1').then(element => {
+ element[0].style.visibility = '';
+ element[0].style.display = '';
+ });
+ cy.cGet('#comment-container-1').trigger('mouseover', {force: true});
+
+ // <div class="cool-annotation-content-wrapper" ...> is its immediate child,
+ // and its internals are the same as in other modules
+ cy.cGet('.cool-annotation-content-wrapper').should('be.visible');
+ cy.cGet('.cool-annotation-content-wrapper').click();
+
+ cy.get('@postMessage').should(stub => {
+ const found = stub.getCalls().some(call => {
+ const msg = JSON.parse(call.args[0]);
+ return msg.MessageId === 'Clicked_Comment'
+ && msg.Values && msg.Values.Id !== undefined;
+ });
+ expect(found, "Clicked_Comment was not posted").to.be.true;
+ });
+ });
+
it('Modify',function() {
desktopHelper.insertComment();

diff --git a/cypress_test/integration_tests/desktop/impress/annotation_spec.js b/cypress_test/integration_tests/desktop/impress/annotation_spec.js
index 3d87fde84c..760feca6dc 100644
--- a/cypress_test/integration_tests/desktop/impress/annotation_spec.js
+++ b/cypress_test/integration_tests/desktop/impress/annotation_spec.js
@@ -1,4 +1,4 @@
-/* global describe it cy require beforeEach Cypress */
+/* global describe it cy require beforeEach expect Cypress */

var desktopHelper = require('../../common/desktop_helper');
var helper = require('../../common/helper');
@@ -68,6 +68,29 @@ describe(['tagdesktop'], 'Annotation Tests', function() {
cy.cGet('.cool-annotation-content > div').should('include.text','some reply text');
});

+ it('Click on comment emits Clicked_Comment postMessage', function() {
+ desktopHelper.insertComment();
+
+ // This will record usage of window.postMessage (called from
+ // _postMessage in browser/src/map/handler/Map.WOPI.js
+ cy.getFrameWindow().then(win => {
+ cy.stub(win.parent, 'postMessage').as('postMessage');
+ });
+
+ // <div class="cool-annotation-content-wrapper" ...> is the topmost element of the comment
+ cy.cGet('.cool-annotation-content-wrapper').should('be.visible');
+ cy.cGet('.cool-annotation-content-wrapper').click();
+
+ cy.get('@postMessage').should(stub => {
+ const found = stub.getCalls().some(call => {
+ const msg = JSON.parse(call.args[0]);
+ return msg.MessageId === 'Clicked_Comment'
+ && msg.Values && msg.Values.Id !== undefined;
+ });
+ expect(found, "Clicked_Comment was not posted").to.be.true;
+ });
+ });
+
it('Tab Navigation', function() {
desktopHelper.insertComment(undefined, false);

diff --git a/cypress_test/integration_tests/desktop/writer/annotation_spec.js b/cypress_test/integration_tests/desktop/writer/annotation_spec.js
index 5e5e9c187f..fe73e24aa9 100644
--- a/cypress_test/integration_tests/desktop/writer/annotation_spec.js
+++ b/cypress_test/integration_tests/desktop/writer/annotation_spec.js
@@ -1,4 +1,4 @@
-/* global describe it cy require beforeEach */
+/* global describe it cy require beforeEach expect */

var helper = require('../../common/helper');
var desktopHelper = require('../../common/desktop_helper');
@@ -55,6 +55,29 @@ describe(['tagdesktop'], 'Annotation Tests', function() {
cy.cGet('.cool-annotation-content-wrapper').should('not.exist');
});

+ it('Click on comment emits Clicked_Comment postMessage', function() {
+ desktopHelper.insertComment();
+
+ // This will record usage of window.postMessage (called from
+ // _postMessage in browser/src/map/handler/Map.WOPI.js
+ cy.getFrameWindow().then(win => {
+ cy.stub(win.parent, 'postMessage').as('postMessage');
+ });
+
+ // <div class="cool-annotation-content-wrapper" ...> is the topmost element of the comment
+ cy.cGet('.cool-annotation-content-wrapper').should('be.visible');
+ cy.cGet('.cool-annotation-content-wrapper').click();
+
+ cy.get('@postMessage').should(stub => {
+ const found = stub.getCalls().some(call => {
+ const msg = JSON.parse(call.args[0]);
+ return msg.MessageId === 'Clicked_Comment'
+ && msg.Values && msg.Values.Id !== undefined;
+ });
+ expect(found, "Clicked_Comment was not posted").to.be.true;
+ });
+ });
+
it('Toggle Resolved/Unresolved', function() {
desktopHelper.insertComment("unresolved comment", true);
cy.cGet('#comment-container-1').should('exist');

"Mike Kaganski (via github)"

unread,
Mar 5, 2026, 8:18:59 AMMar 5
to collaboraon...@googlegroups.com
browser/src/map/handler/Map.WOPI.js | 11 +++++
cypress_test/integration_tests/desktop/writer/annotation_spec.js | 19 ++++++++++
2 files changed, 30 insertions(+)

New commits:
commit 20e356caf0322e14fd2cd6f479fae9821cd7792d
Author: Mike Kaganski <mike.k...@collabora.com>
AuthorDate: Wed Mar 4 20:12:39 2026 +0500
Commit: Miklos Vajna <vmi...@collabora.com>
CommitDate: Thu Mar 5 14:18:14 2026 +0100

Add Action_ResolveComment PostMessage

With this message sent from a WOPI host, it is possible to specify
which comment (identified by Id) to resolve.

The Id matches the Id sent in Clicked_Comment notification message.

This now works only in Writer: there is no "Resolve" feature in
other components.

Signed-off-by: Mike Kaganski <mike.k...@collabora.com>
Change-Id: I8ce6796bb4822d09f634a5bf2087bede58b759a3

diff --git a/browser/src/map/handler/Map.WOPI.js b/browser/src/map/handler/Map.WOPI.js
index 0103639892..182cad6f8d 100644
--- a/browser/src/map/handler/Map.WOPI.js
+++ b/browser/src/map/handler/Map.WOPI.js
@@ -787,6 +787,17 @@ window.L.Map.WOPI = window.L.Handler.extend({
var list = msg.Values.list;
this._map.mention.openMentionPopup(list);
}
+ else if (msg.MessageId === 'Action_ResolveComment') {
+ if (msg.Values) {
+ const commentSection = app.sectionContainer.getSectionWithName(app.CSections.CommentList.name);
+ if (commentSection) {
+ const comment = commentSection.getComment(msg.Values.Id);
+ if (comment && comment.sectionProperties.data.resolved !== 'true') {
+ commentSection.resolve(comment);
+ }
+ }
+ }
+ }
else if (msg.sender === 'EIDEASY_SINGLE_METHOD_SIGNATURE') {
// This is produced by the esign popup.
const eSignature = this._map.eSignature;
diff --git a/cypress_test/integration_tests/desktop/writer/annotation_spec.js b/cypress_test/integration_tests/desktop/writer/annotation_spec.js
index fe73e24aa9..248e767dd3 100644
--- a/cypress_test/integration_tests/desktop/writer/annotation_spec.js
+++ b/cypress_test/integration_tests/desktop/writer/annotation_spec.js
@@ -78,6 +78,25 @@ describe(['tagdesktop'], 'Annotation Tests', function() {
});
});

+ it('Action_ResolveComment postMessage resolves a comment', function() {
+ desktopHelper.insertComment();
+
+ cy.cGet('.cool-annotation-content-wrapper').should('be.visible');
+ cy.cGet('.cool-annotation-content-resolved').should('have.text', '');
+
+ // Send Action_ResolveComment postMessage with the comment's Id
+ cy.getFrameWindow().then(win => {
+ const message = {
+ 'MessageId': 'Action_ResolveComment',
+ 'Values': {'Id': '1'}
+ };
+ win.postMessage(JSON.stringify(message), '*');
+ });
+
+ // The comment should now show as resolved
+ cy.cGet('.cool-annotation-content-resolved').should('have.text', 'Resolved');

"Miklos Vajna (via github)"

unread,
Mar 11, 2026, 10:45:08 AMMar 11
to collaboraon...@googlegroups.com
browser/src/control/Control.NotebookbarWriter.js | 12 ++------
browser/src/control/jsdialog/Definitions.Menu.ts | 15 ++++++++++
browser/src/control/jsdialog/Widget.MenuButton.js | 2 -
cypress_test/integration_tests/desktop/writer/track_changes_spec.js | 14 ++++-----
4 files changed, 25 insertions(+), 18 deletions(-)

New commits:
commit d8972a97ae438600046f1f11e71308643e97eefe
Author: Miklos Vajna <vmi...@collabora.com>
AuthorDate: Wed Mar 11 11:38:53 2026 +0100
Commit: Caolán McNamara <cao...@gmail.com>
CommitDate: Wed Mar 11 14:44:41 2026 +0000

cool#14940 document compare: merge Show and View Changes buttons into a single menu button

Open a Writer document, the Review tab's Tracking section has both
'Show' and 'View Changes' buttons, which is confusing: showing and
viewing is similar.

The underlying action is different: Show will do a Writer core
layout-level show/hide for the redlines; View Changes will toggle
between the JS layout's (normal) ViewLayout vs ViewLayoutCompareChanges.

Fix the problem by replacing the two buttons with a menu button: just
clicking on the button does the JS layout toogle / side-by-side view.
And turn the old Show button into an item in the merge button's menu,
with an Inline label.

This uncovered a problem in Widget.MenuButton.js: the custom JS action
for the main button (not the arrow button -> dropdown items' click
handler) invoked the callback twice: once for the label and once for the
button that contains the label. It seems this was added in commit
361039c528503a9dec47e4f12639a32324848355 (remove-w2ui: reuse MenuButton
for color picker button, 2024-03-18), but given that the click from the
label propagates to the button, this looks not necessary and here it's
actively harmful: we quickly switched to the compare layout and then
back to normal on click. Most such buttons dispatch an UNO command (and
not a JS action), and there this is not a problem in practice. Also
adapt cypress tests for the UI change.

Signed-off-by: Miklos Vajna <vmi...@collabora.com>
Change-Id: I0d3d5592d69c2dde26f02689b7ac11eabdd121cb

diff --git a/browser/src/control/Control.NotebookbarWriter.js b/browser/src/control/Control.NotebookbarWriter.js
index 8ce3476cc3..bd861f8124 100644
--- a/browser/src/control/Control.NotebookbarWriter.js
+++ b/browser/src/control/Control.NotebookbarWriter.js
@@ -2337,16 +2337,10 @@ window.L.Control.NotebookbarWriter = window.L.Control.Notebookbar.extend({
'accessibility': { focusBack: true, combination: 'TC', de: null }
},
{
- 'id': 'review-show-tracked-changes',
- 'type': 'bigtoolitem',
- 'text': _UNO('.uno:ShowTrackedChanges', 'text'),
- 'command': '.uno:ShowTrackedChanges',
- 'accessibility': { focusBack: true, combination: 'SC', de: null }
- },
- {
- 'id': 'compare-tracked-change',
- 'type': 'bigcustomtoolitem',
+ 'id': 'compare-tracked-change:ViewChangesMenu',
+ 'type': 'menubutton',
'text': _('View Changes'),
+ 'applyCallback': 'comparechanges',
'command': 'comparechanges',
'accessibility': { focusBack: true, combination: 'CC' }
},
diff --git a/browser/src/control/jsdialog/Definitions.Menu.ts b/browser/src/control/jsdialog/Definitions.Menu.ts
index 59ac50675a..86ecbc8e93 100644
--- a/browser/src/control/jsdialog/Definitions.Menu.ts
+++ b/browser/src/control/jsdialog/Definitions.Menu.ts
@@ -1781,6 +1781,21 @@ menuDefinitions.set('PasteMenu', [
},
] as Array<MenuDefinition>);

+menuDefinitions.set('ViewChangesMenu', [
+ {
+ id: 'review-show-tracked-changes',
+ img: 'showtrackedchanges',
+ text: _('Inline'),
+ uno: '.uno:ShowTrackedChanges',
+ },
+ {
+ id: 'compare-tracked-change',
+ img: 'comparechanges',
+ text: _('Side by Side'),
+ action: 'comparechanges',
+ },
+] as Array<MenuDefinition>);
+
menuDefinitions.set('RecordTrackedChangesMenu', [
{
id: 'review-track-changes-off',
diff --git a/browser/src/control/jsdialog/Widget.MenuButton.js b/browser/src/control/jsdialog/Widget.MenuButton.js
index 9a3f7b63e1..4c39b190ad 100644
--- a/browser/src/control/jsdialog/Widget.MenuButton.js
+++ b/browser/src/control/jsdialog/Widget.MenuButton.js
@@ -145,8 +145,6 @@ function _menubuttonControl (parentContainer, data, builder) {
// make it possible to setup separate callbacks for split button
if (isSplitButton) {
JSDialog.AddOnClick(control.button, applyCallback);
- if (control.label)
- JSDialog.AddOnClick(control.label, applyCallback);
if (control.arrow)
control.arrow.tabIndex = 0;
} else {
diff --git a/cypress_test/integration_tests/desktop/writer/track_changes_spec.js b/cypress_test/integration_tests/desktop/writer/track_changes_spec.js
index 56eaee0de7..a0bf49d908 100644
--- a/cypress_test/integration_tests/desktop/writer/track_changes_spec.js
+++ b/cypress_test/integration_tests/desktop/writer/track_changes_spec.js
@@ -146,7 +146,7 @@ describe(['tagdesktop', 'tagnextcloud', 'tagproxy'], 'Track Changes', function (
cy.cGet('#AcceptRejectChangesDialog').parents('.jsdialog-window').find('.ui-dialog-titlebar-close').click();
cy.cGet('#AcceptRejectChangesDialog').should('not.exist');
desktopHelper.getNbIconArrow('TrackChanges', 'Review').click();
- cy.cGet('#compare-tracked-change').filter(':visible').click();
+ cy.cGet('#compare-tracked-change-button').filter(':visible').click();

// Then the left label should show the old document name:
cy.cGet('#compare-changes-left-title').should(function($el) {
@@ -177,7 +177,7 @@ describe(['tagdesktop', 'tagnextcloud', 'tagproxy'], 'Track Changes', function (
cy.cGet('#review-tracking').click();
// If this is visible or not is not interesting, we want to assert the resulting
// title.
- cy.cGet('#compare-tracked-change').click({force: true});
+ cy.cGet('#compare-tracked-change-button').click({force: true});
cy.cGet('#compare-changes-left-title').should(function($el) {
expect($el.text()).to.match(/^remote_old\.odt/);
});
@@ -193,7 +193,7 @@ describe(['tagdesktop', 'tagnextcloud', 'tagproxy'], 'Track Changes', function (

// When entering doc compare mode via View Changes:
desktopHelper.getNbIconArrow('TrackChanges', 'Review').click();
- cy.cGet('#compare-tracked-change').filter(':visible').click();
+ cy.cGet('#compare-tracked-change-button').filter(':visible').click();

// Then tiles should exist for both mode=1 (LeftSide) and mode=2 (RightSide)
// with content:
@@ -226,7 +226,7 @@ describe(['tagdesktop', 'tagnextcloud', 'tagproxy'], 'Track Changes', function (
desktopHelper.switchUIToNotebookbar();
cy.cGet('#Review-tab-label').click();
desktopHelper.getNbIconArrow('TrackChanges', 'Review').click();
- cy.cGet('#compare-tracked-change').filter(':visible').click();
+ cy.cGet('#compare-tracked-change-button').filter(':visible').click();
cy.cGet('.compare-changes-labels').should('not.have.css', 'display', 'none');

// When faking a tooltip message for a tracked change on the right side:
@@ -255,7 +255,7 @@ describe(['tagdesktop', 'tagnextcloud', 'tagproxy'], 'Track Changes', function (
desktopHelper.switchUIToNotebookbar();
cy.cGet('#Review-tab-label').click();
desktopHelper.getNbIconArrow('TrackChanges', 'Review').click();
- cy.cGet('#compare-tracked-change').filter(':visible').click();
+ cy.cGet('#compare-tracked-change-button').filter(':visible').click();
cy.cGet('.compare-changes-labels').should('not.have.css', 'display', 'none');

// When faking a tooltip message with anchor rectangles for a deletion:
@@ -289,7 +289,7 @@ describe(['tagdesktop', 'tagnextcloud', 'tagproxy'], 'Track Changes', function (
helper.typeIntoDocument('x');
cy.cGet('#Review-tab-label').click();
desktopHelper.getNbIconArrow('TrackChanges', 'Review').click();
- cy.cGet('#compare-tracked-change').filter(':visible').click();
+ cy.cGet('#compare-tracked-change-button').filter(':visible').click();
cy.cGet('.compare-changes-labels').should('not.have.css', 'display', 'none');

// When double-clicking at the cursor position on the right side to create a selection:
@@ -312,7 +312,7 @@ describe(['tagdesktop', 'tagnextcloud', 'tagproxy'], 'Track Changes', function (
desktopHelper.switchUIToNotebookbar();
cy.cGet('#Review-tab-label').click();
desktopHelper.getNbIconArrow('TrackChanges', 'Review').click();
- cy.cGet('#compare-tracked-change').filter(':visible').click();
+ cy.cGet('#compare-tracked-change-button').filter(':visible').click();
cy.cGet('.compare-changes-labels').should('not.have.css', 'display', 'none');

// When zooming out:

"Caolán McNamara (via github)"

unread,
Mar 13, 2026, 9:53:16 AMMar 13
to collaboraon...@googlegroups.com
browser/src/control/Control.NotebookbarCalc.js | 2 +-
browser/src/control/Control.NotebookbarDraw.js | 2 +-
browser/src/control/Control.NotebookbarImpress.js | 2 +-
browser/src/control/Control.NotebookbarWriter.js | 2 +-
cypress_test/integration_tests/desktop/writer/top_toolbar_spec.js | 6 +++---
5 files changed, 7 insertions(+), 7 deletions(-)

New commits:
commit 3635bed87243d0fc0705019a72620223d686c45a
Author: Caolán McNamara <caolan....@collabora.com>
AuthorDate: Thu Mar 12 19:44:52 2026 +0000
Commit: Caolán McNamara <cao...@gmail.com>
CommitDate: Fri Mar 13 13:52:53 2026 +0000

shortcut label for undo not visible

while that of redo is visible.

Undo's "ZZ" shortcut label was invisible because the
querySelector('[id^="home-undo"]'), intended to match something like
"home-undo13", instead matched the parent container of undo and redo
called "home-undo-redo".

And the box was ended up underneath Redo's "O" label.

Take the easy route here of renaming "home-undo-redo" to "home-do",
so it won't match.

Signed-off-by: Caolán McNamara <caolan....@collabora.com>
Change-Id: I92bcf92bc69d3205b2066792ab923d1d259fe9a7

diff --git a/browser/src/control/Control.NotebookbarCalc.js b/browser/src/control/Control.NotebookbarCalc.js
index 0e57362e91..866488af42 100644
--- a/browser/src/control/Control.NotebookbarCalc.js
+++ b/browser/src/control/Control.NotebookbarCalc.js
@@ -392,7 +392,7 @@ window.L.Control.NotebookbarCalc = window.L.Control.NotebookbarWriter.extend({
getHomeTab: function() {
var content = [
{
- 'id': 'home-undo-redo',
+ 'id': 'home-do',
'type': 'container',
'children': [
{
diff --git a/browser/src/control/Control.NotebookbarDraw.js b/browser/src/control/Control.NotebookbarDraw.js
index 1d0f80bca8..cb0112099a 100644
--- a/browser/src/control/Control.NotebookbarDraw.js
+++ b/browser/src/control/Control.NotebookbarDraw.js
@@ -586,7 +586,7 @@ window.L.Control.NotebookbarDraw = window.L.Control.NotebookbarImpress.extend({
getHomeTab: function() {
var content = [
{
- 'id': 'home-undo-redo',
+ 'id': 'home-do',
'type': 'container',
'children': [
{
diff --git a/browser/src/control/Control.NotebookbarImpress.js b/browser/src/control/Control.NotebookbarImpress.js
index 1d992f43be..62b40539c1 100644
--- a/browser/src/control/Control.NotebookbarImpress.js
+++ b/browser/src/control/Control.NotebookbarImpress.js
@@ -725,7 +725,7 @@ window.L.Control.NotebookbarImpress = window.L.Control.NotebookbarWriter.extend(
getHomeTab: function() {
var content = [
{
- 'id': 'home-undo-redo',
+ 'id': 'home-do',
'type': 'container',
'children': [
{
diff --git a/browser/src/control/Control.NotebookbarWriter.js b/browser/src/control/Control.NotebookbarWriter.js
index bd861f8124..5b91d4edab 100644
--- a/browser/src/control/Control.NotebookbarWriter.js
+++ b/browser/src/control/Control.NotebookbarWriter.js
@@ -580,7 +580,7 @@ window.L.Control.NotebookbarWriter = window.L.Control.Notebookbar.extend({
getHomeTab: function() {
var content = [
{
- 'id': 'home-undo-redo',
+ 'id': 'home-do',
'type': 'container',
'children': [
{
diff --git a/cypress_test/integration_tests/desktop/writer/top_toolbar_spec.js b/cypress_test/integration_tests/desktop/writer/top_toolbar_spec.js
index 96f6da981a..6e1bf2dff1 100644
--- a/cypress_test/integration_tests/desktop/writer/top_toolbar_spec.js
+++ b/cypress_test/integration_tests/desktop/writer/top_toolbar_spec.js
@@ -570,7 +570,7 @@ describe(['tagdesktop'], 'Top toolbar tests.', function() {

it.skip('Scroll', function() {
// Start all the way on the left side of the toolbar
- cy.cGet('#Home-container #home-undo-redo').should('be.visible');
+ cy.cGet('#Home-container #home-do').should('be.visible');
// TODO: Cypress thinks buttons are visible even though they are not
//cy.cGet('#Home-container #home-search-dialog').should('not.be.visible');
cy.cGet('#toolbar-up .ui-scroll-left').should('not.be.visible');
@@ -588,7 +588,7 @@ describe(['tagdesktop'], 'Top toolbar tests.', function() {

// Now we are all the way on the right side of the toolbar
// TODO: Cypress thinks buttons are visible even though they are not
- //cy.cGet('#Home-container #home-undo-redo').should('not.be.visible');
+ //cy.cGet('#Home-container #home-do').should('not.be.visible');
cy.cGet('#Home-container #home-search-dialog').should('be.visible');
cy.cGet('#toolbar-up .ui-scroll-left').should('be.visible');
cy.cGet('#toolbar-up .ui-scroll-right').should('not.be.visible');
@@ -604,7 +604,7 @@ describe(['tagdesktop'], 'Top toolbar tests.', function() {
});

// Now back on the left side of the toolbar
- cy.cGet('#Home-container #home-undo-redo').should('be.visible');
+ cy.cGet('#Home-container #home-do').should('be.visible');
// TODO: Cypress thinks buttons are visible even though they are not
//cy.cGet('#Home-container #home-search-dialog').should('not.be.visible');
cy.cGet('#toolbar-up .ui-scroll-left').should('not.be.visible');

"Pedro Pinto Silva (via github)"

unread,
Mar 18, 2026, 4:38:37 AMMar 18
to collaboraon...@googlegroups.com
browser/src/control/Control.JSDialogBuilder.js | 8 +
browser/src/control/Control.NotebookbarCalc.js | 6 +
cypress_test/integration_tests/desktop/calc/notebookbar_tooltip_spec.js | 42 ++++++++++
3 files changed, 56 insertions(+)

New commits:
commit 50769377bb89b73549eaa3a62935607a99853ebb
Author: Pedro Pinto Silva <pedro...@collabora.com>
AuthorDate: Mon Mar 16 13:50:57 2026 +0100
Commit: Szymon Kłos <eszk...@gmail.com>
CommitDate: Wed Mar 18 09:37:39 2026 +0100

calc notebookbar: add tooltips to InsertCalcTable btn (table styles)

The Table button in the Insert tab only works with .xlsx files since
ODF does not support table styles. Add tooltips that explain this:
- ODS: "Table styles are only available in .xlsx files"
- Alternatively we could use "Switch to .xlsx format to insert
table" but I was afraid that could be seen as motivating multiple
format switches (odf > ooxml -> odf -> ooxml) which is not a good
recommendation.
- XLSX enabled: "Insert a styled table"
- XLSX disabled: "Select a cell range first to insert a styled table"

Also add generic disabledTooltip support to the JSDialog toolitem
handler, so any button can show a different tooltip when disabled.

Includes Cypress tests for the ODS and XLSX tooltip states.

NOTE: For future plan we should probably reduce the number of steps
until the user can see the Table Design tab: In xlsx, show the Table
Design tab and have all the controls disable until the user selects
multiple cells and then a click in any of the controls of that Tab
would create the table and apply the style in one go.

Signed-off-by: Pedro Pinto Silva <pedro...@collabora.com>
Change-Id: I6f1fd81319037e7980f7355623c0f17c7892fad4

diff --git a/browser/src/control/Control.JSDialogBuilder.js b/browser/src/control/Control.JSDialogBuilder.js
index fceb87931a..4dc84faabc 100644
--- a/browser/src/control/Control.JSDialogBuilder.js
+++ b/browser/src/control/Control.JSDialogBuilder.js
@@ -1809,17 +1809,24 @@ window.L.Control.JSDialogBuilder = window.L.Control.extend({
if (data.visible === false)
div.classList.add('hidden');

+ let enabledTooltip = null;
const setDisabled = (disabled) => {
if (disabled) {
div.setAttribute('disabled', 'true');
if (button) {
button.setAttribute('aria-disabled', true);
}
+ if (data.disabledTooltip) {
+ div.setAttribute('data-cooltip', builder._cleanText(data.disabledTooltip));
+ }
} else {
div.removeAttribute('disabled');
if (button) {
button.removeAttribute('aria-disabled');
}
+ if (data.disabledTooltip && enabledTooltip) {
+ div.setAttribute('data-cooltip', enabledTooltip);
+ }
}
};

@@ -1916,6 +1923,7 @@ window.L.Control.JSDialogBuilder = window.L.Control.extend({
if (data.command && (!tooltip || !tooltip.includes('('))) // Add shortcut to tooltip based on command
tooltip = JSDialog.ShortcutsUtil.getShortcut(tooltip, data.command);
div.setAttribute('data-cooltip', tooltip);
+ enabledTooltip = tooltip;

// Set aria-pressed only if:
// 1. A real toggle button
diff --git a/browser/src/control/Control.NotebookbarCalc.js b/browser/src/control/Control.NotebookbarCalc.js
index 1c2b5e7b44..6d1eff9975 100644
--- a/browser/src/control/Control.NotebookbarCalc.js
+++ b/browser/src/control/Control.NotebookbarCalc.js
@@ -1763,6 +1763,12 @@ window.L.Control.NotebookbarCalc = window.L.Control.NotebookbarWriter.extend({
'type': 'bigtoolitem',
'text': _UNO('.uno:InsertCalcTable', 'spreadsheet'),
'command': '.uno:InsertCalcTable',
+ 'tooltip': app.LOUtil.isFileODF(this.map)
+ ? _('Table styles are only available in .xlsx files')
+ : _('Insert a styled table'),
+ 'disabledTooltip': app.LOUtil.isFileODF(this.map)
+ ? _('Table styles are only available in .xlsx files')
+ : _('Select a cell range first to insert a styled table'),
'accessibility': { focusBack: true, combination: 'IT', de: null }
}
]
diff --git a/cypress_test/integration_tests/desktop/calc/notebookbar_tooltip_spec.js b/cypress_test/integration_tests/desktop/calc/notebookbar_tooltip_spec.js
new file mode 100644
index 0000000000..9e01632ed6
--- /dev/null
+++ b/cypress_test/integration_tests/desktop/calc/notebookbar_tooltip_spec.js
@@ -0,0 +1,42 @@
+/* global describe it cy beforeEach require */
+
+var helper = require('../../common/helper');
+var desktopHelper = require('../../common/desktop_helper');
+var calcHelper = require('../../common/calc_helper');
+
+describe(['tagdesktop'], 'Notebookbar tooltip tests (ODS).', function() {
+
+ beforeEach(function() {
+ helper.setupAndLoadDocument('calc/top_toolbar.ods');
+ desktopHelper.switchUIToNotebookbar();
+ });
+
+ it('InsertCalcTable button shows ODS tooltip when disabled', function() {
+ cy.cGet('#Insert-tab-label').click();
+
+ cy.cGet('[modelid="insert-insert-calc-table"]')
+ .should('have.attr', 'disabled');
+ cy.cGet('[modelid="insert-insert-calc-table"]')
+ .should('have.attr', 'data-cooltip', 'Table styles are only available in .xlsx files');
+ });
+});
+
+describe(['tagdesktop'], 'Notebookbar tooltip tests (XLSX).', function() {
+
+ beforeEach(function() {
+ helper.setupAndLoadDocument('calc/testfile.xlsx');
+ desktopHelper.switchUIToNotebookbar();
+ });
+
+ it('InsertCalcTable button shows enabled tooltip when cells are selected', function() {
+ calcHelper.selectCellsInRange('A1:C3');
+
+ cy.cGet('#Insert-tab-label').click();
+
+ cy.cGet('[modelid="insert-insert-calc-table"]')
+ .should('not.have.attr', 'disabled');
+ cy.cGet('[modelid="insert-insert-calc-table"]')
+ .should('have.attr', 'data-cooltip', 'Insert a styled table');
+ });
+
+});

"Caolán McNamara (via github)"

unread,
Mar 19, 2026, 4:30:17 AMMar 19
to collaboraon...@googlegroups.com
browser/src/control/Control.JSDialog.js | 8 ++++++--
browser/src/control/jsdialog/Util.MessageRouter.ts | 6 +++++-
cypress_test/integration_tests/common/helper.js | 2 ++
3 files changed, 13 insertions(+), 3 deletions(-)

New commits:
commit a22df103f41eb90480c751c13ffd6f0b5e98fa52
Author: Caolán McNamara <caolan....@collabora.com>
AuthorDate: Fri Mar 13 11:36:27 2026 +0000
Commit: Miklos Vajna <vmi...@collabora.com>
CommitDate: Thu Mar 19 09:29:58 2026 +0100

Register jsdialog-deferred and wait for those in processToIdle

Failure context for 'Search wrap at document end':
cy:log ✱ >> findNext - start
cy:command ✔ cGet #search-button
cy:command ✔ assert expected **<button#search-button.ui-pushbutton.jsdialog.button-primary>** not to have attribute **disabled**
cy:command ✔ cGet #search-button
cy:command ✔ click
cy:command ✔ wrap null
cy:command ✔ assert .uno:ReportWhenIdle result with idleID 42: expected **{ Object (proxy, thisValue, ...) }** to be an object
cy:log ✱ << findNext - end
cy:log ✱ >> assertAddressAfterIdle - start
cy:log ✱ Param - expectedAddress: A1
cy:command ✔ wrap null
cy:command ✔ assert .uno:ReportWhenIdle result with idleID 43: expected **{ Object (proxy, thisValue, ...) }** to be an object
cy:command ✔ cGet #addressInput input
cy:command ✘ assert expected **<input#pos_window-input-address.ui-combobox-content.addressInput.jsdialog>** to have value **A1**, but the value was **A2**
cy:command ✔ fail:
Timed out retrying after 10000ms: expected '<input#pos_window-input-address.ui-combobox-content.addressInput.jsdialog>' to have value 'A1', but the value was 'A2'
/home/collabora/jenkins/workspace/github_online_master_debug_vs_co-26.04_cypress_desktop/cypress_test/integration_tests/common/calc_helper.js:299:42
297 | cy.log('Param - expectedAddress: ' + expectedAddress);
298 | helper.processToIdle(win);
> 299 | cy.cGet(helper.addressInputSelector).should('have.value', expectedAddress);
| ^
300 | cy.log('<< assertAddressAfterIdle - end');
301 | }
302 | module.exports.clickAtOffset = clickAt ...
cy:log ✱ Finishing test: integration_tests/desktop/calc/find_dialog_spec.js / Searching via find dialog. / Search wrap at document end
Builds:
- desktop#681 (PR#14909: Private/timar/fixtoc)
- desktop#686 (PR#14679: Revert \"cypress: skip some failing tests while po...)
- desktop#695 (PR#14952: Partly revert of 88bb87902: Fix context toolbar's ...)
- desktop#702 (PR#14954: css: unify UI font-family to single --cool-font to...)
- desktop#715 (PR#14947: add and use helpers::processToIdle for these tests...)
- desktop#716 (PR#14977: these calc dialog pass a11y testing now)

Signed-off-by: Caolán McNamara <caolan....@collabora.com>
Change-Id: I758dc7418cc884629412ebda09f3fce1734cbc60

diff --git a/browser/src/control/Control.JSDialog.js b/browser/src/control/Control.JSDialog.js
index 1cb963f9b3..4efecf55ac 100644
--- a/browser/src/control/Control.JSDialog.js
+++ b/browser/src/control/Control.JSDialog.js
@@ -298,12 +298,16 @@ window.L.Control.JSDialog = window.L.Control.extend({
let timeoutId = null;
const finallyClose = () => {
instance.that.close(instance.id, false);
- clearTimeout(timeoutId);
+ app.timerRegistry.clearTimeout(timeoutId);
};

container.onanimationend = finallyClose;
// be sure it will be removed if onanimationend will not be executed
- timeoutId = setTimeout(finallyClose, 700);
+ timeoutId = app.timerRegistry.setTimeout(
+ 'jsdialog-deferred',
+ finallyClose,
+ 700,
+ );
});
},

diff --git a/browser/src/control/jsdialog/Util.MessageRouter.ts b/browser/src/control/jsdialog/Util.MessageRouter.ts
index d7b45f9fb9..fd55d7e0ea 100644
--- a/browser/src/control/jsdialog/Util.MessageRouter.ts
+++ b/browser/src/control/jsdialog/Util.MessageRouter.ts
@@ -51,7 +51,11 @@ class JSDialogMessageRouter {
(msgData.jsontype === 'addressinputfield' &&
!app.socket._map.addressInputField)
) {
- setTimeout(fireJSDialogEvent, 1000);
+ app.timerRegistry.setTimeout(
+ 'jsdialog-deferred',
+ fireJSDialogEvent,
+ 1000,
+ );
return;
} else if (fireJSDialogEvent() === true) {
return;
diff --git a/cypress_test/integration_tests/common/helper.js b/cypress_test/integration_tests/common/helper.js
index 237c616c81..77190be9f1 100644
--- a/cypress_test/integration_tests/common/helper.js
+++ b/cypress_test/integration_tests/common/helper.js
@@ -1309,6 +1309,8 @@ function waitUntilLayoutingIsIdle(win) {

function processToIdle(win) {
return waitUntilCoreIsIdle(win).then(function() {
+ return waitForTimers(win, 'jsdialog-deferred');
+ }).then(function() {
return waitUntilLayoutingIsIdle(win);
});
}

"Szymon Kłos (via github)"

unread,
Mar 26, 2026, 7:09:36 AMMar 26
to collaboraon...@googlegroups.com
browser/src/control/Control.JSDialogBuilder.js | 2
cypress_test/integration_tests/desktop/writer/context_toolbar_spec.js | 30 ++++++++++
2 files changed, 31 insertions(+), 1 deletion(-)

New commits:
commit 5333fa18ab05d4797abdfdd8e879697bca736180
Author: Szymon Kłos <szymo...@collabora.com>
AuthorDate: Thu Mar 26 07:11:04 2026 +0000
Commit: Szymon Kłos <eszk...@gmail.com>
CommitDate: Thu Mar 26 12:08:43 2026 +0100

jsdialog: use provided instance of builder #15225

Fixes #15225

- we pass instance, not always callbacks are correctly bound

Signed-off-by: Szymon Kłos <szymo...@collabora.com>
Change-Id: Ifd39654d5ad5dccc703bacc26fa708d8573ce9e6

diff --git a/browser/src/control/Control.JSDialogBuilder.js b/browser/src/control/Control.JSDialogBuilder.js
index 726aa7c88b..66951e619c 100644
--- a/browser/src/control/Control.JSDialogBuilder.js
+++ b/browser/src/control/Control.JSDialogBuilder.js
@@ -340,7 +340,7 @@ window.L.Control.JSDialogBuilder = window.L.Control.extend({

dispatcher.dispatch('closeapp');
}
- this._defaultCallbackHandlerSendMessage(objectType, eventType, object, data, builder);
+ builder._defaultCallbackHandlerSendMessage(objectType, eventType, object, data, builder);
}
},

diff --git a/cypress_test/integration_tests/desktop/writer/context_toolbar_spec.js b/cypress_test/integration_tests/desktop/writer/context_toolbar_spec.js
new file mode 100644
index 0000000000..e19c43e597
--- /dev/null
+++ b/cypress_test/integration_tests/desktop/writer/context_toolbar_spec.js
@@ -0,0 +1,30 @@
+/* global describe it cy beforeEach require */
+
+var helper = require('../../common/helper');
+var desktopHelper = require('../../common/desktop_helper');
+
+describe(['tagdesktop'], 'Context toolbar tests.', function() {
+
+ beforeEach(function() {
+ cy.viewport(1920, 1080);
+ helper.setupAndLoadDocument('writer/top_toolbar.odt');
+ desktopHelper.switchUIToNotebookbar();
+ });
+
+ it('Font name combobox has previews.', function() {
+ // Double-click on text to select a word and trigger context toolbar
+ helper.getBlinkingCursorPosition('cursorPos');
+ helper.clickAt('cursorPos', true);
+
+ // Context toolbar should appear
+ cy.cGet('#context-toolbar').should('not.have.class', 'hidden');
+
+ // Open font name combobox in context toolbar
+ cy.cGet('#context-toolbar #fontnamecombobox .ui-combobox-button').click();
+
+ // Dropdown should appear with font preview images
+ cy.cGet('[id$="-dropdown"].modalpopup').should('be.visible');
+ cy.wait(1000); // Wait for custom entry rendering
+ cy.cGet('[id$="-dropdown"].modalpopup img').should('have.length.greaterThan', 0);
+ });
+});

"Miklos Vajna (via github)"

unread,
Mar 27, 2026, 4:49:36 AMMar 27
to collaboraon...@googlegroups.com
browser/src/app/ViewLayoutCompareChanges.ts | 9 +++++
browser/src/canvas/sections/CommentListSection.ts | 18 +++++++---
cypress_test/integration_tests/desktop/writer/track_changes_spec.js | 16 ++++++++
3 files changed, 38 insertions(+), 5 deletions(-)

New commits:
commit ce52a2c0c1d2022be42f4a21cebab07a5fb86887
Author: Miklos Vajna <vmi...@collabora.com>
AuthorDate: Thu Mar 26 10:11:08 2026 +0100
Commit: Caolán McNamara <cao...@gmail.com>
CommitDate: Fri Mar 27 08:49:02 2026 +0000

cool#15195 document compare: fix bad horizontal position of comments

Open the Writer bugdoc which has a comment, switch to doc compare mode,
the comment is between the "old first" and "new first" pages (which are
next to each other horizontally), while the expectation is that the
comment is on the right of the document content, so should be on the
right of the "new first" page.

It seems ViewLayoutMultiPage already has some code for this and
CommentListSection knows how to use that.

Fix this by making that mechanism more general: also implement a
getTotalSideSpace() in ViewLayoutCompareChanges, then improve
CommentListSection to work with "multi-column layouts" (multi-page view,
doc compare view), at a number of locations where previously it only
considered the multi-page view.

Test that comments are now over the right edge of the "new" pages.

Signed-off-by: Miklos Vajna <vmi...@collabora.com>
Change-Id: I746fe21d952b6d4da849878c7c3c31a672f1f1a0

diff --git a/browser/src/app/ViewLayoutCompareChanges.ts b/browser/src/app/ViewLayoutCompareChanges.ts
index 1c3e926dcf..376d0f5a58 100644
--- a/browser/src/app/ViewLayoutCompareChanges.ts
+++ b/browser/src/app/ViewLayoutCompareChanges.ts
@@ -198,6 +198,15 @@ class ViewLayoutCompareChanges extends ViewLayoutNewBase {
return this.viewSize.pX > Math.round(documentAnchor.size[0] * 0.5);
}

+ public getTotalSideSpace(): number {
+ Util.ensureValue(app.activeDocument);
+
+ const anchorWidth = this.getDocumentAnchorSection().size[0];
+ // Two pages side by side, with a gap in-between.
+ const contentWidth = 2 * app.activeDocument.fileSize.pX + this.viewGap;
+ return anchorWidth - contentWidth;
+ }
+
public override scroll(pX: number, pY: number): boolean {
const scrolled = super.scroll(pX, pY);

diff --git a/browser/src/canvas/sections/CommentListSection.ts b/browser/src/canvas/sections/CommentListSection.ts
index b31b480b3e..af43fb2a3a 100644
--- a/browser/src/canvas/sections/CommentListSection.ts
+++ b/browser/src/canvas/sections/CommentListSection.ts
@@ -355,9 +355,17 @@ export class CommentSection extends CanvasSectionObject {
}
}

+ /// If the current layout has more than one pages in a row, so the comment should be next to
+ /// the document content instead of next to the page.
+ private static isMultiColumnLayout(): boolean {
+ const type = app.activeDocument.activeLayout.type;
+ return type === 'ViewLayoutMultiPage' || type === 'ViewLayoutCompareChanges';
+ }
+
public calculateAvailableSpace() {
- if (app.activeDocument.activeLayout.type === 'ViewLayoutMultiPage') {
- const availableSpace = (app.activeDocument.activeLayout as ViewLayoutMultiPage).getTotalSideSpace();
+ if (CommentSection.isMultiColumnLayout()) {
+ const layout = app.activeDocument.activeLayout as ViewLayoutMultiPage | ViewLayoutCompareChanges;
+ const availableSpace = layout.getTotalSideSpace();
return Math.round(availableSpace * 0.5 / app.dpiScale);
}
else {
@@ -1416,7 +1424,7 @@ export class CommentSection extends CanvasSectionObject {
// This manually shows/hides comments
if (!this.sectionProperties.showResolved && app.map._docLayer._docType === 'text') {
let hide = annotation.isContainerVisible() && annotation.sectionProperties.data.resolved === 'true';
- hide = hide || (app.activeDocument.activeLayout.type === 'ViewLayoutMultiPage' && this.calculateAvailableSpace() < this.sectionProperties.collapsedCommentWidth);
+ hide = hide || (CommentSection.isMultiColumnLayout() && this.calculateAvailableSpace() < this.sectionProperties.collapsedCommentWidth);

if (hide && annotation.isContainerVisible()) {
if (this.sectionProperties.selectedComment == annotation) {
@@ -1431,7 +1439,7 @@ export class CommentSection extends CanvasSectionObject {
}
this.update();
}
- else if (app.activeDocument.activeLayout.type === 'ViewLayoutMultiPage') {
+ else if (CommentSection.isMultiColumnLayout()) {
const hide = this.calculateAvailableSpace() < this.sectionProperties.collapsedCommentWidth;

if (hide && annotation.isContainerVisible()) {
@@ -2198,7 +2206,7 @@ export class CommentSection extends CanvasSectionObject {
var selectedIndex = null;
var x = isRTL ? 0 : topRight[0];

- if (app.activeDocument.activeLayout.type === 'ViewLayoutMultiPage') {
+ if (CommentSection.isMultiColumnLayout()) {
x = topRight[0] - availableSpace;
}
else if (isRTL)
diff --git a/cypress_test/integration_tests/desktop/writer/track_changes_spec.js b/cypress_test/integration_tests/desktop/writer/track_changes_spec.js
index a00550e62d..1d01e30a7c 100644
--- a/cypress_test/integration_tests/desktop/writer/track_changes_spec.js
+++ b/cypress_test/integration_tests/desktop/writer/track_changes_spec.js
@@ -368,6 +368,22 @@ describe(['tagdesktop', 'tagnextcloud', 'tagproxy'], 'Track Changes', function (
cy.cGet('#comment-container-1').should(function($el) {
expect($el.position().top, 'comment top after scroll').to.be.lessThan(initialTop);
});
+
+ // Also verify the comment is to the right of the "new" page (right half),
+ // not between the two pages:
+ cy.getFrameWindow().then(function(win) {
+ // Compute the right edge of the right-side page in view coordinates.
+ var layout = win.app.activeDocument.activeLayout;
+ var rightEdgePoint = new win.cool.SimplePoint(win.app.activeDocument.fileSize.pX, 0);
+ rightEdgePoint.mode = 2; // TileMode.RightSide
+ var rightPageEdge = layout.documentToViewX(rightEdgePoint) / win.app.dpiScale;
+ cy.cGet('#comment-container-1').should(function($el) {
+ // Without the accompanying fix in place, this test would have failed with:
+ // - comment left position: expected 593 to be above 747
+ // i.e. the comment left is 1247.125 with the fix.
+ expect($el.position().left, 'comment left position').to.be.greaterThan(rightPageEdge);
+ });
+ });
});

"Mike Kaganski (via github)"

unread,
Apr 2, 2026, 7:56:16 AMApr 2
to collaboraon...@googlegroups.com
browser/src/canvas/sections/CommentListSection.ts | 16 +
browser/src/canvas/sections/CommentSection.ts | 6
browser/src/map/handler/Map.WOPI.js | 132 ++++++++++
cypress_test/integration_tests/desktop/calc/annotation_spec.js | 53 ++++
cypress_test/integration_tests/desktop/writer/annotation_spec.js | 85 ++++++
5 files changed, 288 insertions(+), 4 deletions(-)

New commits:
commit fc970b103cabe08f3075a43d595e589f4c1f1c5e
Author: Mike Kaganski <mike.k...@collabora.com>
AuthorDate: Tue Mar 31 11:50:26 2026 +0500
Commit: Miklos Vajna <vmi...@collabora.com>
CommitDate: Thu Apr 2 13:55:34 2026 +0200

Add Action_GoToComment WOPI postMessage, and resp. response

Navigate to a comment by ID via postMessage from the host.
Supports Writer (scroll + focus) and Calc (cross-sheet navigation
with a guard timer to prevent async events from hiding the comment).
Synchronizes calcMasterList on comment add/remove.
Includes cypress tests for Writer and Calc.

Signed-off-by: Mike Kaganski <mike.k...@collabora.com>
Change-Id: Ib6175ce047ea9acf8f1a9e1022a6d08644c07093

diff --git a/browser/src/canvas/sections/CommentListSection.ts b/browser/src/canvas/sections/CommentListSection.ts
index c9330ca268..a3aac0b719 100644
--- a/browser/src/canvas/sections/CommentListSection.ts
+++ b/browser/src/canvas/sections/CommentListSection.ts
@@ -142,6 +142,7 @@ export class CommentSection extends CanvasSectionObject {
this.sectionProperties.calcLastTab = -1;
this.sectionProperties.calcMasterList = [];
this.sectionProperties.calcCommandStateChanged = true;
+ this.sectionProperties.doNotHideCommentTimer = null; // For _goToCalcComment, where comment needs to show, despite async events trying to hide it
this.sectionProperties.commentList = new Array(0);
this.sectionProperties.selectedComment = null;
this.sectionProperties.arrow = null;
@@ -398,7 +399,9 @@ export class CommentSection extends CanvasSectionObject {

public hideAllComments (): void {
for (var i: number = 0; i < this.sectionProperties.commentList.length; i++) {
- this.sectionProperties.commentList[i].hide();
+ if (!this.sectionProperties.doNotHideCommentTimer
+ || this.sectionProperties.commentList[i] !== this.sectionProperties.selectedComment)
+ this.sectionProperties.commentList[i].hide();
var part = app.map._docLayer._selectedPart;
if (app.map._docLayer._docType === 'spreadsheet') {
// Change drawing order so they don't prevent each other from being shown.
@@ -746,6 +749,11 @@ export class CommentSection extends CanvasSectionObject {
break;
}
}
+ // Synchronize calcMasterList
+ var masterElementIndex = this.sectionProperties.calcMasterList.findIndex(el => el.id == id);
+ if (masterElementIndex >= 0) {
+ this.sectionProperties.calcMasterList.splice(masterElementIndex, 1);
+ }
this.checkSize();
app.map.fire('deleteannotation');
}
@@ -1397,7 +1405,7 @@ export class CommentSection extends CanvasSectionObject {

public onNewDocumentTopLeft (): void {
if (app.map._docLayer._docType === 'spreadsheet') {
- if (this.sectionProperties.selectedComment)
+ if (this.sectionProperties.selectedComment && !this.sectionProperties.doNotHideCommentTimer)
this.sectionProperties.selectedComment.hide();
}

@@ -1715,6 +1723,10 @@ export class CommentSection extends CanvasSectionObject {
return;
}

+ // Synchronize calcMasterList
+ if (app.map._docLayer._docType === 'spreadsheet' && obj.comment.id !== 'new')
+ this.sectionProperties.calcMasterList.push(structuredClone(obj.comment));
+
this.adjustComment(obj.comment);
annotation = this.add(obj.comment);
if (app.map._docLayer._docType === 'spreadsheet')
diff --git a/browser/src/canvas/sections/CommentSection.ts b/browser/src/canvas/sections/CommentSection.ts
index b935688c56..f99857bd9c 100644
--- a/browser/src/canvas/sections/CommentSection.ts
+++ b/browser/src/canvas/sections/CommentSection.ts
@@ -1489,8 +1489,10 @@ export class Comment extends CanvasSectionObject {
if (this.sectionProperties.data.rectangles[0].containsPoint(app.calc.cellCursorRectangle.center))
this.sectionProperties.commentListSection.sectionProperties.calcCurrentComment = this;
else if (this.isSelected()) {
- this.hide();
- this.sectionProperties.commentListSection.sectionProperties.calcCurrentComment = null;
+ if (!this.sectionProperties.commentListSection.sectionProperties.doNotHideCommentTimer) {
+ this.hide();
+ this.sectionProperties.commentListSection.sectionProperties.calcCurrentComment = null;
+ }
}
else if (this.sectionProperties.commentListSection.sectionProperties.calcCurrentComment == this)
this.sectionProperties.commentListSection.sectionProperties.calcCurrentComment = null;
diff --git a/browser/src/map/handler/Map.WOPI.js b/browser/src/map/handler/Map.WOPI.js
index 0954a40aab..f9aa1d6ff8 100644
--- a/browser/src/map/handler/Map.WOPI.js
+++ b/browser/src/map/handler/Map.WOPI.js
@@ -804,6 +804,22 @@ window.L.Map.WOPI = window.L.Handler.extend({
}
}
}
+ else if (msg.MessageId === 'Action_GoToComment') {
+ if (msg.Values) {
+ var commentSection = app.sectionContainer.getSectionWithName(app.CSections.CommentList.name);
+ if (!commentSection) {
+ this._sendGoToCommentResp(msg.Values.Id, false, 'Comment section not available');
+ return;
+ }
+ var docType = this._map._docLayer._docType;
+ if (docType === 'spreadsheet')
+ this._goToCalcComment(commentSection, msg.Values.Id);
+ else if (docType === 'text')
+ this._goToComment(commentSection, msg.Values.Id);
+ else
+ this._sendGoToCommentResp(msg.Values.Id, false, 'Unsupported document type'); // TODO: support Draw/Impress
+ }
+ }
else if (msg.sender === 'EIDEASY_SINGLE_METHOD_SIGNATURE') {
// This is produced by the esign popup.
const eSignature = this._map.eSignature;
@@ -813,6 +829,122 @@ window.L.Map.WOPI = window.L.Handler.extend({
}
},

+ _goToComment: function(commentSection, commentId) {
+ // Writer and Calc use different types for Id: string vs. integer?
+ var comment = commentSection.getComment(commentId);
+ if (!comment)
+ comment = commentSection.getComment(parseInt(commentId));
+ if (!comment) {
+ this._sendGoToCommentResp(commentId, false, 'Comment not found');
+ return;
+ }
+
+ this._map.showComments(true);
+ if (comment.sectionProperties.data.resolved === 'true')
+ this._map.showResolvedComments(true);
+
+ // Move the cursor to the comment's anchor
+ var clickX, clickY;
+ var cellRange = comment.sectionProperties.data.cellRange;
+ if (this._map._docLayer._docType === 'spreadsheet' && cellRange) {
+ var cellRect = this._map._docLayer._cellRangeToTwipRect(cellRange).toRectangle();
+ clickX = Math.round(cellRect[0] + cellRect[2] / 2);
+ clickY = Math.round(cellRect[1] + cellRect[3] / 2);
+ } else {
+ var anchorPos = comment.sectionProperties.data.anchorPos;
+ clickX = anchorPos[0];
+ clickY = anchorPos[1];
+ }
+ if (clickX && clickY) {
+ this._map._docLayer._postMouseEvent('buttondown', clickX, clickY, 1, 1, 0);
+ this._map._docLayer._postMouseEvent('buttonup', clickX, clickY, 1, 1, 0);
+ }
+
+ commentSection.navigateAndFocusComment(comment);
+
+ if (this._map._docLayer._docType === 'spreadsheet') {
+ // The sheet switch and mouse click (which sets cursor to anchor) trigger
+ // async events (_onSetPartMsg, onNewDocumentTopLeft, onCellAddressChanged)
+ // that would normally hide the comment. Set a guard to prevent that, show
+ // the comment, then clear the guard after a timeout to let events settle.
+ // 2 s timeout is an arbitrary value, hoped to cover typical cases, and at
+ // the same time, not block expected responsiveness, when user expects it
+ // to hide.
+ var props = commentSection.sectionProperties;
+ if (props.doNotHideCommentTimer)
+ clearTimeout(props.doNotHideCommentTimer);
+ props.doNotHideCommentTimer = setTimeout(function() {
+ props.doNotHideCommentTimer = null;
+ }, 2000);
+
+ // Finally, an additional operation specific to Calc (maybe also Draw?):
+ // it actually shows the comment on mouse hover
+ comment.onMouseEnter();
+ }
+
+ this._sendGoToCommentResp(commentId, true);
+ },
+
+ _goToCalcComment: function(commentSection, commentId) {
+ var map = this._map;
+ var props = commentSection.sectionProperties;
+
+ // Try to find the comment in the current calcMasterList.
+ var entry = props.calcMasterList.find(el => el.id == commentId);
+ if (!entry) {
+ this._sendGoToCommentResp(commentId, false, 'Comment not found');
+ return;
+ }
+
+ // If timeout from previous command is still active, stop it; and hide any shown comment,
+ // to avoid a case when doNotHideCommentTimer would prevent another comment from hiding.
+ if (props.doNotHideCommentTimer)
+ clearTimeout(props.doNotHideCommentTimer);
+ props.doNotHideCommentTimer = null;
+ commentSection.hideAllComments();
+
+ var targetTab = parseInt(entry.tab);
+ if (map._docLayer._selectedPart == targetTab) {
+ // Already on the right sheet - navigate immediately.
+ this._goToComment(commentSection, commentId);
+ return;
+ }
+
+ // The comment is on a different sheet. Switch to it and wait for
+ // sheetgeometrychanged so the geometry is ready for positioning.
+
+ var safetyTimer = null;
+ var self = this;
+
+ function cleanup() {
+ clearTimeout(safetyTimer);
+ map.off('sheetgeometrychanged', onGeometry);
+ }
+
+ function onGeometry() {
+ cleanup();
+ self._goToComment(commentSection, commentId);
+ }
+
+ safetyTimer = setTimeout(function() {
+ cleanup();
+ self._sendGoToCommentResp(commentId, false, 'Timed out waiting for server');
+ }, 10000);
+
+ map.once('sheetgeometrychanged', onGeometry);
+ map.setPart(targetTab);
+ },
+
+ _sendGoToCommentResp: function(commentId, success, errorMsg) {
+ var args = { Id: String(commentId), success: success };
+ if (errorMsg)
+ args.errorMsg = errorMsg;
+ this._map.fire('postMessage', {
+ msgId: 'Action_GoToComment_Resp',
+ args: args
+ });
+ },
+
_postMessage: function(e) {
if (!this.enabled) { return; }
var msgId = e.msgId;
diff --git a/cypress_test/integration_tests/desktop/calc/annotation_spec.js b/cypress_test/integration_tests/desktop/calc/annotation_spec.js
index 3aa2de53f8..dfe3538e08 100644
--- a/cypress_test/integration_tests/desktop/calc/annotation_spec.js
+++ b/cypress_test/integration_tests/desktop/calc/annotation_spec.js
@@ -204,6 +204,59 @@ describe(['tagdesktop'], 'Annotation Tests', function() {
desktopHelper.assertScrollbarPosition('vertical', 249, 252);
});

+ it('Action_GoToComment postMessage navigates to a comment across sheets', function() {
+ // Put text and a comment in cell B2 on Sheet 1.
+ calcHelper.enterCellAddressAndConfirm(this.win, 'B2');
+ helper.typeIntoDocument('COMMENT_CELL{enter}');
+ calcHelper.enterCellAddressAndConfirm(this.win, 'B2');
+ desktopHelper.insertComment();
+ cy.cGet('#comment-container-1').should('exist');
+
+ // Move to a faraway cell, to check that GoToComment will scroll back
+ calcHelper.enterCellAddressAndConfirm(this.win, 'Z1000');
+ helper.typeIntoDocument('FAR_AWAY_CELL{enter}');
+
+ // Create a new sheet and put text there.
+ cy.cGet('#spreadsheet-toolbar #insertsheet').click();
+ calcHelper.assertNumberofSheets(2);
+ calcHelper.enterCellAddressAndConfirm(this.win, 'A1');
+ helper.typeIntoDocument('SHEET2_CELL{enter}');
+
+ // We are now on Sheet 2; the comment is on Sheet 1.
+
+ // Stub postMessage to capture the response.
+ cy.getFrameWindow().then(win => {
+ cy.stub(win.parent, 'postMessage').as('postMessage');
+ });
+
+ // Send Action_GoToComment postMessage with the comment's Id.
+ cy.getFrameWindow().then(win => {
+ var message = {
+ 'MessageId': 'Action_GoToComment',
+ 'Values': { 'Id': '1' }
+ };
+ win.postMessage(JSON.stringify(message), '*');
+ });
+
+ // Verify the response postMessage was sent with success and no error.
+ cy.get('@postMessage').should(stub => {
+ var calls = stub.getCalls().filter(call => {
+ try {
+ var msg = typeof call.args[0] === 'string' ? JSON.parse(call.args[0]) : call.args[0];
+ return msg.MessageId === 'Action_GoToComment_Resp';
+ } catch (e) { return false; }
+ });
+ expect(calls.length, 'Action_GoToComment_Resp was not posted').to.be.greaterThan(0);
+ var resp = typeof calls[0].args[0] === 'string' ? JSON.parse(calls[0].args[0]) : calls[0].args[0];
+ expect(resp.Values.success, 'Action_GoToComment_Resp reported error: ' + resp.Values.errorMsg).to.be.true;
+ expect(resp.Values.Id).to.equal('1');
+ });
+
+ // The comment should be visible and the anchor cell (B2) selected.
+ cy.cGet('#comment-container-1').should('be.visible');
+ cy.cGet(helper.addressInputSelector).should('have.prop', 'value', 'B2');
+ });
+
});

describe(['tagdesktop'], 'Annotation Autosave Tests', function() {
diff --git a/cypress_test/integration_tests/desktop/writer/annotation_spec.js b/cypress_test/integration_tests/desktop/writer/annotation_spec.js
index 166a0e24df..a0d153e943 100644
--- a/cypress_test/integration_tests/desktop/writer/annotation_spec.js
+++ b/cypress_test/integration_tests/desktop/writer/annotation_spec.js
@@ -297,6 +297,91 @@ describe(['tagdesktop'], 'Annotation Tests', function() {
cy.get('@windowOpen').should('be.called');
});

+ it('Action_GoToComment postMessage navigates to a comment', function() {
+ // Type identifiable text on the first line where the comment will be.
+ helper.typeIntoDocument('COMMENT_ANCHOR_LINE');
+
+ desktopHelper.insertComment();
+
+ cy.cGet('.cool-annotation-content-wrapper').should('be.visible');
+ cy.cGet('#annotation-content-area-1').should('contain', 'some text0');
+
+ // Type lots of paragraph breaks so the document scrolls well past the comment.
+ helper.typeIntoDocument('{ctrl}{end}');
+ helper.typeIntoDocument('{enter}'.repeat(80) + 'BOTTOM_OF_DOCUMENT');
+
+ // The comment should now be scrolled out of view.
+ cy.cGet('#comment-container-1').should('not.be.visible');
+
+ // Stub postMessage to capture the response.
+ cy.getFrameWindow().then(win => {
+ cy.stub(win.parent, 'postMessage').as('postMessage');
+ });
+
+ // Send Action_GoToComment postMessage with the comment's Id.
+ cy.getFrameWindow().then(win => {
+ var message = {
+ 'MessageId': 'Action_GoToComment',
+ 'Values': { 'Id': '1' }
+ };
+ win.postMessage(JSON.stringify(message), '*');
+ });
+
+ // Verify the response postMessage was sent with success and no error.
+ cy.get('@postMessage').should(stub => {
+ var calls = stub.getCalls().filter(call => {
+ try {
+ var msg = typeof call.args[0] === 'string' ? JSON.parse(call.args[0]) : call.args[0];
+ return msg.MessageId === 'Action_GoToComment_Resp';
+ } catch (e) { return false; }
+ });
+ expect(calls.length, 'Action_GoToComment_Resp was not posted').to.be.greaterThan(0);
+ var resp = typeof calls[0].args[0] === 'string' ? JSON.parse(calls[0].args[0]) : calls[0].args[0];
+ expect(resp.Values.success, 'Action_GoToComment_Resp reported error: ' + resp.Values.errorMsg).to.be.true;
+ expect(resp.Values.Id).to.equal('1');
+ });
+
+ // After GoToComment, the comment should be scrolled back into view.
+ cy.cGet('#comment-container-1').should('be.visible');
+ // The cursor should be at the end of the first paragraph (the comment anchor).
+ // #clipboard-area has a copy of current cursor's node text (including anchor character):
+ cy.cGet('#clipboard-area').should('have.prop', 'textContent', 'COMMENT_ANCHOR_LINE\uFFFC');
+ cy.getFrameWindow().then(win => {
+ var textInput = win.app.map._textInput;
+ expect(textInput._lastSelectionStart).to.equal(20);
+ expect(textInput._lastSelectionEnd).to.equal(20);
+ });
+ });
+
+ it('Action_GoToComment postMessage returns error for invalid comment', function() {
+ // Stub postMessage to capture the response.
+ cy.getFrameWindow().then(win => {
+ cy.stub(win.parent, 'postMessage').as('postMessage');
+ });
+
+ // Send Action_GoToComment with a non-existent comment Id.
+ cy.getFrameWindow().then(win => {
+ var message = {
+ 'MessageId': 'Action_GoToComment',
+ 'Values': { 'Id': '999' }
+ };
+ win.postMessage(JSON.stringify(message), '*');
+ });
+
+ // Verify error response was sent.
+ cy.get('@postMessage').should(stub => {
+ var found = stub.getCalls().some(call => {
+ try {
+ var msg = typeof call.args[0] === 'string' ? JSON.parse(call.args[0]) : call.args[0];
+ return msg.MessageId === 'Action_GoToComment_Resp'
+ && msg.Values && msg.Values.success === false
+ && msg.Values.Id === '999';
+ } catch (e) { return false; }
+ });
+ expect(found, 'Action_GoToComment_Resp with failure was not posted').to.be.true;
+ });
+ });
+
});

describe(['tagdesktop'], 'Collapsed Annotation Tests', function() {

"Gökay Şatır (via github)"

unread,
Apr 7, 2026, 10:18:30 AMApr 7
to collaboraon...@googlegroups.com
browser/src/canvas/sections/ScrollSection.ts | 9
cypress_test/integration_tests/desktop/writer/compare_changes_scroll_spec.js | 191 ----------
2 files changed, 3 insertions(+), 197 deletions(-)

New commits:
commit b3489f1f7ecc59d5c15ee3d2985e6dfa44a0b476
Author: Gökay Şatır <gokay...@collabora.com>
AuthorDate: Mon Mar 30 15:58:16 2026 +0300
Commit: Gökay Şatır <gokay...@gmail.com>
CommitDate: Tue Apr 7 17:18:11 2026 +0300

Follow up improvements for compare changes view.

Follow up to: https://github.com/CollaboraOnline/online/pull/15240

Fixes available width calculation of compare changes view (small diff).
Removes an extra call to "canScrollVertical" function.
Renames the new Compare Changes view test to a more general and reusable name.

Signed-off-by: Gökay Şatır <gokay...@collabora.com>
Change-Id: I5acd70ea47ed3c68b70e745434ad8b0bfa92e558

diff --git a/browser/src/canvas/sections/ScrollSection.ts b/browser/src/canvas/sections/ScrollSection.ts
index a0c98d80ba..d8a04e7462 100644
--- a/browser/src/canvas/sections/ScrollSection.ts
+++ b/browser/src/canvas/sections/ScrollSection.ts
@@ -782,13 +782,10 @@ export class ScrollSection extends CanvasSectionObject {
const mirrorX = this.isRTL();
const documentAnchor = app.sectionContainer.getSectionWithName(app.CSections.Tiles.name);

- // For CompareChanges view, viewedRectangle.pY1 can be negative (due to
- // yStart offset) even when scrolling is possible, so use canScrollVertical instead.
- const canScrollV = layout.type === 'ViewLayoutCompareChanges'
- ? layout.canScrollVertical(documentAnchor)
- : layout.viewedRectangle.pY1 >= 0;
+ // For CompareChanges view, viewedRectangle.pY1 can be negative while scrolling is possible.
+ const initialVerticalCheck = layout.type === 'ViewLayoutCompareChanges' ? true : layout.viewedRectangle.pY1 >= 0;

- if (canScrollV) {
+ if (initialVerticalCheck) {
if ((!mirrorX && point.pX >= this.size[0] - scrollProps.usableThickness)
|| (mirrorX && point.pY <= scrollProps.usableThickness)) {
if (point.pY > scrollProps.yOffset) {
diff --git a/cypress_test/integration_tests/desktop/writer/compare_changes_scroll_spec.js b/cypress_test/integration_tests/desktop/writer/compare_changes_scroll_spec.js
deleted file mode 100644
index 8ef024f299..0000000000
--- a/cypress_test/integration_tests/desktop/writer/compare_changes_scroll_spec.js
+++ /dev/null
@@ -1,191 +0,0 @@
-/* global describe it cy beforeEach require Cypress expect */
-
-var helper = require('../../common/helper');
-var desktopHelper = require('../../common/desktop_helper');
-
-describe(['tagdesktop', 'tagnextcloud', 'tagproxy'], 'Compare Changes view scrolling.', function() {
-
- beforeEach(function() {
- cy.viewport(1400, 600);
- helper.setupAndLoadDocument('writer/track_changes.odt');
- desktopHelper.switchUIToNotebookbar();
- cy.cGet('#Review-tab-label').click();
- });
-
- function enterCompareChangesMode() {
- desktopHelper.getNbIconArrow('TrackChanges', 'Review').click();
- cy.cGet('#compare-tracked-change-button').filter(':visible').click();
- cy.cGet('.compare-changes-labels').should('not.have.css', 'display', 'none');
- }
-
- // Regression test for: compare changes view: fix "scroll by pressing scrollbar" feature.
- // Bug: In CompareChanges view, viewedRectangle.pY1 can be negative (due to
- // yStart offset) even when scrolling is possible, so the scrollbar click was
- // blocked. The fix uses canScrollVertical/canScrollHorizontal instead.
- it('Scrollbar vertical scroll works in compare changes mode.', function() {
- enterCompareChangesMode();
-
- cy.getFrameWindow().then(function(win) {
- helper.processToIdle(win);
- });
-
- // Record initial viewY.
- var initialViewY;
- cy.getFrameWindow().then(function(win) {
- var layout = win.app.activeDocument.activeLayout;
- expect(layout.type).to.equal('ViewLayoutCompareChanges');
- initialViewY = layout.scrollProperties.viewY;
- });
-
- // Scroll down using the scroll section offset API (simulates scrollbar drag).
- cy.getFrameWindow().then(function(win) {
- win.app.sectionContainer.getSectionWithName('scroll').scrollVerticalWithOffset(200);
- });
-
- // viewY should increase even though viewedRectangle.pY1 might be negative.
- cy.getFrameWindow().then(function(win) {
- cy.wrap(null).should(function() {
- var layout = win.app.activeDocument.activeLayout;
- expect(layout.scrollProperties.viewY, 'viewY after vertical scroll').to.be.greaterThan(initialViewY);
- });
- });
- });
-
- // Verify that canScrollVertical returns true in compare changes mode
- // even when viewedRectangle.pY1 is negative (due to yStart offset).
- it('canScrollVertical is true in compare changes mode.', function() {
- enterCompareChangesMode();
-
- cy.getFrameWindow().then(function(win) {
- helper.processToIdle(win);
- });
-
- cy.getFrameWindow().then(function(win) {
- var layout = win.app.activeDocument.activeLayout;
- var documentAnchor = win.app.sectionContainer.getSectionWithName(win.app.CSections.Tiles.name);
-
- // canScrollVertical should be true (the document is taller than the viewport).
- expect(layout.canScrollVertical(documentAnchor), 'canScrollVertical').to.be.true;
- });
- });
-
- // Regression test for: comparechanges view: fix onresize and vertical scroll.
- // Bug: When the browser window was resized, the compare changes layout was not
- // refreshed, so the two side-by-side views were not repositioned correctly.
- // The fix adds a resize handler that updates halfWidth and refreshes the view.
- it('Resize updates compare changes layout.', function() {
- enterCompareChangesMode();
-
- cy.getFrameWindow().then(function(win) {
- helper.processToIdle(win);
- });
-
- // Ensure sidebar is closed so we start from a known state.
- cy.cGet('#sidebar-dock-wrapper').then(function($el) {
- if (Cypress.dom.isVisible($el[0])) {
- desktopHelper.sidebarToggle();
- cy.cGet('#sidebar-dock-wrapper').should('not.be.visible');
- }
- });
-
- cy.getFrameWindow().then(function(win) {
- helper.processToIdle(win);
- });
-
- // Record halfWidth with sidebar closed (full canvas width).
- cy.getFrameWindow().then(function(win) {
- var layout = win.app.activeDocument.activeLayout;
- expect(layout.halfWidth, 'initial halfWidth').to.be.greaterThan(0);
- cy.wrap(layout.halfWidth).as('initialHalfWidth');
- });
-
- // Open sidebar to reduce canvas width, which fires the resize event.
- desktopHelper.sidebarToggle();
- cy.cGet('#sidebar-dock-wrapper').should('be.visible');
-
- // After sidebar opens, halfWidth should decrease since the
- // document anchor section is narrower.
- cy.get('@initialHalfWidth').then(function(initialHalfWidth) {
- cy.getFrameWindow().then(function(win) {
- cy.wrap(null, {timeout: 10000}).should(function() {
- var newHalfWidth = win.app.activeDocument.activeLayout.halfWidth;
- expect(newHalfWidth, 'halfWidth after sidebar toggle').to.not.equal(initialHalfWidth);
- });
- });
- });
- });
-
- // Verify that viewSize covers both left and right document pages.
- // Bug: viewSize.pX was calculated using halfWidth instead of full canvas width,
- // so the horizontal extent did not account for both pages.
- it('View size covers both pages in compare changes mode.', function() {
- enterCompareChangesMode();
-
- cy.getFrameWindow().then(function(win) {
- helper.processToIdle(win);
- });
-
- cy.getFrameWindow().then(function(win) {
- var layout = win.app.activeDocument.activeLayout;
- var documentAnchor = win.app.sectionContainer.getSectionWithName(win.app.CSections.Tiles.name);
-
- // viewSize.pX should be at least as wide as the canvas (both pages fit).
- expect(layout.viewSize.pX, 'viewSize.pX').to.be.at.least(documentAnchor.size[0]);
- });
- });
-
- // Regression test for: compare changes view: tile rendering issue.
- // Bug: The visible area rectangle (viewedRectangle) was too narrow, so tiles
- // for the left page were not requested. The fix widens it to cover the full
- // document width so that both left-side and right-side tiles are rendered.
- it('Visible area covers full document width for tile rendering.', function() {
- enterCompareChangesMode();
-
- cy.getFrameWindow().then(function(win) {
- helper.processToIdle(win);
- });
-
- // The viewedRectangle should be wide enough to cover the full document.
- cy.getFrameWindow().then(function(win) {
- cy.wrap(null).should(function() {
- var layout = win.app.activeDocument.activeLayout;
- var docWidth = win.app.activeDocument.fileSize.pX;
-
- // viewedRectangle width must cover the full document width (both pages
- // need tiles from the same X range). The fix added margin, so the
- // rectangle width should be >= document width.
- expect(layout.viewedRectangle.width, 'viewedRectangle covers document width').to.be.at.least(docWidth);
- });
- });
- });
-
- // Verify both tile modes (LeftSide and RightSide) have content after the
- // tile rendering fix. This complements the existing "View Changes mode has
- // tiles for both modes" test by checking specifically after the visible area fix.
- it('Both left and right side tiles have content after visible area fix.', function() {
- enterCompareChangesMode();
-
- // Wait for tiles to be rendered.
- cy.getFrameWindow().then(function(win) {
- helper.processToIdle(win);
- });
-
- cy.getFrameWindow().then(function(win) {
- cy.wrap(null).should(function() {
- var tiles = win.TileManager.getTiles();
- var hasMode1 = false;
- var hasMode2 = false;
- tiles.forEach(function(tile) {
- if (tile.coords.mode === 1 && tile.hasContent() && tile.distanceFromView < Number.MAX_SAFE_INTEGER) {
- hasMode1 = true;
- }
- if (tile.coords.mode === 2 && tile.hasContent() && tile.distanceFromView < Number.MAX_SAFE_INTEGER) {
- hasMode2 = true;
- }
- });
- expect(hasMode1, 'mode=1 (LeftSide) tiles with content').to.be.true;
- expect(hasMode2, 'mode=2 (RightSide) tiles with content').to.be.true;
- });
- });
- });
-});

"Caolán McNamara (via github)"

unread,
Apr 15, 2026, 9:30:43 AM (14 days ago) Apr 15
to collaboraon...@googlegroups.com
browser/src/app/GraphicSelectionMiddleware.ts | 6 ++++-
cypress_test/integration_tests/desktop/impress/image_operation_spec.js | 12 ++++++++++
2 files changed, 17 insertions(+), 1 deletion(-)

New commits:
commit c4746ce11cceb130a109f18d3a47557cb6dc5783
Author: Caolán McNamara <caolan....@collabora.com>
AuthorDate: Wed Apr 15 07:58:11 2026 +0000
Commit: Miklos Vajna <vmi...@collabora.com>
CommitDate: Wed Apr 15 15:30:27 2026 +0200

fix embedded video svg dimensions

regression from:

commit 221ee06f481cfaf67a3bcf3d4d5814196c46b2c7
Date: Wed Jan 14 08:54:42 2026 +0000

tidy shapehandle svg data

which added a svg container to the embeddedobj, but its defaults
are 300x150 pixels.

Extended the cypress video test to check for sizes

Signed-off-by: Caolán McNamara <caolan....@collabora.com>
Change-Id: I34c3f0506e7949028d30b1b3b13ad472061e6742

diff --git a/browser/src/app/GraphicSelectionMiddleware.ts b/browser/src/app/GraphicSelectionMiddleware.ts
index 8ad1644600..9ebdbf796c 100644
--- a/browser/src/app/GraphicSelectionMiddleware.ts
+++ b/browser/src/app/GraphicSelectionMiddleware.ts
@@ -65,7 +65,11 @@ class GraphicSelection {

var videoToInsert =
'<?xml version="1.0" encoding="UTF-8"?>\
- <svg xmlns="http://www.w3.org/2000/svg">\
+ <svg xmlns="http://www.w3.org/2000/svg" width="' +
+ videoDesc.width +
+ '" height="' +
+ videoDesc.height +
+ '">\
<foreignObject overflow="visible" width="' +
videoDesc.width +
'" height="' +
diff --git a/cypress_test/integration_tests/desktop/impress/image_operation_spec.js b/cypress_test/integration_tests/desktop/impress/image_operation_spec.js
index 36ed1c8876..062b0e01fd 100644
--- a/cypress_test/integration_tests/desktop/impress/image_operation_spec.js
+++ b/cypress_test/integration_tests/desktop/impress/image_operation_spec.js
@@ -24,6 +24,18 @@ describe(['tagdesktop'], 'Image Operation Tests', function() {

it("Insert multimedia", function () {
desktopHelper.insertVideo();
+
+ // The video foreignObject lives inside a nested <svg> wrapper.
+ // Verify the wrapper has explicit dimensions so it does not
+ // fall back to the SVG default 300x150 and clip the video.
+ cy.cGet('#document-container svg svg').should('have.attr', 'width');
+ cy.cGet('#document-container svg svg').should('have.attr', 'height');
+ cy.cGet('#document-container svg svg foreignObject').then(function ($fo) {
+ var foWidth = $fo.attr('width');
+ var foHeight = $fo.attr('height');
+ cy.cGet('#document-container svg svg').should('have.attr', 'width', foWidth);
+ cy.cGet('#document-container svg svg').should('have.attr', 'height', foHeight);
+ });
});

it.skip('Crop Image', function () {

"Mike Kaganski (via github)"

unread,
Apr 16, 2026, 3:34:25 AM (13 days ago) Apr 16
to collaboraon...@googlegroups.com
browser/src/map/handler/Map.WOPI.js | 40 ++++++
cypress_test/integration_tests/desktop/calc/annotation_spec.js | 62 ++++++++++
cypress_test/integration_tests/desktop/writer/annotation_spec.js | 45 +++++++
3 files changed, 147 insertions(+)

New commits:
commit da6d3b15f686e9f5883be7c18f7cdde41f36d478
Author: Mike Kaganski <mike.k...@collabora.com>
AuthorDate: Wed Apr 15 16:53:09 2026 +0500
Commit: Miklos Vajna <vmi...@collabora.com>
CommitDate: Thu Apr 16 09:33:22 2026 +0200

Add Get_Comments WOPI postMessage command

Allow WOPI hosts to query all document comments via a Get_Comments
postMessage. Returns Id, Author, DateTime, Text, and for Writer /
Calc threaded comments also Resolved and Parent. For Calc, returns
comments from all sheets.

Signed-off-by: Mike Kaganski <mike.k...@collabora.com>
Change-Id: I5d48549691f405df8c13f81ee52f733b7ca9116e

diff --git a/browser/src/map/handler/Map.WOPI.js b/browser/src/map/handler/Map.WOPI.js
index fb11c18329..16bd6b9c1d 100644
--- a/browser/src/map/handler/Map.WOPI.js
+++ b/browser/src/map/handler/Map.WOPI.js
@@ -717,6 +717,46 @@ window.L.Map.WOPI = window.L.Handler.extend({

this._postMessage({msgId: 'Get_Export_Formats_Resp', args: exportFormatsResp});
}
+ else if (msg.MessageId === 'Get_Comments') {
+ let commentsResp = [];
+ const commentSection = app.sectionContainer.getSectionWithName(app.CSections.CommentList.name);
+ if (commentSection) {
+ if (this._map._docLayer._docType === 'spreadsheet') {
+ // calcMasterList has raw data for all sheets.
+ const masterList = commentSection.sectionProperties.calcMasterList;
+ for (let i = 0; i < masterList.length; i++) {
+ const data = masterList[i];
+ const entry = {
+ Id: data.id,
+ Author: data.author,
+ DateTime: data.dateTime,
+ Text: data.text,
+ };
+ if (data.threaded) {
+ entry.Resolved = data.resolved;
+ entry.Parent = data.parent;
+ }
+ commentsResp.push(entry);
+ }
+ } else {
+ const commentList = commentSection.sectionProperties.commentList;
+ for (let i = 0; i < commentList.length; i++) {
+ const data = commentList[i].sectionProperties.data;
+ if (data.trackchange)
+ continue;
+ commentsResp.push({
+ Id: data.id,
+ Author: data.author,
+ DateTime: data.dateTime,
+ Text: commentList[i].sectionProperties.contentText.textContent,
+ Resolved: data.resolved,
+ Parent: data.parent,
+ });
+ }
+ }
+ }
+ this._postMessage({msgId: 'Get_Comments_Resp', args: { Comments: commentsResp }});
+ }
else if (msg.MessageId === 'Action_SaveAs') {
if (msg.Values) {
if (msg.Values.Filename !== null && msg.Values.Filename !== undefined) {
diff --git a/cypress_test/integration_tests/desktop/calc/annotation_spec.js b/cypress_test/integration_tests/desktop/calc/annotation_spec.js
index e961eb89f0..516a132cb2 100644
--- a/cypress_test/integration_tests/desktop/calc/annotation_spec.js
+++ b/cypress_test/integration_tests/desktop/calc/annotation_spec.js
@@ -257,6 +257,68 @@ describe(['tagdesktop'], 'Annotation Tests', function() {
cy.cGet(helper.addressInputSelector).should('have.prop', 'value', 'B2');
});

+ it('Get_Comments postMessage returns note and threaded comments from all sheets', function() {
+ // Insert a legacy Note comment on Sheet 1.
+ calcHelper.enterCellAddressAndConfirm(this.win, 'B2');
+ desktopHelper.insertComment('note comment');
+ cy.cGet('#comment-container-1').should('exist');
+
+ // Avoid notebookbar collapse before the next insertion.
+ cy.cGet('#Home-tab-label').click();
+
+ // Insert a Threaded comment on Sheet 1.
+ calcHelper.enterCellAddressAndConfirm(this.win, 'D4');
+ cy.cGet('#Insert-tab-label').click();
+ cy.cGet('#insert-insert-threaded-comment').click();
+ cy.cGet('#comment-container-new').should('exist');
+ cy.cGet('.cool-annotation').last().find('.modify-annotation .cool-annotation-textarea')
+ .should('not.have.attr', 'disabled');
+ cy.cGet('.cool-annotation').last().find('.modify-annotation .cool-annotation-textarea')
+ .type('threaded comment', {force: true});
+ cy.cGet('.cool-annotation').last().find('[value="Save"]').click({force: true});
+ cy.cGet('#comment-container-2').should('exist');
+ cy.getFrameWindow().then(win => { helper.processToIdle(win); });
+
+ // Create a new sheet and switch to it, so we're not on the sheet with comments.
+ cy.cGet('#spreadsheet-toolbar #insertsheet').click();
+ calcHelper.assertNumberofSheets(2);
+
+ // Stub postMessage to capture the response.
+ cy.getFrameWindow().then(win => {
+ cy.stub(win.parent, 'postMessage').as('postMessage');
+ });
+
+ // Send Get_Comments postMessage from Sheet 2.
+ cy.getFrameWindow().then(win => {
+ const message = { 'MessageId': 'Get_Comments' };
+ win.postMessage(JSON.stringify(message), '*');
+ });
+
+ // Verify the response contains both comments from Sheet 1.
+ cy.get('@postMessage').should(stub => {
+ const calls = stub.getCalls().filter(call => {
+ try {
+ const msg = typeof call.args[0] === 'string' ? JSON.parse(call.args[0]) : call.args[0];
+ return msg.MessageId === 'Get_Comments_Resp';
+ } catch (e) { return false; }
+ });
+ expect(calls.length, 'Get_Comments_Resp was not posted').to.be.greaterThan(0);
+ const resp = typeof calls[0].args[0] === 'string' ? JSON.parse(calls[0].args[0]) : calls[0].args[0];
+ const comments = resp.Values.Comments;
+ expect(comments.length).to.equal(2);
+ // Legacy Note: has Text, Author, DateTime; no Resolved.
+ expect(comments[0].Text).to.equal('note comment');
+ expect(comments[0]).to.have.property('Author');
+ expect(comments[0]).to.have.property('DateTime');
+ expect(comments[0]).to.not.have.property('Resolved');
+ // Threaded comment: has Text, Author, DateTime, Resolved.
+ expect(comments[1].Text).to.equal('threaded comment');
+ expect(comments[1]).to.have.property('Author');
+ expect(comments[1]).to.have.property('DateTime');
+ expect(comments[1].Resolved).to.equal('false');
+ });
+ });
+
});

describe(['tagdesktop'], 'Annotation Autosave Tests', function() {
diff --git a/cypress_test/integration_tests/desktop/writer/annotation_spec.js b/cypress_test/integration_tests/desktop/writer/annotation_spec.js
index d3471c9a8a..ed8f11ba26 100644
--- a/cypress_test/integration_tests/desktop/writer/annotation_spec.js
+++ b/cypress_test/integration_tests/desktop/writer/annotation_spec.js
@@ -404,6 +404,51 @@ describe(['tagdesktop'], 'Annotation Tests', function() {
.should(el => expect(el.width()).gte(200));
});

+ it('Get_Comments postMessage returns all comments', function() {
+ desktopHelper.insertComment('first comment');
+ cy.cGet('#annotation-content-area-1').should('contain', 'first comment');
+
+ // Avoid notebookbar collapse before the second insertComment.
+ cy.cGet('#Home-tab-label').click();
+ helper.typeIntoDocument('{end}{enter}');
+
+ desktopHelper.insertComment('second comment');
+ cy.cGet('#annotation-content-area-2').should('contain', 'second comment');
+
+ // Stub postMessage to capture the response.
+ cy.getFrameWindow().then(win => {
+ cy.stub(win.parent, 'postMessage').as('postMessage');
+ });
+
+ // Send Get_Comments postMessage.
+ cy.getFrameWindow().then(win => {
+ const message = { 'MessageId': 'Get_Comments' };
+ win.postMessage(JSON.stringify(message), '*');
+ });
+
+ // Verify the response contains both comments.
+ cy.get('@postMessage').should(stub => {
+ const calls = stub.getCalls().filter(call => {
+ try {
+ const msg = typeof call.args[0] === 'string' ? JSON.parse(call.args[0]) : call.args[0];
+ return msg.MessageId === 'Get_Comments_Resp';
+ } catch (e) { return false; }
+ });
+ expect(calls.length, 'Get_Comments_Resp was not posted').to.be.greaterThan(0);
+ const resp = typeof calls[0].args[0] === 'string' ? JSON.parse(calls[0].args[0]) : calls[0].args[0];
+ const comments = resp.Values.Comments;
+ expect(comments.length).to.equal(2);
+ expect(comments[0].Id).to.equal('1');
+ expect(comments[0].Text).to.equal('first comment');
+ expect(comments[0]).to.have.property('Author');
+ expect(comments[0]).to.have.property('DateTime');
+ expect(comments[0].Resolved).to.equal('false');
+ expect(comments[0].Parent).to.equal('0');
+ expect(comments[1].Id).to.equal('2');
+ expect(comments[1].Text).to.equal('second comment');
+ });
+ });

"Jaume Pujantell (via cogerrit)"

unread,
Apr 24, 2026, 7:32:05 AM (5 days ago) Apr 24
to collaboraon...@googlegroups.com
browser/src/canvas/sections/CommentSection.ts | 8 +++--
cypress_test/integration_tests/desktop/writer/annotation_spec.js | 16 ++++++++++
2 files changed, 22 insertions(+), 2 deletions(-)

New commits:
commit f55db62e58f4cc0c624a94c80c4eedc70924c6f9
Author: Jaume Pujantell <jaume.p...@collabora.com>
AuthorDate: Wed Apr 22 17:02:52 2026 +0200
Commit: Miklos Vajna <vmi...@collabora.com>
CommitDate: Fri Apr 24 11:31:43 2026 +0000

browser: focus input on reply/modify

After commit 11dfc37df75f4035e6abb3a6a919818b6f506683 (browser: use
jsdialog based menu for comments) when a user choses to reply or modify
a comment, the focus goes to the document instead of the newly created
input area.

Signed-off-by: Jaume Pujantell <jaume.p...@collabora.com>
Change-Id: I02feb55fbab21b1daab49c7d6fc1515fc8beb3b7
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/1397
Tested-by: Jenkins CPCI <rel...@collaboraoffice.com>
Reviewed-by: Miklos Vajna <vmi...@collabora.com>

diff --git a/browser/src/canvas/sections/CommentSection.ts b/browser/src/canvas/sections/CommentSection.ts
index 8fb7a84354b2..bfc1405b1f44 100644
--- a/browser/src/canvas/sections/CommentSection.ts
+++ b/browser/src/canvas/sections/CommentSection.ts
@@ -1155,16 +1155,20 @@ export class Comment extends CanvasSectionObject {
return;
}

+ const dropdownId = 'comment-menu-' + data.id;
const callback = function (_objectType: string, eventType: string, _object: any, _data: any, entry: any) {
if (eventType !== 'selected')
return false;
+ // reply and modify set focus on the comment textbox
+ // tell the dropdown not to restore focus in those cases
+ const focusHandled = entry?.id === 'reply' || entry?.id === 'modify';
+ JSDialog.CloseDropdown(dropdownId, focusHandled);
this.handleMenuAction(entry?.id);
- JSDialog.CloseAllDropdowns();
return true;
}.bind(this);

JSDialog.OpenDropdown(
- 'comment-menu-' + data.id,
+ dropdownId,
menuEl,
entries,
callback,
diff --git a/cypress_test/integration_tests/desktop/writer/annotation_spec.js b/cypress_test/integration_tests/desktop/writer/annotation_spec.js
index ed8f11ba2685..1b00d3e48980 100644
--- a/cypress_test/integration_tests/desktop/writer/annotation_spec.js
+++ b/cypress_test/integration_tests/desktop/writer/annotation_spec.js
@@ -449,6 +449,22 @@ describe(['tagdesktop'], 'Annotation Tests', function() {
});
});

+ it('Reply focuses the reply textbox', function () {
+ desktopHelper.insertComment();
+ cy.cGet('.cool-annotation-content-wrapper').should('exist');
+ cy.cGet('#comment-annotation-menu-1').click();
+ cy.cGet('body').contains('.ui-combobox-entry.jsdialog.ui-grid-cell', 'Reply').click();
+ cy.cGet('#annotation-reply-textarea-1').should('have.focus');
+ });
+
+ it('Modify focuses the modify textbox', function () {
+ desktopHelper.insertComment();
+ cy.cGet('.cool-annotation-content-wrapper').should('exist');
+ cy.cGet('#comment-annotation-menu-1').click();
+ cy.cGet('body').contains('.ui-combobox-entry.jsdialog.ui-grid-cell', 'Modify').click();
+ cy.cGet('#annotation-modify-textarea-1').should('have.focus');

"Jaume Pujantell (via cogerrit)"

unread,
Apr 24, 2026, 8:07:37 AM (5 days ago) Apr 24
to collaboraon...@googlegroups.com
browser/src/canvas/CanvasSectionContainer.ts | 20 +
browser/src/canvas/sections/ScrollSection.ts | 101 ----------
cypress_test/integration_tests/desktop/writer/scrolling_spec.js | 44 ++++
3 files changed, 70 insertions(+), 95 deletions(-)

New commits:
commit 4542a0ee064ed78b860c0ab3f92a52c191a4914f
Author: Jaume Pujantell <jaume.p...@collabora.com>
AuthorDate: Wed Apr 22 18:13:07 2026 +0000
Commit: Miklos Vajna <vmi...@collabora.com>
CommitDate: Fri Apr 24 12:07:22 2026 +0000

browser: keep scrolling even when cursor leaves the canvas

Dragging the document scrollbar stopped scrolling once the cursor moved
outside the canvas, e.g. over the sidebar. The sync check required
the cursor to stay inside the document anchor.

Remove the check and scroll while the drag is active. Capture the
pointer at mousedown and release it at mouseup so events keep flowing
while the cursor is elsewhere.

Adds a cypress test that drags the vertical scrollbar into the sidebar
area and asserts the scrollbar moved.

Signed-off-by: Jaume Pujantell <jaume.p...@collabora.com>
Change-Id: I3e14c54e10c148ce7ca49eb71cf1fa1d2f4a8cf1
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/1413
Reviewed-by: Miklos Vajna <vmi...@collabora.com>
Tested-by: Jenkins CPCI <rel...@collaboraoffice.com>

diff --git a/browser/src/canvas/CanvasSectionContainer.ts b/browser/src/canvas/CanvasSectionContainer.ts
index 4f997807953f..087ec97b0908 100644
--- a/browser/src/canvas/CanvasSectionContainer.ts
+++ b/browser/src/canvas/CanvasSectionContainer.ts
@@ -181,6 +181,7 @@ class CanvasSectionContainer {
private documentBackgroundColor = '#ffffff'; // This is the background color of the document
private useCSSForBackgroundColor = true;
private touchEventInProgress: boolean = false; // This prevents multiple calling of mouse down and up events.
+ private activePointerId: number | null = null;
public testing: boolean = false; // If this set to true, container will create a div element for every section. So, cypress tests can find where to click etc.
public lowestPropagatedBoundSection: string = null; // Event propagating to bound sections. The first section which stops propagating and the sections those are on top of that section, get the event.
public targetSection: string = null;
@@ -226,6 +227,11 @@ class CanvasSectionContainer {
this.canvas.onwheel = this.onMouseWheel.bind(this);
this.canvas.onmouseleave = this.onMouseLeave.bind(this);
this.canvas.onmouseenter = this.onMouseEnter.bind(this);
+ this.canvas.addEventListener('pointerdown', (e: PointerEvent) => {
+ if (e.button === 0 && e.isPrimary) this.activePointerId = e.pointerId;
+ });
+ this.canvas.addEventListener('pointerup', () => { this.activePointerId = null; });
+ this.canvas.addEventListener('pointercancel', () => { this.activePointerId = null; });
this.canvas.ontouchstart = this.onTouchStart.bind(this);
this.canvas.ontouchmove = this.onTouchMove.bind(this);
this.canvas.ontouchend = this.onTouchEnd.bind(this);
@@ -508,6 +514,20 @@ class CanvasSectionContainer {
return this.draggingSomething;
}

+ // Capture/release can throw if the pointer is gone or was never captured;
+ // we ignore that and carry on.
+ public capturePointerForDrag(): void {
+ if (this.activePointerId !== null) {
+ try { this.canvas.setPointerCapture(this.activePointerId); } catch (_) { /* ignore */ }
+ }
+ }
+
+ public releasePointerForDrag(): void {
+ if (this.activePointerId !== null) {
+ try { this.canvas.releasePointerCapture(this.activePointerId); } catch (_) { /* ignore */ }
+ }
+ }
+
public getDragDistance(): number[] {
return [...this.dragDistance];
}
diff --git a/browser/src/canvas/sections/ScrollSection.ts b/browser/src/canvas/sections/ScrollSection.ts
index e25db2a4a289..eeb5bc036524 100644
--- a/browser/src/canvas/sections/ScrollSection.ts
+++ b/browser/src/canvas/sections/ScrollSection.ts
@@ -97,10 +97,6 @@ export class ScrollSection extends CanvasSectionObject {
this.sectionProperties.scrollAnimationDisableTimeout = null;
this.sectionProperties.scrollWheelDelta = [0, 0]; // Used for non-animated scrolling

- this.sectionProperties.pointerSyncWithVerticalScrollBar = true;
- this.sectionProperties.pointerSyncWithHorizontalScrollBar = true;
- this.sectionProperties.pointerReCaptureSpacer = null; // Clicked point of the scroll bar.
-
// Step by step scrolling interval in ms
this.sectionProperties.stepDuration = 50;
this.sectionProperties.quickScrollHorizontalTimer = null;
@@ -562,78 +558,6 @@ export class ScrollSection extends CanvasSectionObject {
return true;
}

- private isMouseInsideDocumentAnchor (point: cool.SimplePoint): boolean {
- var docSection = this.containerObject.getDocumentAnchorSection();
- return this.containerObject.doesSectionIncludePoint(docSection, point.pToArray());
- }
-
- // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
- private isMousePointerSyncedWithVerticalScrollBar (scrollProps: any, position: cool.SimplePoint): boolean {
- // Keep this desktop-only for now.
- if (!(<any>window).mode.isDesktop())
- return true;
-
- var spacer = 0;
- if (!this.sectionProperties.pointerSyncWithVerticalScrollBar) {
- spacer = this.sectionProperties.pointerReCaptureSpacer;
- }
-
- var pointerIsSyncWithScrollBar = false;
- if (this.sectionProperties.pointerSyncWithVerticalScrollBar) {
- pointerIsSyncWithScrollBar = scrollProps.startY < position.pX && scrollProps.startY + scrollProps.scrollSize - this.sectionProperties.scrollBarThickness > position.pY;
- pointerIsSyncWithScrollBar = pointerIsSyncWithScrollBar || (this.isMouseInsideDocumentAnchor(position) && spacer === 0);
- }
- else {
- // See if the scroll bar is on top or bottom.
- var docAncSectionY = this.containerObject.getDocumentAnchorSection().myTopLeft[1];
- if (scrollProps.startY < 30 * window.app.roundedDpiScale + docAncSectionY) {
- pointerIsSyncWithScrollBar = scrollProps.startY + spacer < position.pY;
- }
- else {
- pointerIsSyncWithScrollBar = scrollProps.startY + spacer > position.pY;
- }
- }
-
- this.sectionProperties.pointerSyncWithVerticalScrollBar = pointerIsSyncWithScrollBar;
- return pointerIsSyncWithScrollBar;
- }
-
- // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
- private isMousePointerSyncedWithHorizontalScrollBar (scrollProps: any, position: cool.SimplePoint): boolean {
- // Keep this desktop-only for now.
- if (!(<any>window).mode.isDesktop())
- return true;
-
- var spacer = 0;
- if (!this.sectionProperties.pointerSyncWithHorizontalScrollBar) {
- spacer = this.sectionProperties.pointerReCaptureSpacer;
- }
-
- const sizeX = scrollProps.scrollSize - this.sectionProperties.scrollBarThickness;
- const docWidth: number = app.sectionContainer.getWidth();
- const startX = this.isRTL() ? docWidth - scrollProps.startX - sizeX : scrollProps.startX;
- const endX = startX + sizeX;
-
- var pointerIsSyncWithScrollBar = false;
- if (this.sectionProperties.pointerSyncWithHorizontalScrollBar) {
- pointerIsSyncWithScrollBar = position.pX > startX && position.pX < endX;
- pointerIsSyncWithScrollBar = pointerIsSyncWithScrollBar || (this.isMouseInsideDocumentAnchor(position) && spacer === 0);
- }
- else {
- // See if the scroll bar is on left or right.
- var docAncSectionX = this.containerObject.getDocumentAnchorSection().myTopLeft[0];
- if (startX < 30 * window.app.roundedDpiScale + docAncSectionX) {
- pointerIsSyncWithScrollBar = startX + spacer < position.pX;
- }
- else {
- pointerIsSyncWithScrollBar = startX + spacer > position.pX;
- }
- }
-
- this.sectionProperties.pointerSyncWithHorizontalScrollBar = pointerIsSyncWithScrollBar;
- return pointerIsSyncWithScrollBar;
- }
-
public onMouseMove (position: cool.SimplePoint, dragDistance: Array<number>, e: MouseEvent): void {
const scrollProps: ScrollProperties = (app.activeDocument as DocumentBase).activeLayout.scrollProperties;

@@ -647,9 +571,7 @@ export class ScrollSection extends CanvasSectionObject {
this.showVerticalScrollBar();

var diffY: number = dragDistance[1] - this.sectionProperties.previousDragDistance[1];
-
- if (this.isMousePointerSyncedWithVerticalScrollBar(scrollProps, position))
- this.scrollVerticalWithOffset(diffY * scrollProps.verticalScrollRatio);
+ this.scrollVerticalWithOffset(diffY * scrollProps.verticalScrollRatio);

this.sectionProperties.previousDragDistance[1] = dragDistance[1];

@@ -663,10 +585,7 @@ export class ScrollSection extends CanvasSectionObject {
this.showHorizontalScrollBar();

var diffX: number = dragDistance[0] - this.sectionProperties.previousDragDistance[0];
- var actualDistance = scrollProps.horizontalScrollRatio * diffX;
-
- if (this.isMousePointerSyncedWithHorizontalScrollBar(scrollProps, position))
- this.scrollHorizontalWithOffset(actualDistance);
+ this.scrollHorizontalWithOffset(scrollProps.horizontalScrollRatio * diffX);

this.sectionProperties.previousDragDistance[0] = dragDistance[0];
this.stopPropagating(); // Don't propagate to other sections.
@@ -750,14 +669,6 @@ export class ScrollSection extends CanvasSectionObject {
this.scrollHorizontalWithOffset(offset);
}

- private getLocalYOnVerticalScrollBar (point: cool.SimplePoint): number {
- return point.pY - app.activeDocument.activeLayout.scrollProperties.startY;
- }
-
- private getLocalXOnHorizontalScrollBar (point: cool.SimplePoint): number {
- return point.pX - app.activeDocument.activeLayout.scrollProperties.startX;
- }
-
private clearQuickScrollTimeout() {
if (this.sectionProperties.quickScrollVerticalTimer) {
clearTimeout(this.sectionProperties.quickScrollVerticalTimer);
@@ -789,8 +700,8 @@ export class ScrollSection extends CanvasSectionObject {
if (point.pY > scrollProps.yOffset) {
this.sectionProperties.clickScrollVertical = true;
this.map.scrollingIsHandled = true;
+ this.containerObject.capturePointerForDrag();
this.quickScrollVertical(point);
- this.sectionProperties.pointerReCaptureSpacer = this.getLocalYOnVerticalScrollBar(point);
e.stopPropagation(); // Don't propagate to map.
this.stopPropagating(); // Don't propagate to bound sections.
}
@@ -814,8 +725,8 @@ export class ScrollSection extends CanvasSectionObject {
|| (mirrorX && point.pX >= scrollProps.xOffset && point.pX >= scrollProps.horizontalScrollRightOffset)) {
this.sectionProperties.clickScrollHorizontal = true;
this.map.scrollingIsHandled = true;
+ this.containerObject.capturePointerForDrag();
this.quickScrollHorizontal(point);
- this.sectionProperties.pointerReCaptureSpacer = this.getLocalXOnHorizontalScrollBar(point);
e.stopPropagation(); // Don't propagate to map.
this.stopPropagating(); // Don't propagate to bound sections.
}
@@ -835,16 +746,16 @@ export class ScrollSection extends CanvasSectionObject {
this.clearQuickScrollTimeout();

if (this.sectionProperties.clickScrollVertical) {
+ this.containerObject.releasePointerForDrag();
e.stopPropagation(); // Don't propagate to map.
this.stopPropagating(); // Don't propagate to bound sections.
this.sectionProperties.clickScrollVertical = false;
- this.sectionProperties.pointerSyncWithVerticalScrollBar = true; // Default.
}
else if (this.sectionProperties.clickScrollHorizontal) {
+ this.containerObject.releasePointerForDrag();
e.stopPropagation(); // Don't propagate to map.
this.stopPropagating(); // Don't propagate to bound sections.
this.sectionProperties.clickScrollHorizontal = false;
- this.sectionProperties.pointerSyncWithHorizontalScrollBar = true; // Default.
}

this.sectionProperties.previousDragDistance = null;
diff --git a/cypress_test/integration_tests/desktop/writer/scrolling_spec.js b/cypress_test/integration_tests/desktop/writer/scrolling_spec.js
index 83c038bf4efd..3c2dd75021ec 100644
--- a/cypress_test/integration_tests/desktop/writer/scrolling_spec.js
+++ b/cypress_test/integration_tests/desktop/writer/scrolling_spec.js
@@ -85,4 +85,48 @@ describe(['tagdesktop', 'tagnextcloud', 'tagproxy'], 'Scroll through document',
cy.cGet('.notebookbar #View-container .unoControlCodes').should('have.class', 'selected');
desktopHelper.assertScrollbarPosition('vertical', 0, 10);
});
+
+ it('Drag vertical scrollbar while mouse moves into sidebar area', function() {
+ desktopHelper.assertScrollbarPosition('vertical', 0, 10);
+
+ cy.getFrameWindow().then(function(win) {
+ helper.processToIdle(win);
+
+ var canvas = win.document.getElementById('document-canvas');
+ var rect = canvas.getBoundingClientRect();
+ var scrollProps = win.app.activeDocument.activeLayout.scrollProperties;
+ var dpi = win.app.dpiScale;
+
+ var barX = Math.floor(rect.right - 10);
+ var barY = Math.floor(
+ rect.top + (scrollProps.startY + scrollProps.verticalScrollSize / 2) / dpi);
+ var sidebarX = Math.floor(rect.right + 80);
+
+ // Hover the canvas so mouseIsInside is true on mousedown.
+ cy.cGet('body').realMouseMove(barX, barY);
+
+ cy.cGet('body').realMouseDown({
+ pointer: 'mouse', button: 'left',
+ x: barX, y: barY,
+ scrollBehavior: false
+ });
+
+ cy.cGet('body').realMouseMove(barX, barY + 30);
+ helper.processToIdle(win);
+
+ // Drag continues while the cursor is over the sidebar.
+ cy.cGet('body').realMouseMove(sidebarX, barY + 100);
+ helper.processToIdle(win);
+ cy.cGet('body').realMouseMove(sidebarX, barY + 200);
+ helper.processToIdle(win);
+ cy.cGet('body').realMouseMove(sidebarX, barY + 300);
+ helper.processToIdle(win);
+
+ cy.cGet('body').realMouseUp({ pointer: 'mouse', button: 'left' });
+ helper.processToIdle(win);
+ });
+
+ desktopHelper.assertScrollbarPosition('vertical', 200, 900);
+ });
+
});

"Pranam Lashkari (via cogerrit)"

unread,
Apr 25, 2026, 5:24:08 PM (4 days ago) Apr 25
to collaboraon...@googlegroups.com
browser/src/slideshow/LayerDrawing.ts | 8 ++++++--
browser/src/slideshow/LayerRenderer.ts | 4 +++-
browser/src/slideshow/LayersCompositor.ts | 2 +-
browser/src/slideshow/RenderContext.ts | 8 ++++++--
cypress_test/integration_tests/snapshots/base/integration_tests/multiuser/impress/follow_me_slideshow_spec.js/effect1.png |binary
cypress_test/integration_tests/snapshots/base/integration_tests/multiuser/impress/follow_me_slideshow_spec.js/slide2_effect2.png |binary
cypress_test/integration_tests/snapshots/base/integration_tests/multiuser/impress/follow_me_slideshow_spec.js/slide2_effect3.png |binary
7 files changed, 16 insertions(+), 6 deletions(-)

New commits:
commit ac63381b2c7fdc2d75d275f502ce778aaf27e401
Author: Pranam Lashkari <lpr...@collabora.com>
AuthorDate: Sat Apr 25 04:05:26 2026 +0530
Commit: Pranam Lashkari <lpr...@collabora.com>
CommitDate: Sat Apr 25 21:23:36 2026 +0000

slideshow: improve slidshow sharpness

Use device pixel ratio when selecting the resolution tier so the
offscreen canvas matches the display's physical pixels on HiDPI
screens. Switch layer texture filtering to NEAREST to preserve
pixel-exact rendering, and use Math.ceil for the width computation
in computeLayerSize so the client-server dimensions agree and no
row duplication artifact appears.

For example, a standard 16:9 Impress page (25400x14288 twips) at
the 3840x2160 tier: Math.trunc(2160 * 25400/14288) = 3839, which
causes a 1-pixel height mismatch with the server (2159 vs 2160),
producing a visible tear. Math.ceil gives 3840, so the server
derives trunc(3840 * 14288/25400) = 2160 — an exact match.

test screenshots updated to sharper shots

Signed-off-by: Pranam Lashkari <lpr...@collabora.com>
Change-Id: I6902f39677b626df8a7da2f54fefb30c1cc7faf0
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/1622
Tested-by: Jenkins CPCI <rel...@collaboraoffice.com>

diff --git a/browser/src/slideshow/LayerDrawing.ts b/browser/src/slideshow/LayerDrawing.ts
index cccf9bb1f237..7b46b2eda65b 100644
--- a/browser/src/slideshow/LayerDrawing.ts
+++ b/browser/src/slideshow/LayerDrawing.ts
@@ -947,8 +947,12 @@ class LayerDrawing {
}

private computeInitialResolution() {
- const viewWidth = window.screen.width;
- const viewHeight = window.screen.height;
+ // window.screen dimensions are in CSS pixels; multiply by DPR so the
+ // tier selection matches the display's physical pixels and the canvas
+ // backing store isn't upscaled by CSS on HiDPI screens.
+ const dpr = window.devicePixelRatio || 1;
+ const viewWidth = window.screen.width * dpr;
+ const viewHeight = window.screen.height * dpr;
this.computeResolution(viewWidth, viewHeight);
}

diff --git a/browser/src/slideshow/LayerRenderer.ts b/browser/src/slideshow/LayerRenderer.ts
index 1fcda7e12a97..e36c7a20714d 100644
--- a/browser/src/slideshow/LayerRenderer.ts
+++ b/browser/src/slideshow/LayerRenderer.ts
@@ -260,10 +260,12 @@ class LayerRendererGl implements LayerRenderer {
// app.console.debug(`LayerDrawing.drawBitmap: cache hit: key: ${textureKey}`);
} else {
if (imageInfo instanceof ImageBitmap) {
- texture = this.glContext.loadTexture(imageInfo);
+ texture = this.glContext.loadTexture(imageInfo, false, true);
} else {
texture = this.glContext.loadTexture(
imageInfo.data as HTMLImageElement,
+ false,
+ true,
);
}
this.textureCache.set(textureKey, texture);
diff --git a/browser/src/slideshow/LayersCompositor.ts b/browser/src/slideshow/LayersCompositor.ts
index 64e870adfba6..660cd3e665d4 100644
--- a/browser/src/slideshow/LayersCompositor.ts
+++ b/browser/src/slideshow/LayersCompositor.ts
@@ -133,7 +133,7 @@ class LayersCompositor extends SlideCompositor {
if (slideRatio > resolutionRatio) {
height = Math.trunc((width * slideHeight) / slideWidth);
} else if (slideRatio < resolutionRatio) {
- width = Math.trunc((height * slideWidth) / slideHeight);
+ width = Math.ceil((height * slideWidth) / slideHeight);
}
return [width, height];
}
diff --git a/browser/src/slideshow/RenderContext.ts b/browser/src/slideshow/RenderContext.ts
index d97426c2ce4e..aab58f4a54ac 100644
--- a/browser/src/slideshow/RenderContext.ts
+++ b/browser/src/slideshow/RenderContext.ts
@@ -67,6 +67,7 @@ abstract class RenderContext {
public abstract loadTexture(
image: HTMLImageElement,
isMipMapEnable?: boolean,
+ nearestFiltering?: boolean,
): WebGLTexture | ImageBitmap;

public abstract deleteTexture(texture: WebGLTexture | ImageBitmap): void;
@@ -109,6 +110,7 @@ class RenderContextGl extends RenderContext {
public loadTexture(
image: HTMLImageElement | ImageBitmap,
isMipMapEnable: boolean = false,
+ nearestFiltering: boolean = false,
): WebGLTexture | ImageBitmap {
if (this.isDisposed()) return null;

@@ -128,6 +130,7 @@ class RenderContextGl extends RenderContext {
gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_WRAP_S, gl.CLAMP_TO_EDGE);
gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_WRAP_T, gl.CLAMP_TO_EDGE);

+ const filter = nearestFiltering ? gl.NEAREST : gl.LINEAR;
if (isMipMapEnable) {
gl.generateMipmap(gl.TEXTURE_2D);
gl.texParameteri(
@@ -136,10 +139,10 @@ class RenderContextGl extends RenderContext {
gl.LINEAR_MIPMAP_LINEAR,
);
} else {
- gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_MIN_FILTER, gl.LINEAR);
+ gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_MIN_FILTER, filter);
}

- gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_MAG_FILTER, gl.LINEAR);
+ gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_MAG_FILTER, filter);

if (image instanceof HTMLImageElement)
app.console.debug(`Texture loaded successfully`);
@@ -277,6 +280,7 @@ class RenderContext2d extends RenderContext {
public loadTexture(
image: HTMLImageElement,
isMipMapEnable: boolean = false,
+ nearestFiltering: boolean = false,
): WebGLTexture | ImageBitmap {
return image;
}
diff --git a/cypress_test/integration_tests/snapshots/base/integration_tests/multiuser/impress/follow_me_slideshow_spec.js/effect1.png b/cypress_test/integration_tests/snapshots/base/integration_tests/multiuser/impress/follow_me_slideshow_spec.js/effect1.png
index 29efa4c622b8..158dc6717458 100644
Binary files a/cypress_test/integration_tests/snapshots/base/integration_tests/multiuser/impress/follow_me_slideshow_spec.js/effect1.png and b/cypress_test/integration_tests/snapshots/base/integration_tests/multiuser/impress/follow_me_slideshow_spec.js/effect1.png differ
diff --git a/cypress_test/integration_tests/snapshots/base/integration_tests/multiuser/impress/follow_me_slideshow_spec.js/slide2_effect2.png b/cypress_test/integration_tests/snapshots/base/integration_tests/multiuser/impress/follow_me_slideshow_spec.js/slide2_effect2.png
index 56fa317eb2e2..28fafafe0ff9 100644
Binary files a/cypress_test/integration_tests/snapshots/base/integration_tests/multiuser/impress/follow_me_slideshow_spec.js/slide2_effect2.png and b/cypress_test/integration_tests/snapshots/base/integration_tests/multiuser/impress/follow_me_slideshow_spec.js/slide2_effect2.png differ
diff --git a/cypress_test/integration_tests/snapshots/base/integration_tests/multiuser/impress/follow_me_slideshow_spec.js/slide2_effect3.png b/cypress_test/integration_tests/snapshots/base/integration_tests/multiuser/impress/follow_me_slideshow_spec.js/slide2_effect3.png
index 0d70b8ac921e..3293eec3131f 100644
Binary files a/cypress_test/integration_tests/snapshots/base/integration_tests/multiuser/impress/follow_me_slideshow_spec.js/slide2_effect3.png and b/cypress_test/integration_tests/snapshots/base/integration_tests/multiuser/impress/follow_me_slideshow_spec.js/slide2_effect3.png differ

"Mike Kaganski (via cogerrit)"

unread,
5:18 AM (1 hour ago) 5:18 AM
to collaboraon...@googlegroups.com
browser/src/canvas/sections/CommentListSection.ts | 125 +++++
browser/src/layer/tile/ImpressTileLayer.js | 13
cypress_test/integration_tests/desktop/draw/annotation_spec.js | 237 ++++++++++
3 files changed, 370 insertions(+), 5 deletions(-)

New commits:
commit 8b0a148f8596313e3a8ae88a435784f20449abb1
Author: Mike Kaganski <mike.k...@collabora.com>
AuthorDate: Tue Apr 28 18:30:13 2026 +0500
Commit: Miklos Vajna <vmi...@collabora.com>
CommitDate: Wed Apr 29 09:18:13 2026 +0000

PDF comments: click-to-place anchor for newly inserted comments

Previously, adding comments to PDF placed them at the page's top left.

Instead, defer creating a new comment in PDF until the user picks a spot
on the page. Insert Comment now enters a placement mode; the captured
click is mapped to document twips and used both as the on-screen anchor
for the editor and as the X/Y args of .uno:InsertAnnotation. Esc cancels.

Pairs with the engine commit I07cd212eef0f5f79d05dd7be5f7e8c1bcaa5f857
"sd: implement placing comments in specified position" which added
PositionX/PositionY to the slot.

Signed-off-by: Mike Kaganski <mike.k...@collabora.com>
Change-Id: I9d7bbb88bc2b1f8401c9a732a9c18c002724f117
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/1777
Tested-by: Jenkins CPCI <rel...@collaboraoffice.com>
Reviewed-by: Miklos Vajna <vmi...@collabora.com>

diff --git a/browser/src/canvas/sections/CommentListSection.ts b/browser/src/canvas/sections/CommentListSection.ts
index 8fc6b423e073..3d30066488db 100644
--- a/browser/src/canvas/sections/CommentListSection.ts
+++ b/browser/src/canvas/sections/CommentListSection.ts
@@ -47,14 +47,20 @@ window.L.Map.include({
if (author in this._viewInfoByUserName) {
avatar = this._viewInfoByUserName[author].userextrainfo.avatar;
}
- this._docLayer.newAnnotation({
+ const commentData = {
text: '',
textrange: '',
author: author,
dateTime: new Date().toISOString(),
id: 'new', // 'new' only when added by us
avatar: avatar
- });
+ };
+ // In PDF, enter a click-to-place mode and let the user pick the spot.
+ if (app.file.fileBasedView && commentSection) {
+ commentSection.startCommentPlacement(commentData);
+ return;
+ }
+ this._docLayer.newAnnotation(commentData);
},

insertThreadedComment: function() {
@@ -161,6 +167,12 @@ export class CommentSection extends CanvasSectionObject {
private annotationMaxSize: number;
escapeListener: (e: KeyboardEvent) => void;

+ // Active "click anywhere on the page to place a new comment" mode for PDF.
+ // When non-null, the next document mousedown is consumed to position a new
+ // comment at that point instead of being forwarded to core.
+ private placementCommentData: any = null;
+ private placementSavedCursor: string | null = null;
+
constructor () {
super(app.CSections.CommentList.name);

@@ -195,6 +207,12 @@ export class CommentSection extends CanvasSectionObject {
this.mobileCommentModalId = this.map.uiManager.generateModalId(this.mobileCommentId);
this.annotationMinSize = Number(getComputedStyle(document.documentElement).getPropertyValue('--annotation-min-size'));
this.annotationMaxSize = Number(getComputedStyle(document.documentElement).getPropertyValue('--annotation-max-size'));
+
+ // PDF placement-mode handlers are added to / removed from the DOM via
+ // addEventListener/removeEventListener; bind once here so the same
+ // reference is used for both calls.
+ this.onPlacementMouseDown = this.onPlacementMouseDown.bind(this);
+ this.onPlacementKeyDown = this.onPlacementKeyDown.bind(this);
}

public onInitialize (): void {
@@ -787,6 +805,91 @@ export class CommentSection extends CanvasSectionObject {
app.map.fire('deleteannotation');
}

+ public startCommentPlacement(commentData: any): void {
+ // An additional Insert Comment trigger while placement is already
+ // pending is a no-op.
+ if (this.placementCommentData)
+ return;
+
+ const canvas = document.getElementById('document-canvas') as HTMLCanvasElement | null;
+ if (!canvas)
+ return;
+
+ this.placementCommentData = commentData;
+ this.placementSavedCursor = canvas.style.cursor;
+ canvas.style.cursor = 'crosshair';
+
+ // Capture phase, before CanvasSectionContainer's onmousedown property
+ // handler dispatches to MouseControl/etc.
+ canvas.addEventListener('mousedown', this.onPlacementMouseDown, true);
+ document.addEventListener('keydown', this.onPlacementKeyDown, true);
+ }
+
+ private exitCommentPlacement(): void {
+ const canvas = document.getElementById('document-canvas') as HTMLCanvasElement | null;
+ if (canvas) {
+ canvas.removeEventListener('mousedown', this.onPlacementMouseDown, true);
+ if (this.placementSavedCursor !== null)
+ canvas.style.cursor = this.placementSavedCursor;
+ }
+ document.removeEventListener('keydown', this.onPlacementKeyDown, true);
+ this.placementCommentData = null;
+ this.placementSavedCursor = null;
+ }
+
+ private onPlacementMouseDown(e: MouseEvent): void {
+ if (e.button !== 0) return; // ignore right/middle clicks
+ e.stopImmediatePropagation();
+ e.preventDefault();
+ this.finishCommentPlacement(e, e.currentTarget as HTMLCanvasElement);
+ }
+
+ private onPlacementKeyDown(e: KeyboardEvent): void {
+ if (e.key !== 'Escape') return;
+ e.stopImmediatePropagation();
+ e.preventDefault();
+ this.exitCommentPlacement();
+ }
+
+ private finishCommentPlacement(e: MouseEvent, canvas: HTMLCanvasElement): void {
+ const commentData = this.placementCommentData;
+ // canvasToDocumentPoint expects coordinates relative to the canvas DOM
+ // element.
+ const rect = canvas.getBoundingClientRect();
+ const point = new cool.SimplePoint(0, 0);
+ point.cX = e.clientX - rect.left;
+ point.cY = e.clientY - rect.top;
+
+ const layout = app.activeDocument.activeLayout;
+ const docPoint = layout.canvasToDocumentPoint(point);
+ if (Number.isNaN(docPoint.x) || Number.isNaN(docPoint.y))
+ return;
+
+ // docPoint.x/.y are stacked-document twips. fileBasedView lays pages
+ // out vertically with a fixed _spaceBetweenParts gap. Compute the
+ // containing page and reject clicks that fell in a horizontal margin,
+ // in an inter-page gap, or past the last page - the user should stay
+ // in placement mode and try again on a real page.
+ const docLayer = app.map._docLayer;
+ const additionPerPart = docLayer._partHeightTwips + docLayer._spaceBetweenParts;
+ const partIndex = Math.floor(docPoint.y / additionPerPart);
+ const yInPart = docPoint.y - partIndex * additionPerPart;
+ if (partIndex < 0 || partIndex >= docLayer._parts
+ || docPoint.x < 0 || docPoint.x > docLayer._partWidthTwips
+ || yInPart > docLayer._partHeightTwips)
+ return;
+
+ // Switching the active part keeps save()'s setPart wrapper consistent
+ // and lets newAnnotation pick up the correct parthash; yAddition lets
+ // save() back the per-page offset out before sending PositionY.
+ commentData.yAddition = partIndex * additionPerPart;
+ this.map.setPart(partIndex, false);
+
+ commentData.position = [docPoint.x, docPoint.y];
+ this.exitCommentPlacement();
+ this.sectionProperties.docLayer.newAnnotation(commentData);
+ }
+
public click (annotation: any): void {
this.select(annotation);
app.map.fire('postMessage', {
@@ -798,6 +901,21 @@ export class CommentSection extends CanvasSectionObject {
public save (annotation: any): void {
var comment;
if (annotation.sectionProperties.data.id === 'new') {
+ // PDF click-to-place: when the user picked a position before
+ // opening the editor, ship it as twips so core can place the
+ // annotation there instead of (0, 0). Mirrors the EditAnnotation
+ // pattern in CommentMarkerSubSection.sendAnnotationPositionChange.
+ let positionArgs: any = {};
+ if (annotation.sectionProperties.data.position) {
+ const pos = annotation.sectionProperties.data.position;
+ let py = pos[1];
+ if (app.file.fileBasedView)
+ py -= annotation.sectionProperties.data.yAddition || 0;
+ positionArgs = {
+ PositionX: { type: 'int32', value: pos[0] },
+ PositionY: { type: 'int32', value: py },
+ };
+ }
comment = {
Author: {
type: 'string',
@@ -813,7 +931,8 @@ export class CommentSection extends CanvasSectionObject {
{ Text: {
type: 'string',
value: annotation.sectionProperties.data.text
- } }
+ } },
+ ...positionArgs
};
var unoCommand = annotation.sectionProperties.data.threaded
? '.uno:InsertThreadedComment' : '.uno:InsertAnnotation';
diff --git a/browser/src/layer/tile/ImpressTileLayer.js b/browser/src/layer/tile/ImpressTileLayer.js
index b42f133f9987..11e2b6449738 100644
--- a/browser/src/layer/tile/ImpressTileLayer.js
+++ b/browser/src/layer/tile/ImpressTileLayer.js
@@ -125,8 +125,17 @@ window.L.ImpressTileLayer = window.L.CanvasTileLayer.extend({
},

newAnnotation: function (commentData) {
- commentData.anchorPos = [app.activeDocument.activeLayout.viewedRectangle.x1, app.activeDocument.activeLayout.viewedRectangle.y1];
- commentData.rectangle = [app.activeDocument.activeLayout.viewedRectangle.x1, app.activeDocument.activeLayout.viewedRectangle.y1, 566, 566];
+ // commentData.position (twips, top-left) lets the caller pin the new
+ // comment to a specific document point; used by the PDF click-to-place
+ // flow. Without it the marker lands at the current viewport top-left.
+ const anchorX = commentData.position
+ ? commentData.position[0]
+ : app.activeDocument.activeLayout.viewedRectangle.x1;
+ const anchorY = commentData.position
+ ? commentData.position[1]
+ : app.activeDocument.activeLayout.viewedRectangle.y1;
+ commentData.anchorPos = [anchorX, anchorY];
+ commentData.rectangle = [anchorX, anchorY, 566, 566];

commentData.parthash = app.impress.partList[this._selectedPart].hash;

diff --git a/cypress_test/integration_tests/desktop/draw/annotation_spec.js b/cypress_test/integration_tests/desktop/draw/annotation_spec.js
index 441be83f20ff..7d9cc54b2a45 100644
--- a/cypress_test/integration_tests/desktop/draw/annotation_spec.js
+++ b/cypress_test/integration_tests/desktop/draw/annotation_spec.js
@@ -189,4 +189,241 @@ describe(['tagdesktop'], 'PDF Threaded Comments', function() {
assertCommentShownAndAnchorVisible(win, 'Zoe');
});
});
+
+ // Insert Comment on a PDF defers comment creation until the user clicks
+ // on the page: the toolbar/menu action enters a placement mode (crosshair
+ // cursor, capture-phase mousedown listener) and the captured click is
+ // converted to document twips for both the on-screen anchor and the
+ // .uno:InsertAnnotation PositionX/Y args.
+ it('Insert Comment in PDF enters placement mode, pins the anchor to the click, and round-trips through core', { env: { 'pdf-view': true } }, function() {
+ const commentText = 'placement-mode-test ' + Date.now();
+ let initialCount = 0;
+
+ cy.getFrameWindow().then(function(win) {
+ const section = win.app.sectionContainer.getSectionWithName(
+ win.app.CSections.CommentList.name);
+ initialCount = section.sectionProperties.commentList.length;
+ win.app.map.insertComment();
+ });
+
+ cy.getFrameWindow().should(function(win) {
+ expect(win.document.getElementById('document-canvas').style.cursor,
+ 'placement mode must switch the canvas cursor to crosshair')
+ .to.equal('crosshair');
+
+ const section = win.app.sectionContainer.getSectionWithName(
+ win.app.CSections.CommentList.name);
+ expect(section.sectionProperties.commentList.length,
+ 'no comment should be created until the user picks a position')
+ .to.equal(initialCount);
+ });
+
+ let expectedX = 0, expectedY = 0;
+ cy.getFrameWindow().then(function(win) {
+ const canvas = win.document.getElementById('document-canvas');
+ const rect = canvas.getBoundingClientRect();
+
+ // Aim for ~25% across page 1 in document twips, then map back to
+ // canvas-relative CSS pixels.
+ const docLayer = win.app.map._docLayer;
+ const targetTwipsX = docLayer._partWidthTwips / 4;
+ const targetTwipsY = docLayer._partHeightTwips / 4;
+ const canvasClickX = (targetTwipsX * win.app.twipsToPixels) / win.app.dpiScale;
+ const canvasClickY = (targetTwipsY * win.app.twipsToPixels) / win.app.dpiScale;
+
+ // canvasToDocumentPoint mutates its input, so build a fresh
+ // SimplePoint here that matches finishCommentPlacement's local point.
+ const expectedPoint = new win.cool.SimplePoint(0, 0);
+ expectedPoint.cX = canvasClickX;
+ expectedPoint.cY = canvasClickY;
+ const docPoint = win.app.activeDocument.activeLayout.canvasToDocumentPoint(expectedPoint);
+ expectedX = docPoint.x;
+ expectedY = docPoint.y;
+
+ // Capture-phase mousedown listener inside startCommentPlacement
+ // consumes this; bubbles:true is needed so the capture path runs.
+ const ev = new win.MouseEvent('mousedown', {
+ clientX: rect.left + canvasClickX,
+ clientY: rect.top + canvasClickY,
+ button: 0,
+ bubbles: true,
+ });
+ canvas.dispatchEvent(ev);
+ });
+
+ // Local 'new' comment exists with the expected on-screen anchor.
+ cy.getFrameWindow().should(function(win) {
+ expect(win.document.getElementById('document-canvas').style.cursor,
+ 'cursor must be restored after placement finishes')
+ .to.not.equal('crosshair');
+
+ const section = win.app.sectionContainer.getSectionWithName(
+ win.app.CSections.CommentList.name);
+ const newComment = section.sectionProperties.commentList.find(
+ function(c) { return c.sectionProperties.data.id === 'new'; });
+ expect(newComment, 'new comment was not created after the click').to.exist;
+
+ const anchorPos = newComment.sectionProperties.data.anchorPos;
+ expect(anchorPos[0],
+ 'on-screen anchor X must match canvasToDocumentPoint of the click').to.be.closeTo(expectedX, 1);
+ expect(anchorPos[1],
+ 'on-screen anchor Y must match canvasToDocumentPoint of the click').to.be.closeTo(expectedY, 1);
+ });
+
+ // Wait for the editor to render with a textarea, then settle so its
+ // id has flipped from 'new' to the final value.
+ cy.cGet('.cool-annotation').last({log: false})
+ .find('#annotation-modify-textarea-new').should('exist');
+ cy.getFrameWindow().then(function(win) { return helper.processToIdle(win); });
+ cy.cGet('.cool-annotation').last({log: false})
+ .find('.modify-annotation .cool-annotation-textarea')
+ .should('not.have.attr', 'disabled');
+ cy.cGet('.cool-annotation').last({log: false})
+ .find('.modify-annotation .cool-annotation-textarea')
+ .type(commentText);
+ cy.cGet('.cool-annotation').last({log: false})
+ .find('[value="Save"]').click();
+
+ // After core's acknowledgement: the commentList grew by one, the new
+ // comment carries a non-'new' id assigned by core, and its anchor
+ // survived the round-trip - the position core stored matches the
+ // position the placement-mode click captured.
+ cy.getFrameWindow().should(function(win) {
+ const section = win.app.sectionContainer.getSectionWithName(
+ win.app.CSections.CommentList.name);
+ expect(section.sectionProperties.commentList.length,
+ 'commentList must have one more entry after save')
+ .to.equal(initialCount + 1);
+
+ const acked = section.sectionProperties.commentList.find(function(c) {
+ return c.sectionProperties.data.text === commentText;
+ });
+ expect(acked, 'a comment with the typed text must be present').to.exist;
+ expect(acked.sectionProperties.data.id,
+ 'core must assign a real (non-\'new\') id').to.not.equal('new');
+
+ // Round-trip goes twips -> mm/100 (core) -> twips, so allow a few
+ // twips of rounding slack rather than asserting exact equality.
+ const anchorPos = acked.sectionProperties.data.anchorPos;
+ expect(anchorPos[0],
+ 'round-tripped anchor X must match the captured click').to.be.closeTo(expectedX, 5);
+ expect(anchorPos[1],
+ 'round-tripped anchor Y must match the captured click').to.be.closeTo(expectedY, 5);
+ });
+ });
+
+ // Negative path through placement mode: enter placement, miss the page
+ // (no comment), then click on the page (anchor visible at the click
+ // point), then cancel the editor before saving (no comment ends up in
+ // the document at all).
+ it('Insert Comment placement mode: off-page miss, on-page anchor, cancel', { env: { 'pdf-view': true } }, function() {
+ let initialCount = 0;
+
+ cy.getFrameWindow().then(function(win) {
+ const section = win.app.sectionContainer.getSectionWithName(
+ win.app.CSections.CommentList.name);
+ initialCount = section.sectionProperties.commentList.length;
+ win.app.map.insertComment();
+ });
+
+ cy.getFrameWindow().should(function(win) {
+ expect(win.document.getElementById('document-canvas').style.cursor)
+ .to.equal('crosshair');
+ });
+
+ // Off-page click. Compute an X past the page's right edge from
+ // _partWidthTwips so it's guaranteed off-page regardless of zoom or
+ // scroll.
+ cy.getFrameWindow().then(function(win) {
+ const canvas = win.document.getElementById('document-canvas');
+ const rect = canvas.getBoundingClientRect();
+ const docLayer = win.app.map._docLayer;
+ const offPageCssX = (docLayer._partWidthTwips
+ * win.app.twipsToPixels) / win.app.dpiScale + 50;
+ const ev = new win.MouseEvent('mousedown', {
+ clientX: rect.left + offPageCssX,
+ clientY: rect.top + 200,
+ button: 0,
+ bubbles: true,
+ });
+ canvas.dispatchEvent(ev);
+ });
+
+ cy.getFrameWindow().then(function(win) { return helper.processToIdle(win); });
+ cy.getFrameWindow().should(function(win) {
+ expect(win.document.getElementById('document-canvas').style.cursor,
+ 'placement mode should remain active after an off-page click')
+ .to.equal('crosshair');
+
+ const section = win.app.sectionContainer.getSectionWithName(
+ win.app.CSections.CommentList.name);
+ expect(section.sectionProperties.commentList.length,
+ 'no comment should be added on an off-page click')
+ .to.equal(initialCount);
+ });
+
+ // On-page click - placement mode exits, a 'new' comment gets the
+ // expected anchor and the editor opens.
+ let expectedX = 0, expectedY = 0;
+ cy.getFrameWindow().then(function(win) {
+ const canvas = win.document.getElementById('document-canvas');
+ const rect = canvas.getBoundingClientRect();
+
+ // Same target-twips-back-to-CSS-pixels strategy as the happy-path
+ // test - aims for ~25% across page 1 regardless of zoom.
+ const docLayer = win.app.map._docLayer;
+ const targetTwipsX = docLayer._partWidthTwips / 4;
+ const targetTwipsY = docLayer._partHeightTwips / 4;
+ const canvasClickX = (targetTwipsX * win.app.twipsToPixels) / win.app.dpiScale;
+ const canvasClickY = (targetTwipsY * win.app.twipsToPixels) / win.app.dpiScale;
+
+ const expectedPoint = new win.cool.SimplePoint(0, 0);
+ expectedPoint.cX = canvasClickX;
+ expectedPoint.cY = canvasClickY;
+ const docPoint = win.app.activeDocument.activeLayout.canvasToDocumentPoint(expectedPoint);
+ expectedX = docPoint.x;
+ expectedY = docPoint.y;
+
+ const ev = new win.MouseEvent('mousedown', {
+ clientX: rect.left + canvasClickX,
+ clientY: rect.top + canvasClickY,
+ button: 0,
+ bubbles: true,
+ });
+ canvas.dispatchEvent(ev);
+ });
+
+ cy.getFrameWindow().should(function(win) {
+ expect(win.document.getElementById('document-canvas').style.cursor,
+ 'cursor must be restored after the on-page click')
+ .to.not.equal('crosshair');
+
+ const section = win.app.sectionContainer.getSectionWithName(
+ win.app.CSections.CommentList.name);
+ const newComment = section.sectionProperties.commentList.find(
+ function(c) { return c.sectionProperties.data.id === 'new'; });
+ expect(newComment, 'new comment must be created at the click').to.exist;
+ const anchorPos = newComment.sectionProperties.data.anchorPos;
+ expect(anchorPos[0],
+ 'temporary anchor X must match the click').to.be.closeTo(expectedX, 1);
+ expect(anchorPos[1],
+ 'temporary anchor Y must match the click').to.be.closeTo(expectedY, 1);
+ });
+
+ // Cancel the editor before typing/saving.
+ cy.cGet('.cool-annotation').last({log: false})
+ .find('#annotation-modify-textarea-new').should('exist');
+ cy.cGet('.cool-annotation').last({log: false})
+ .find('#annotation-cancel-new').click();
+
+ // Settle and verify the document carries no extra comment.
+ cy.getFrameWindow().then(function(win) { return helper.processToIdle(win); });
+ cy.getFrameWindow().should(function(win) {
+ const section = win.app.sectionContainer.getSectionWithName(
+ win.app.CSections.CommentList.name);
+ expect(section.sectionProperties.commentList.length,
+ 'no comment should remain after cancelling the editor')
+ .to.equal(initialCount);
Reply all
Reply to author
Forward
0 new messages