online.git: browser/src cypress_test/integration_tests
0 views
Skip to first unread message
"Jaume Pujantell (via cogerrit)"
May 11, 2026, 2:47:25 AMMay 11
to collaboraon...@googlegroups.com
browser/src/layer/tile/WriterTileLayer.js | 32 ++++++++--
cypress_test/integration_tests/desktop/writer3/reconnect_spec.js | 22 +++++-
2 files changed, 46 insertions(+), 8 deletions(-)
New commits:
commit 3b137c5bca4377def4f07196b7aa6dd95c02f45b
Author: Jaume Pujantell <jaume.p...@collabora.com>
AuthorDate: Fri May 1 11:19:49 2026 +0200
Commit: Miklos Vajna <vmi...@collabora.com>
CommitDate: Mon May 11 06:47:13 2026 +0000
maintain view position on reconnection
This is a follow-up to commit 94520f969c397f2bf83472759732eb84ac5a3d66
(writer: jump to previous position on reconnection). Moving the
simulated click until we have enough view loaded is not enough to avoid
view jumps. This approach also waits before updating the file size in
any way, with and added fail-safe timer to avoid getting stuck if the
document actually shrinks.
Signed-off-by: Jaume Pujantell <jaume.p...@collabora.com>
Change-Id: I58d4aba5c061f6e08ba741ceae11e2543b4328e0
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/2172
Tested-by: Jenkins CPCI <rel...@collaboraoffice.com>
Reviewed-by: Miklos Vajna <vmi...@collabora.com>
diff --git a/browser/src/layer/tile/WriterTileLayer.js b/browser/src/layer/tile/WriterTileLayer.js
index 034af0d2340c..523b228853a7 100644
--- a/browser/src/layer/tile/WriterTileLayer.js
+++ b/browser/src/layer/tile/WriterTileLayer.js
@@ -107,6 +107,18 @@ window.L.WriterTileLayer = window.L.CanvasTileLayer.extend({
}
},
+ _setNewSize: function (/*cool.SimplePoint*/ size) {
+ app.activeDocument.fileSize = size;
+ app.activeDocument.activeLayout.viewSize = app.activeDocument.fileSize.clone();
+ this._updateMaxBounds(true);
+ },
+
+ _releaseReconnectFileSize: function () {
+ this._setNewSize(this._reconnectFileSize);
+ this._reconnectFileSize = null;
+ this._reconnectFileSizeTimer = null;
+ },
+
_onStatusMsg: function (textMsg) {
const statusJSON = JSON.parse(textMsg.replace('status:', '').replace('statusupdate:', ''));
@@ -128,6 +140,19 @@ window.L.WriterTileLayer = window.L.CanvasTileLayer.extend({
this._map.setPermission('readonly');
var sizeChanged = statusJSON.width !== app.activeDocument.fileSize.x || statusJSON.height !== app.activeDocument.fileSize.y;
+ if (sizeChanged && (app.socket._reconnecting || this._reconnectFileSize)) {
+ if (this._reconnectFileSizeTimer)
+ clearTimeout(this._reconnectFileSizeTimer);
+ if (statusJSON.width >= app.activeDocument.fileSize.x && statusJSON.height >= app.activeDocument.fileSize.y) {
+ this._reconnectFileSize = null;
+ } else {
+ // Suppress shrinking sizes during reconnection incremental reload
+ // The timer avoids this supression being permanent
+ sizeChanged = false;
+ this._reconnectFileSize = new cool.SimplePoint(statusJSON.width, statusJSON.height);
+ this._reconnectFileSizeTimer = setTimeout(this._releaseReconnectFileSize.bind(this), 5000);
+ }
+ }
if (statusJSON.viewid !== undefined) {
this._viewId = statusJSON.viewid;
@@ -147,11 +172,8 @@ window.L.WriterTileLayer = window.L.CanvasTileLayer.extend({
console.assert(this._viewId >= 0, 'Incorrect viewId received: ' + this._viewId);
if (sizeChanged) {
- app.activeDocument.fileSize = new cool.SimplePoint(statusJSON.width, statusJSON.height);
- app.activeDocument.activeLayout.viewSize = app.activeDocument.fileSize.clone();
-
this._docType = statusJSON.type;
- this._updateMaxBounds(true);
+ this._setNewSize(new cool.SimplePoint(statusJSON.width, statusJSON.height));
}
this._documentInfo = textMsg;
@@ -181,7 +203,7 @@ window.L.WriterTileLayer = window.L.CanvasTileLayer.extend({
});
TileManager.resetPreFetching(true);
- if (this._savedCursorPos && this._savedCursorPos.center[0] <= app.activeDocument.fileSize.x && this._savedCursorPos.center[1] <= app.activeDocument.fileSize.y) {
+ if (this._savedCursorPos && this._savedCursorPos.center[0] <= statusJSON.width && this._savedCursorPos.center[1] <= statusJSON.height) {
this._postMouseEvent('buttondown', this._savedCursorPos.center[0], this._savedCursorPos.center[1], 1, 1, 0);
this._postMouseEvent('buttonup', this._savedCursorPos.center[0], this._savedCursorPos.center[1], 1, 1, 0);
this._savedCursorPos = null;
diff --git a/cypress_test/integration_tests/desktop/writer3/reconnect_spec.js b/cypress_test/integration_tests/desktop/writer3/reconnect_spec.js
index 2d863650ebc5..e23e14260f78 100644
--- a/cypress_test/integration_tests/desktop/writer3/reconnect_spec.js
+++ b/cypress_test/integration_tests/desktop/writer3/reconnect_spec.js
@@ -1,4 +1,4 @@
-/* global describe it cy beforeEach require Cypress */
+/* global describe it cy beforeEach require expect */
var helper = require('../../common/helper');
var desktopHelper = require('../../common/desktop_helper');
@@ -16,6 +16,12 @@ describe(['tagdesktop'], 'WebSocket reconnection', function () {
helper.typeIntoDocument('{ctrl}{End}');
desktopHelper.assertVisiblePage(12, 12, 12);
+ var preDisconnectY1;
+ cy.getFrameWindow().then(function (win) {
+ preDisconnectY1 = win.app.activeDocument.activeLayout.viewedRectangle.y1;
+ expect(preDisconnectY1).to.be.greaterThan(0);
+ });
+
// Close the raw WebSocket to trigger automatic reconnection
cy.getFrameWindow().then(function (win) {
win.app.socket.socket.close();
@@ -24,7 +30,6 @@ describe(['tagdesktop'], 'WebSocket reconnection', function () {
// Can't use processToIdle with the socket closed
cy.wait(500);
- // Wait for reconnection to complete
cy.cGet('#document-canvas').should('be.visible');
cy.getFrameWindow().its('app.socket._reconnecting')
.should('eq', false);
@@ -32,7 +37,18 @@ describe(['tagdesktop'], 'WebSocket reconnection', function () {
helper.processToIdle(win);
});
- // Verify the page position is preserved after reconnection
desktopHelper.assertVisiblePage(12, 12, 12);
+
+ cy.cGet('#document-canvas').click(200, 200);
+ cy.getFrameWindow().then(function (win) {
+ helper.processToIdle(win);
+ });
+
+ desktopHelper.assertVisiblePage(12, 12, 12);
+ cy.getFrameWindow().then(function (win) {
+ var postClickY1 = win.app.activeDocument.activeLayout.viewedRectangle.y1;
+ var drift = Math.abs(postClickY1 - preDisconnectY1);
+ expect(drift).to.be.lessThan(preDisconnectY1 / 2);
+ });
});
});
cypress_test/integration_tests/desktop/writer3/reconnect_spec.js | 22 +++++-
2 files changed, 46 insertions(+), 8 deletions(-)
New commits:
commit 3b137c5bca4377def4f07196b7aa6dd95c02f45b
Author: Jaume Pujantell <jaume.p...@collabora.com>
AuthorDate: Fri May 1 11:19:49 2026 +0200
Commit: Miklos Vajna <vmi...@collabora.com>
CommitDate: Mon May 11 06:47:13 2026 +0000
maintain view position on reconnection
This is a follow-up to commit 94520f969c397f2bf83472759732eb84ac5a3d66
(writer: jump to previous position on reconnection). Moving the
simulated click until we have enough view loaded is not enough to avoid
view jumps. This approach also waits before updating the file size in
any way, with and added fail-safe timer to avoid getting stuck if the
document actually shrinks.
Signed-off-by: Jaume Pujantell <jaume.p...@collabora.com>
Change-Id: I58d4aba5c061f6e08ba741ceae11e2543b4328e0
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/2172
Tested-by: Jenkins CPCI <rel...@collaboraoffice.com>
Reviewed-by: Miklos Vajna <vmi...@collabora.com>
diff --git a/browser/src/layer/tile/WriterTileLayer.js b/browser/src/layer/tile/WriterTileLayer.js
index 034af0d2340c..523b228853a7 100644
--- a/browser/src/layer/tile/WriterTileLayer.js
+++ b/browser/src/layer/tile/WriterTileLayer.js
@@ -107,6 +107,18 @@ window.L.WriterTileLayer = window.L.CanvasTileLayer.extend({
}
},
+ _setNewSize: function (/*cool.SimplePoint*/ size) {
+ app.activeDocument.fileSize = size;
+ app.activeDocument.activeLayout.viewSize = app.activeDocument.fileSize.clone();
+ this._updateMaxBounds(true);
+ },
+
+ _releaseReconnectFileSize: function () {
+ this._setNewSize(this._reconnectFileSize);
+ this._reconnectFileSize = null;
+ this._reconnectFileSizeTimer = null;
+ },
+
_onStatusMsg: function (textMsg) {
const statusJSON = JSON.parse(textMsg.replace('status:', '').replace('statusupdate:', ''));
@@ -128,6 +140,19 @@ window.L.WriterTileLayer = window.L.CanvasTileLayer.extend({
this._map.setPermission('readonly');
var sizeChanged = statusJSON.width !== app.activeDocument.fileSize.x || statusJSON.height !== app.activeDocument.fileSize.y;
+ if (sizeChanged && (app.socket._reconnecting || this._reconnectFileSize)) {
+ if (this._reconnectFileSizeTimer)
+ clearTimeout(this._reconnectFileSizeTimer);
+ if (statusJSON.width >= app.activeDocument.fileSize.x && statusJSON.height >= app.activeDocument.fileSize.y) {
+ this._reconnectFileSize = null;
+ } else {
+ // Suppress shrinking sizes during reconnection incremental reload
+ // The timer avoids this supression being permanent
+ sizeChanged = false;
+ this._reconnectFileSize = new cool.SimplePoint(statusJSON.width, statusJSON.height);
+ this._reconnectFileSizeTimer = setTimeout(this._releaseReconnectFileSize.bind(this), 5000);
+ }
+ }
if (statusJSON.viewid !== undefined) {
this._viewId = statusJSON.viewid;
@@ -147,11 +172,8 @@ window.L.WriterTileLayer = window.L.CanvasTileLayer.extend({
console.assert(this._viewId >= 0, 'Incorrect viewId received: ' + this._viewId);
if (sizeChanged) {
- app.activeDocument.fileSize = new cool.SimplePoint(statusJSON.width, statusJSON.height);
- app.activeDocument.activeLayout.viewSize = app.activeDocument.fileSize.clone();
-
this._docType = statusJSON.type;
- this._updateMaxBounds(true);
+ this._setNewSize(new cool.SimplePoint(statusJSON.width, statusJSON.height));
}
this._documentInfo = textMsg;
@@ -181,7 +203,7 @@ window.L.WriterTileLayer = window.L.CanvasTileLayer.extend({
});
TileManager.resetPreFetching(true);
- if (this._savedCursorPos && this._savedCursorPos.center[0] <= app.activeDocument.fileSize.x && this._savedCursorPos.center[1] <= app.activeDocument.fileSize.y) {
+ if (this._savedCursorPos && this._savedCursorPos.center[0] <= statusJSON.width && this._savedCursorPos.center[1] <= statusJSON.height) {
this._postMouseEvent('buttondown', this._savedCursorPos.center[0], this._savedCursorPos.center[1], 1, 1, 0);
this._postMouseEvent('buttonup', this._savedCursorPos.center[0], this._savedCursorPos.center[1], 1, 1, 0);
this._savedCursorPos = null;
diff --git a/cypress_test/integration_tests/desktop/writer3/reconnect_spec.js b/cypress_test/integration_tests/desktop/writer3/reconnect_spec.js
index 2d863650ebc5..e23e14260f78 100644
--- a/cypress_test/integration_tests/desktop/writer3/reconnect_spec.js
+++ b/cypress_test/integration_tests/desktop/writer3/reconnect_spec.js
@@ -1,4 +1,4 @@
-/* global describe it cy beforeEach require Cypress */
+/* global describe it cy beforeEach require expect */
var helper = require('../../common/helper');
var desktopHelper = require('../../common/desktop_helper');
@@ -16,6 +16,12 @@ describe(['tagdesktop'], 'WebSocket reconnection', function () {
helper.typeIntoDocument('{ctrl}{End}');
desktopHelper.assertVisiblePage(12, 12, 12);
+ var preDisconnectY1;
+ cy.getFrameWindow().then(function (win) {
+ preDisconnectY1 = win.app.activeDocument.activeLayout.viewedRectangle.y1;
+ expect(preDisconnectY1).to.be.greaterThan(0);
+ });
+
// Close the raw WebSocket to trigger automatic reconnection
cy.getFrameWindow().then(function (win) {
win.app.socket.socket.close();
@@ -24,7 +30,6 @@ describe(['tagdesktop'], 'WebSocket reconnection', function () {
// Can't use processToIdle with the socket closed
cy.wait(500);
- // Wait for reconnection to complete
cy.cGet('#document-canvas').should('be.visible');
cy.getFrameWindow().its('app.socket._reconnecting')
.should('eq', false);
@@ -32,7 +37,18 @@ describe(['tagdesktop'], 'WebSocket reconnection', function () {
helper.processToIdle(win);
});
- // Verify the page position is preserved after reconnection
desktopHelper.assertVisiblePage(12, 12, 12);
+
+ cy.cGet('#document-canvas').click(200, 200);
+ cy.getFrameWindow().then(function (win) {
+ helper.processToIdle(win);
+ });
+
+ desktopHelper.assertVisiblePage(12, 12, 12);
+ cy.getFrameWindow().then(function (win) {
+ var postClickY1 = win.app.activeDocument.activeLayout.viewedRectangle.y1;
+ var drift = Math.abs(postClickY1 - preDisconnectY1);
+ expect(drift).to.be.lessThan(preDisconnectY1 / 2);
+ });
});
});
"Jaume Pujantell (via cogerrit)"
May 11, 2026, 7:13:21 AMMay 11
to collaboraon...@googlegroups.com
browser/src/canvas/sections/CommentListSection.ts | 55 ++--------
cypress_test/integration_tests/desktop/writer1/annotation_spec.js | 38 ++++++
2 files changed, 51 insertions(+), 42 deletions(-)
New commits:
commit 4b4b1bac99853415f5b367197b6fba827f0a958b
Author: Jaume Pujantell <jaume.p...@collabora.com>
AuthorDate: Fri May 1 11:21:22 2026 +0200
Commit: Miklos Vajna <vmi...@collabora.com>
CommitDate: Mon May 11 11:12:23 2026 +0000
comments: unresolve thread only if all is resolved
Right now the "Unresolve Thread" option appears even if only some
comments in a thread are resolved, but the internal UNO call resolves
the thread (as it should). This change fixes the label so that it
corresponds to the correct action. A cypress test is also added.
Signed-off-by: Jaume Pujantell <jaume.p...@collabora.com>
Change-Id: I4867d44b76357d5c1af66efec3973c455abb77bf
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/2061
index f2919ee598b0..29ecf02c3b75 100644
--- a/browser/src/canvas/sections/CommentListSection.ts
+++ b/browser/src/canvas/sections/CommentListSection.ts
@@ -1528,29 +1528,22 @@ export class CommentSection extends CanvasSectionObject {
return (index === undefined) ? -1 : index;
}
- public isThreadResolved (annotation: any): boolean {
- // If comment has children.
- if (annotation.sectionProperties.children.length > 0) {
- for (var i = 0; i < annotation.sectionProperties.children.length; i++) {
- if (annotation.sectionProperties.children[i].sectionProperties.data.resolved !== 'true')
- return false;
- }
- return true;
- }
- // If it has a parent.
- else if (annotation.sectionProperties.data.parent !== '0') {
- const index = this.getSubRootIndexOf(annotation.sectionProperties.data.parent);
- const comment = this.sectionProperties.commentList[index];
- if (comment.sectionProperties.data.resolved !== 'true')
+ private isSubThreadResolved(annotation: Comment): boolean {
+ if (annotation.sectionProperties.data.resolved !== 'true')
+ return false;
+ for (var i = 0; i < annotation.sectionProperties.children.length; i++) {
+ if (!this.isSubThreadResolved(annotation.sectionProperties.children[i]))
return false;
- else if (comment.sectionProperties.children.length > 0) {
- for (var i = 0; i < comment.sectionProperties.children.length; i++) {
- if (comment.sectionProperties.children[i].sectionProperties.data.resolved !== 'true')
- return false;
- }
- return true;
- }
}
+ return true;
+ }
+
+ public isThreadResolved(annotation: Comment): boolean {
+ if (annotation.isRootComment())
+ return this.isSubThreadResolved(annotation);
+ const index = this.getRootIndexOf(annotation.sectionProperties.data.parent);
+ const top_comment = this.sectionProperties.commentList[index];
+ return this.isSubThreadResolved(top_comment);
}
public isShownBig (annotation: any): boolean {
@@ -2487,26 +2480,6 @@ export class CommentSection extends CanvasSectionObject {
return index;
}
- // Returns the sub-root comment index of given id
- private getSubRootIndexOf (id: any): number {
- var index = this.getIndexOf(id);
-
- if (index !== -1)
- {
- var comment = this.sectionProperties.commentList[index];
- var parentId = comment.sectionProperties.data.parent;
-
- while (index >= 0) {
- if (this.sectionProperties.commentList[index].sectionProperties.data.id !== parentId && this.sectionProperties.commentList[index].sectionProperties.data.parent !== '0')
- index--;
- else
- break;
- }
- }
-
- return index;
- }
-
public setViewResolved (state: boolean): void {
this.sectionProperties.showResolved = state;
diff --git a/cypress_test/integration_tests/desktop/writer1/annotation_spec.js b/cypress_test/integration_tests/desktop/writer1/annotation_spec.js
index 1b00d3e48980..3da6532668b7 100644
--- a/cypress_test/integration_tests/desktop/writer1/annotation_spec.js
+++ b/cypress_test/integration_tests/desktop/writer1/annotation_spec.js
@@ -161,7 +161,7 @@ describe(['tagdesktop'], 'Annotation Tests', function() {
cy.cGet('#comment-container-1').should('be.not.visible');
});
- it('Visibility at Different Window Widths (increasing)', function() {
+ it('Visibility at Different Window Widths (increasing)', function () {
/*
1. start with collapsed comment and increase window width
2. cy.viewport(1400, 600); at 150% comment is collapsed
@@ -465,6 +465,42 @@ describe(['tagdesktop'], 'Annotation Tests', function() {
cy.cGet('#annotation-modify-textarea-1').should('have.focus');
});
+ it('Resolve/Unresolve Thread on partially resolved thread', function () {
+ desktopHelper.insertComment();
+ cy.cGet('#comment-container-1').should('exist');
+
+ // Reply to create a thread (root id 1, reply id 2).
+ 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').type('reply text');
+ cy.cGet('#annotation-reply-1').click();
+ cy.cGet('#annotation-content-area-2').should('contain', 'reply text');
+
+ // Resolve only the reply, leaving the root unresolved.
+ cy.cGet('#comment-annotation-menu-2').click();
+ cy.cGet('body').contains('.ui-combobox-entry.jsdialog.ui-grid-cell', 'Resolve').click();
+ cy.cGet('#comment-container-2 .cool-annotation-content-resolved').should('have.text', 'Resolved');
+ cy.cGet('#comment-container-1 .cool-annotation-content-resolved').should('have.text', '');
+
+ // Root menu must offer 'Resolve Thread' since the thread is not fully resolved.
+ cy.cGet('#comment-annotation-menu-1').click();
+ cy.cGet('body').contains('.ui-combobox-entry.jsdialog.ui-grid-cell', 'Resolve Thread').should('be.visible');
+ cy.cGet('body').contains('.ui-combobox-entry.jsdialog.ui-grid-cell', 'Unresolve Thread').should('not.exist');
+ cy.cGet('body').contains('.ui-combobox-entry.jsdialog.ui-grid-cell', 'Resolve Thread').click();
+
+ // All comments in the thread are now resolved.
+ cy.cGet('#comment-container-1 .cool-annotation-content-resolved').should('have.text', 'Resolved');
+ cy.cGet('#comment-container-2 .cool-annotation-content-resolved').should('have.text', 'Resolved');
+
+ // Root menu now offers 'Unresolve Thread'.
+ cy.cGet('#comment-annotation-menu-1').click();
+ cy.cGet('body').contains('.ui-combobox-entry.jsdialog.ui-grid-cell', 'Unresolve Thread').should('be.visible');
+ cy.cGet('body').contains('.ui-combobox-entry.jsdialog.ui-grid-cell', 'Unresolve Thread').click();
+
+ // All comments in the thread are unresolved again.
+ cy.cGet('#comment-container-1 .cool-annotation-content-resolved').should('have.text', '');
+ cy.cGet('#comment-container-2 .cool-annotation-content-resolved').should('have.text', '');
+ });
});
describe(['tagdesktop'], 'Collapsed Annotation Tests', function() {
cypress_test/integration_tests/desktop/writer1/annotation_spec.js | 38 ++++++
2 files changed, 51 insertions(+), 42 deletions(-)
New commits:
commit 4b4b1bac99853415f5b367197b6fba827f0a958b
Author: Jaume Pujantell <jaume.p...@collabora.com>
AuthorDate: Fri May 1 11:21:22 2026 +0200
Commit: Miklos Vajna <vmi...@collabora.com>
CommitDate: Mon May 11 11:12:23 2026 +0000
comments: unresolve thread only if all is resolved
Right now the "Unresolve Thread" option appears even if only some
comments in a thread are resolved, but the internal UNO call resolves
the thread (as it should). This change fixes the label so that it
corresponds to the correct action. A cypress test is also added.
Signed-off-by: Jaume Pujantell <jaume.p...@collabora.com>
Change-Id: I4867d44b76357d5c1af66efec3973c455abb77bf
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/2061
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
Reviewed-by: Miklos Vajna <vmi...@collabora.com>
index f2919ee598b0..29ecf02c3b75 100644
--- a/browser/src/canvas/sections/CommentListSection.ts
+++ b/browser/src/canvas/sections/CommentListSection.ts
@@ -1528,29 +1528,22 @@ export class CommentSection extends CanvasSectionObject {
return (index === undefined) ? -1 : index;
}
- public isThreadResolved (annotation: any): boolean {
- // If comment has children.
- if (annotation.sectionProperties.children.length > 0) {
- for (var i = 0; i < annotation.sectionProperties.children.length; i++) {
- if (annotation.sectionProperties.children[i].sectionProperties.data.resolved !== 'true')
- return false;
- }
- return true;
- }
- // If it has a parent.
- else if (annotation.sectionProperties.data.parent !== '0') {
- const index = this.getSubRootIndexOf(annotation.sectionProperties.data.parent);
- const comment = this.sectionProperties.commentList[index];
- if (comment.sectionProperties.data.resolved !== 'true')
+ private isSubThreadResolved(annotation: Comment): boolean {
+ if (annotation.sectionProperties.data.resolved !== 'true')
+ return false;
+ for (var i = 0; i < annotation.sectionProperties.children.length; i++) {
+ if (!this.isSubThreadResolved(annotation.sectionProperties.children[i]))
return false;
- else if (comment.sectionProperties.children.length > 0) {
- for (var i = 0; i < comment.sectionProperties.children.length; i++) {
- if (comment.sectionProperties.children[i].sectionProperties.data.resolved !== 'true')
- return false;
- }
- return true;
- }
}
+ return true;
+ }
+
+ public isThreadResolved(annotation: Comment): boolean {
+ if (annotation.isRootComment())
+ return this.isSubThreadResolved(annotation);
+ const index = this.getRootIndexOf(annotation.sectionProperties.data.parent);
+ const top_comment = this.sectionProperties.commentList[index];
+ return this.isSubThreadResolved(top_comment);
}
public isShownBig (annotation: any): boolean {
@@ -2487,26 +2480,6 @@ export class CommentSection extends CanvasSectionObject {
return index;
}
- // Returns the sub-root comment index of given id
- private getSubRootIndexOf (id: any): number {
- var index = this.getIndexOf(id);
-
- if (index !== -1)
- {
- var comment = this.sectionProperties.commentList[index];
- var parentId = comment.sectionProperties.data.parent;
-
- while (index >= 0) {
- if (this.sectionProperties.commentList[index].sectionProperties.data.id !== parentId && this.sectionProperties.commentList[index].sectionProperties.data.parent !== '0')
- index--;
- else
- break;
- }
- }
-
- return index;
- }
-
public setViewResolved (state: boolean): void {
this.sectionProperties.showResolved = state;
diff --git a/cypress_test/integration_tests/desktop/writer1/annotation_spec.js b/cypress_test/integration_tests/desktop/writer1/annotation_spec.js
index 1b00d3e48980..3da6532668b7 100644
--- a/cypress_test/integration_tests/desktop/writer1/annotation_spec.js
+++ b/cypress_test/integration_tests/desktop/writer1/annotation_spec.js
@@ -161,7 +161,7 @@ describe(['tagdesktop'], 'Annotation Tests', function() {
cy.cGet('#comment-container-1').should('be.not.visible');
});
- it('Visibility at Different Window Widths (increasing)', function() {
+ it('Visibility at Different Window Widths (increasing)', function () {
/*
1. start with collapsed comment and increase window width
2. cy.viewport(1400, 600); at 150% comment is collapsed
@@ -465,6 +465,42 @@ describe(['tagdesktop'], 'Annotation Tests', function() {
cy.cGet('#annotation-modify-textarea-1').should('have.focus');
});
+ it('Resolve/Unresolve Thread on partially resolved thread', function () {
+ desktopHelper.insertComment();
+ cy.cGet('#comment-container-1').should('exist');
+
+ // Reply to create a thread (root id 1, reply id 2).
+ 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').type('reply text');
+ cy.cGet('#annotation-reply-1').click();
+ cy.cGet('#annotation-content-area-2').should('contain', 'reply text');
+
+ // Resolve only the reply, leaving the root unresolved.
+ cy.cGet('#comment-annotation-menu-2').click();
+ cy.cGet('body').contains('.ui-combobox-entry.jsdialog.ui-grid-cell', 'Resolve').click();
+ cy.cGet('#comment-container-2 .cool-annotation-content-resolved').should('have.text', 'Resolved');
+ cy.cGet('#comment-container-1 .cool-annotation-content-resolved').should('have.text', '');
+
+ // Root menu must offer 'Resolve Thread' since the thread is not fully resolved.
+ cy.cGet('#comment-annotation-menu-1').click();
+ cy.cGet('body').contains('.ui-combobox-entry.jsdialog.ui-grid-cell', 'Resolve Thread').should('be.visible');
+ cy.cGet('body').contains('.ui-combobox-entry.jsdialog.ui-grid-cell', 'Unresolve Thread').should('not.exist');
+ cy.cGet('body').contains('.ui-combobox-entry.jsdialog.ui-grid-cell', 'Resolve Thread').click();
+
+ // All comments in the thread are now resolved.
+ cy.cGet('#comment-container-1 .cool-annotation-content-resolved').should('have.text', 'Resolved');
+ cy.cGet('#comment-container-2 .cool-annotation-content-resolved').should('have.text', 'Resolved');
+
+ // Root menu now offers 'Unresolve Thread'.
+ cy.cGet('#comment-annotation-menu-1').click();
+ cy.cGet('body').contains('.ui-combobox-entry.jsdialog.ui-grid-cell', 'Unresolve Thread').should('be.visible');
+ cy.cGet('body').contains('.ui-combobox-entry.jsdialog.ui-grid-cell', 'Unresolve Thread').click();
+
+ // All comments in the thread are unresolved again.
+ cy.cGet('#comment-container-1 .cool-annotation-content-resolved').should('have.text', '');
+ cy.cGet('#comment-container-2 .cool-annotation-content-resolved').should('have.text', '');
+ });
});
describe(['tagdesktop'], 'Collapsed Annotation Tests', function() {
"Gökay Şatır (via cogerrit)"
May 12, 2026, 5:05:47 AM (14 days ago) May 12
to collaboraon...@googlegroups.com
browser/src/canvas/sections/CommentSection.ts | 35 +++
cypress_test/integration_tests/desktop/writer1/annotation_spec.js | 95 +++++++++-
2 files changed, 129 insertions(+), 1 deletion(-)
New commits:
commit 983cb9783e3d3889899ff7b3332eb8cc3a78c07b
Author: Gökay Şatır <gokay...@collabora.com>
AuthorDate: Mon May 11 12:04:50 2026 +0300
Commit: Gökay Şatır <gokay...@collabora.com>
CommitDate: Tue May 12 09:05:27 2026 +0000
Writer: Starting selection from a commented portion of text is not possible.
Issue: Because comment section handles the events.
Fix: Redirect the dragging actions to mouse controller. It doesn't interfere with other comment section actions because it doesn't handle dragging currently.
Also a test is added and fix is confirmed.
Signed-off-by: Gökay Şatır <gokay...@collabora.com>
Change-Id: Ibdc26f7dae8aa3fa08a269b5787cf245e3b3a854
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/2360
index bdec576f0243..fb077490f6ed 100644
--- a/browser/src/canvas/sections/CommentSection.ts
+++ b/browser/src/canvas/sections/CommentSection.ts
@@ -1815,7 +1815,42 @@ export class Comment extends CanvasSectionObject {
}
}
+ // In Writer, a comment-highlighted region overlays the document. By default
+ // this section would swallow mouse events, so a text selection drag started
+ // on top of a commented passage never reaches core. Forward the drag
+ // lifecycle (down/move/up) to MouseControl while leaving click handling
+ // to the existing onClick path.
+ private forwardWriterMouseEventToCore(handler: 'onMouseDown' | 'onMouseMove' | 'onMouseUp', point: cool.SimplePoint, dragDistance: Array<number>, e: MouseEvent): void {
+ const mousePoint = point.clone();
+ mousePoint.pX += this.myTopLeft[0];
+ mousePoint.pY += this.myTopLeft[1];
+ const mouseControl = app.activeDocument.mouseControl;
+ if (handler === 'onMouseMove')
+ mouseControl.onMouseMove(mousePoint, dragDistance, e);
+ else if (handler === 'onMouseDown')
+ mouseControl.onMouseDown(mousePoint, e);
+ else
+ mouseControl.onMouseUp(mousePoint, e);
+ }
+
+ public onMouseDown(point: cool.SimplePoint, e: MouseEvent): void {
+ if (app.map._docLayer._docType === 'text')
+ this.forwardWriterMouseEventToCore('onMouseDown', point, [0, 0], e);
+ }
+
+ public onMouseMove(point: cool.SimplePoint, dragDistance: Array<number>, e: MouseEvent): void {
+ if (app.map._docLayer._docType === 'text' && this.containerObject.isDraggingSomething())
+ this.forwardWriterMouseEventToCore('onMouseMove', point, dragDistance, e);
+ }
+
public onMouseUp (point: cool.SimplePoint, e: MouseEvent): void {
+ // A drag finishing on top of a commented region: let MouseControl
+ // send the matching buttonup so core completes the selection.
+ if (this.containerObject.isDraggingSomething() && app.map._docLayer._docType === 'text') {
+ this.forwardWriterMouseEventToCore('onMouseUp', point, [0, 0], e);
+ return;
+ }
+
// Hammer.js doesn't fire onClick event after touchEnd event.
// CanvasSectionContainer fires the onClick event. But since Hammer.js is used for map, it disables the onClick for SectionContainer.
// We will use this event as click event on touch devices, until we remove Hammer.js (then this code will be removed from here).
diff --git a/cypress_test/integration_tests/desktop/writer1/annotation_spec.js b/cypress_test/integration_tests/desktop/writer1/annotation_spec.js
index 3da6532668b7..dff9bd420763 100644
--- a/cypress_test/integration_tests/desktop/writer1/annotation_spec.js
+++ b/cypress_test/integration_tests/desktop/writer1/annotation_spec.js
@@ -232,7 +232,7 @@ describe(['tagdesktop'], 'Annotation Tests', function() {
cy.cGet('#comment-container-1').should('be.visible');
/*
- at this point, the space on the left of the document and the
+ at this point, the space on the left of the document and the
space on the right of the document (without moving the document
to the left) is same, equal to half of the comment width;
*/
@@ -353,6 +353,99 @@ describe(['tagdesktop'], 'Annotation Tests', function() {
});
});
+ it('Drag inside commented region forwards mouse events to core', function() {
+ // A Writer comment overlays the commented passage with a
+ // CommentSection that used to swallow mouse events. That blocked
+ // users from starting a text selection by mouse-dragging from
+ // inside the highlighted passage - core never saw the drag.
+ // CommentSection now delegates onMouseDown/Move/Up to MouseControl
+ // when a drag is active; this test exercises that path.
+
+ // 50% zoom (set in beforeEach) makes the highlight too small for a
+ // reliable drag, so switch to 100%.
+ desktopHelper.selectZoomLevel('100', false);
+ cy.getFrameWindow().then(function(win) {
+ return helper.processToIdle(win);
+ });
+
+ // Lay down a passage and select a slice of it so the comment is
+ // anchored to a real range and ends up with a meaningful
+ // highlight rectangle to drag inside.
+ helper.typeIntoDocument('Hello world from this test document for drag testing.{enter}');
+ helper.typeIntoDocument('{ctrl}{home}');
+ for (var i = 0; i < 17; ++i)
+ helper.typeIntoDocument('{shift}{rightArrow}');
+ helper.textSelectionShouldExist();
+
+ desktopHelper.insertComment('drag-test', true);
+ cy.cGet('.cool-annotation-content-wrapper').should('exist');
+
+ cy.getFrameWindow().then(function(win) {
+ return helper.processToIdle(win);
+ });
+
+ // Spy on MouseControl - if the new delegation works, the drag we
+ // dispatch below must reach these methods.
+ cy.getFrameWindow().then(function(win) {
+ cy.spy(win.app.activeDocument.mouseControl, 'onMouseDown').as('mcDown');
+ cy.spy(win.app.activeDocument.mouseControl, 'onMouseMove').as('mcMove');
+ cy.spy(win.app.activeDocument.mouseControl, 'onMouseUp').as('mcUp');
+ });
+
+ cy.getFrameWindow().then(function(win) {
+ var commentList = win.app.sectionContainer.getSectionWithName(
+ win.app.CSections.CommentList.name);
+ var comment = commentList.sectionProperties.commentList[0];
+ expect(comment, 'comment must exist').to.exist;
+ expect(comment.sectionProperties.data.rectangles,
+ 'comment must own a highlight rectangle').to.exist;
+ var rects = comment.sectionProperties.data.rectangles;
+ expect(rects.length, 'at least one rectangle').to.be.greaterThan(0);
+ var rect = rects[0];
+
+ var canvas = win.document.getElementById('document-canvas');
+ var bounds = canvas.getBoundingClientRect();
+
+ // v1X/v1Y are view pixels inside the canvas (viewport offset
+ // baked in); /dpiScale -> CSS pixels.
+ var cssCenterX = ((rect.v1X + rect.v2X) / 2) / win.app.dpiScale;
+ var cssCenterY = ((rect.v1Y + rect.v4Y) / 2) / win.app.dpiScale;
+ var startX = bounds.left + cssCenterX - 10;
+ var startY = bounds.top + cssCenterY;
+ var endX = startX + 30;
+ var endY = startY;
+
+ // mouseenter primes mouseIsInside; the container's mousedown
+ // short-circuits otherwise.
+ canvas.dispatchEvent(new win.MouseEvent('mouseenter', {
+ clientX: startX, clientY: startY, button: 0, bubbles: true,
+ }));
+ canvas.dispatchEvent(new win.MouseEvent('mousedown', {
+ clientX: startX, clientY: startY, button: 0, bubbles: true,
+ }));
+ // mousemove and mouseup are wired on document, not canvas.
+ win.document.dispatchEvent(new win.MouseEvent('mousemove', {
+ clientX: endX, clientY: endY, button: 0, buttons: 1, bubbles: true,
+ }));
+ win.document.dispatchEvent(new win.MouseEvent('mouseup', {
+ clientX: endX, clientY: endY, button: 0, bubbles: true,
+ }));
+ });
+
+ // Each phase must have flowed through MouseControl. Without the
+ // CommentSection delegation, the spies stay silent because the
+ // section consumed the events.
+ cy.get('@mcDown').should('have.been.called');
+ cy.get('@mcMove').should('have.been.called');
+ cy.get('@mcUp').should('have.been.called');
+
+ // And core must have responded with a real text selection.
+ cy.getFrameWindow().then(function(win) {
+ return helper.processToIdle(win);
+ });
+ helper.textSelectionShouldExist();
+ });
+
it('Action_GoToComment postMessage returns error for invalid comment', function() {
// Stub postMessage to capture the response.
cy.getFrameWindow().then(win => {
cypress_test/integration_tests/desktop/writer1/annotation_spec.js | 95 +++++++++-
2 files changed, 129 insertions(+), 1 deletion(-)
New commits:
commit 983cb9783e3d3889899ff7b3332eb8cc3a78c07b
Author: Gökay Şatır <gokay...@collabora.com>
AuthorDate: Mon May 11 12:04:50 2026 +0300
Commit: Gökay Şatır <gokay...@collabora.com>
CommitDate: Tue May 12 09:05:27 2026 +0000
Writer: Starting selection from a commented portion of text is not possible.
Issue: Because comment section handles the events.
Fix: Redirect the dragging actions to mouse controller. It doesn't interfere with other comment section actions because it doesn't handle dragging currently.
Also a test is added and fix is confirmed.
Signed-off-by: Gökay Şatır <gokay...@collabora.com>
Change-Id: Ibdc26f7dae8aa3fa08a269b5787cf245e3b3a854
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/2360
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
Reviewed-by: Miklos Vajna <vmi...@collabora.com>
index bdec576f0243..fb077490f6ed 100644
--- a/browser/src/canvas/sections/CommentSection.ts
+++ b/browser/src/canvas/sections/CommentSection.ts
@@ -1815,7 +1815,42 @@ export class Comment extends CanvasSectionObject {
}
}
+ // In Writer, a comment-highlighted region overlays the document. By default
+ // this section would swallow mouse events, so a text selection drag started
+ // on top of a commented passage never reaches core. Forward the drag
+ // lifecycle (down/move/up) to MouseControl while leaving click handling
+ // to the existing onClick path.
+ private forwardWriterMouseEventToCore(handler: 'onMouseDown' | 'onMouseMove' | 'onMouseUp', point: cool.SimplePoint, dragDistance: Array<number>, e: MouseEvent): void {
+ const mousePoint = point.clone();
+ mousePoint.pX += this.myTopLeft[0];
+ mousePoint.pY += this.myTopLeft[1];
+ const mouseControl = app.activeDocument.mouseControl;
+ if (handler === 'onMouseMove')
+ mouseControl.onMouseMove(mousePoint, dragDistance, e);
+ else if (handler === 'onMouseDown')
+ mouseControl.onMouseDown(mousePoint, e);
+ else
+ mouseControl.onMouseUp(mousePoint, e);
+ }
+
+ public onMouseDown(point: cool.SimplePoint, e: MouseEvent): void {
+ if (app.map._docLayer._docType === 'text')
+ this.forwardWriterMouseEventToCore('onMouseDown', point, [0, 0], e);
+ }
+
+ public onMouseMove(point: cool.SimplePoint, dragDistance: Array<number>, e: MouseEvent): void {
+ if (app.map._docLayer._docType === 'text' && this.containerObject.isDraggingSomething())
+ this.forwardWriterMouseEventToCore('onMouseMove', point, dragDistance, e);
+ }
+
public onMouseUp (point: cool.SimplePoint, e: MouseEvent): void {
+ // A drag finishing on top of a commented region: let MouseControl
+ // send the matching buttonup so core completes the selection.
+ if (this.containerObject.isDraggingSomething() && app.map._docLayer._docType === 'text') {
+ this.forwardWriterMouseEventToCore('onMouseUp', point, [0, 0], e);
+ return;
+ }
+
// Hammer.js doesn't fire onClick event after touchEnd event.
// CanvasSectionContainer fires the onClick event. But since Hammer.js is used for map, it disables the onClick for SectionContainer.
// We will use this event as click event on touch devices, until we remove Hammer.js (then this code will be removed from here).
diff --git a/cypress_test/integration_tests/desktop/writer1/annotation_spec.js b/cypress_test/integration_tests/desktop/writer1/annotation_spec.js
index 3da6532668b7..dff9bd420763 100644
--- a/cypress_test/integration_tests/desktop/writer1/annotation_spec.js
+++ b/cypress_test/integration_tests/desktop/writer1/annotation_spec.js
@@ -232,7 +232,7 @@ describe(['tagdesktop'], 'Annotation Tests', function() {
cy.cGet('#comment-container-1').should('be.visible');
/*
- at this point, the space on the left of the document and the
+ at this point, the space on the left of the document and the
space on the right of the document (without moving the document
to the left) is same, equal to half of the comment width;
*/
@@ -353,6 +353,99 @@ describe(['tagdesktop'], 'Annotation Tests', function() {
});
});
+ it('Drag inside commented region forwards mouse events to core', function() {
+ // A Writer comment overlays the commented passage with a
+ // CommentSection that used to swallow mouse events. That blocked
+ // users from starting a text selection by mouse-dragging from
+ // inside the highlighted passage - core never saw the drag.
+ // CommentSection now delegates onMouseDown/Move/Up to MouseControl
+ // when a drag is active; this test exercises that path.
+
+ // 50% zoom (set in beforeEach) makes the highlight too small for a
+ // reliable drag, so switch to 100%.
+ desktopHelper.selectZoomLevel('100', false);
+ cy.getFrameWindow().then(function(win) {
+ return helper.processToIdle(win);
+ });
+
+ // Lay down a passage and select a slice of it so the comment is
+ // anchored to a real range and ends up with a meaningful
+ // highlight rectangle to drag inside.
+ helper.typeIntoDocument('Hello world from this test document for drag testing.{enter}');
+ helper.typeIntoDocument('{ctrl}{home}');
+ for (var i = 0; i < 17; ++i)
+ helper.typeIntoDocument('{shift}{rightArrow}');
+ helper.textSelectionShouldExist();
+
+ desktopHelper.insertComment('drag-test', true);
+ cy.cGet('.cool-annotation-content-wrapper').should('exist');
+
+ cy.getFrameWindow().then(function(win) {
+ return helper.processToIdle(win);
+ });
+
+ // Spy on MouseControl - if the new delegation works, the drag we
+ // dispatch below must reach these methods.
+ cy.getFrameWindow().then(function(win) {
+ cy.spy(win.app.activeDocument.mouseControl, 'onMouseDown').as('mcDown');
+ cy.spy(win.app.activeDocument.mouseControl, 'onMouseMove').as('mcMove');
+ cy.spy(win.app.activeDocument.mouseControl, 'onMouseUp').as('mcUp');
+ });
+
+ cy.getFrameWindow().then(function(win) {
+ var commentList = win.app.sectionContainer.getSectionWithName(
+ win.app.CSections.CommentList.name);
+ var comment = commentList.sectionProperties.commentList[0];
+ expect(comment, 'comment must exist').to.exist;
+ expect(comment.sectionProperties.data.rectangles,
+ 'comment must own a highlight rectangle').to.exist;
+ var rects = comment.sectionProperties.data.rectangles;
+ expect(rects.length, 'at least one rectangle').to.be.greaterThan(0);
+ var rect = rects[0];
+
+ var canvas = win.document.getElementById('document-canvas');
+ var bounds = canvas.getBoundingClientRect();
+
+ // v1X/v1Y are view pixels inside the canvas (viewport offset
+ // baked in); /dpiScale -> CSS pixels.
+ var cssCenterX = ((rect.v1X + rect.v2X) / 2) / win.app.dpiScale;
+ var cssCenterY = ((rect.v1Y + rect.v4Y) / 2) / win.app.dpiScale;
+ var startX = bounds.left + cssCenterX - 10;
+ var startY = bounds.top + cssCenterY;
+ var endX = startX + 30;
+ var endY = startY;
+
+ // mouseenter primes mouseIsInside; the container's mousedown
+ // short-circuits otherwise.
+ canvas.dispatchEvent(new win.MouseEvent('mouseenter', {
+ clientX: startX, clientY: startY, button: 0, bubbles: true,
+ }));
+ canvas.dispatchEvent(new win.MouseEvent('mousedown', {
+ clientX: startX, clientY: startY, button: 0, bubbles: true,
+ }));
+ // mousemove and mouseup are wired on document, not canvas.
+ win.document.dispatchEvent(new win.MouseEvent('mousemove', {
+ clientX: endX, clientY: endY, button: 0, buttons: 1, bubbles: true,
+ }));
+ win.document.dispatchEvent(new win.MouseEvent('mouseup', {
+ clientX: endX, clientY: endY, button: 0, bubbles: true,
+ }));
+ });
+
+ // Each phase must have flowed through MouseControl. Without the
+ // CommentSection delegation, the spies stay silent because the
+ // section consumed the events.
+ cy.get('@mcDown').should('have.been.called');
+ cy.get('@mcMove').should('have.been.called');
+ cy.get('@mcUp').should('have.been.called');
+
+ // And core must have responded with a real text selection.
+ cy.getFrameWindow().then(function(win) {
+ return helper.processToIdle(win);
+ });
+ helper.textSelectionShouldExist();
+ });
+
it('Action_GoToComment postMessage returns error for invalid comment', function() {
// Stub postMessage to capture the response.
cy.getFrameWindow().then(win => {
"Gökay Şatır (via cogerrit)"
May 18, 2026, 6:56:07 AM (7 days ago) May 18
to collaboraon...@googlegroups.com
browser/src/canvas/sections/AutoFillBaseSection.ts | 14 ++
cypress_test/integration_tests/mobile/calc/autofill_marker_spec.js | 57 ++++++++++
2 files changed, 71 insertions(+)
New commits:
commit 1f134fe3134baba4cdcf823bfbdb1582d0032ae1
Author: Gökay Şatır <gokay...@collabora.com>
AuthorDate: Thu May 7 17:17:55 2026 +0300
Commit: Caolán McNamara <caolan....@collabora.com>
CommitDate: Mon May 18 10:54:52 2026 +0000
Calc: Fix autofillmarker behaviour on mobile.
Issue: Autofill marker doesn't work on mobile view.
Cause: Since the position of marker is different than the desktop view, we need to alter it before sending it core side.
Fix: Alter the position before sending it to core side.
A test is also added and I confirmed that the new test doesn't pass without the fix.
Signed-off-by: Gökay Şatır <gokay...@collabora.com>
Change-Id: I6bb85a47064c46dcf9651ea50e787c85c69c3322
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/2138
Tested-by: Caolán McNamara <caolan....@collabora.com>
Reviewed-by: Caolán McNamara <caolan....@collabora.com>
Tested-by: Jenkins CPCI <rel...@collaboraoffice.com>
diff --git a/browser/src/canvas/sections/AutoFillBaseSection.ts b/browser/src/canvas/sections/AutoFillBaseSection.ts
index 6b3b8c27ae9d..9dd24c7caf88 100644
--- a/browser/src/canvas/sections/AutoFillBaseSection.ts
+++ b/browser/src/canvas/sections/AutoFillBaseSection.ts
@@ -210,6 +210,16 @@ class AutoFillBaseSection extends CanvasSectionObject {
return p2;
}
+ // On mobile, setMarkerPosition shifts the marker left by half the cell
+ // width so it sits visually under the cell. Core's autofill hit-test is at
+ // the cell's bottom-right corner, so undo that shift before posting events.
+ private adjustForMobileCenterOffset(p: cool.SimplePoint): void {
+ if (!(<any>window).mode.isDesktop()) {
+ Util.ensureValue(app.calc.cellCursorRectangle);
+ p.pX += app.calc.cellCursorRectangle.pWidth * 0.5;
+ }
+ }
+
protected autoScroll(point: cool.SimplePoint) {
Util.ensureValue(app.activeDocument);
const viewedRectangle = app.activeDocument.activeLayout.viewedRectangle;
@@ -237,6 +247,7 @@ class AutoFillBaseSection extends CanvasSectionObject {
return; // No dragging or no event handling or auto fill marker is not visible.
const p2 = this.getDocumentPositionFromLocal(point);
+ this.adjustForMobileCenterOffset(p2);
app.map._docLayer._postMouseEvent('move', p2.x, p2.y, 1, 1, 0);
if (
@@ -248,6 +259,7 @@ class AutoFillBaseSection extends CanvasSectionObject {
public onMouseUp(point: cool.SimplePoint, e: MouseEvent) {
const p2 = this.getDocumentPositionFromLocal(point);
+ this.adjustForMobileCenterOffset(p2);
app.map._docLayer._postMouseEvent('buttonup', p2.x, p2.y, 1, 1, 0);
}
@@ -255,6 +267,7 @@ class AutoFillBaseSection extends CanvasSectionObject {
// revert coordinates to global and fire event again with position in the center
// inverse of convertPositionToCanvasLocale
const p2 = this.getCenterRegardingDocument();
+ this.adjustForMobileCenterOffset(p2);
app.map._docLayer._postMouseEvent('buttondown', p2.x, p2.y, 1, 1, 0);
}
@@ -269,6 +282,7 @@ class AutoFillBaseSection extends CanvasSectionObject {
public onDoubleClick(point: cool.SimplePoint, e: MouseEvent) {
const pos = this.getCenterRegardingDocument();
+ this.adjustForMobileCenterOffset(pos);
this.sectionProperties.docLayer._postMouseEvent(
'buttondown',
pos.x,
diff --git a/cypress_test/integration_tests/mobile/calc/autofill_marker_spec.js b/cypress_test/integration_tests/mobile/calc/autofill_marker_spec.js
new file mode 100644
index 000000000000..18191871b544
--- /dev/null
+++ b/cypress_test/integration_tests/mobile/calc/autofill_marker_spec.js
@@ -0,0 +1,57 @@
+var helper = require('../../common/helper');
+var calcHelper = require('../../common/calc_helper');
+var mobileHelper = require('../../common/mobile_helper');
+
+describe(['tagmobile', 'tagnextcloud'], 'Calc autofill marker on mobile.', function() {
+
+ beforeEach(function() {
+ helper.setupAndLoadDocument('calc/formulabar.ods');
+ mobileHelper.enableEditingMobile();
+ });
+
+ // On mobile the autofill marker is rendered at the bottom-centre of the
+ // selected cell instead of the bottom-right corner like on desktop. The
+ // mouse events posted to core must still be at the bottom-right corner
+ // where core's autofill hit-test lives, otherwise the drag has no effect
+ // and no cells get filled.
+ it('Drag autofill marker fills cells below the source', function() {
+ var expectedText = 'long line long line long line';
+
+ calcHelper.clickOnFirstCell();
+ cy.cGet('[id="test-div-auto fill marker"]').should('exist');
+
+ // Cypress synthetic DOM events are guarded against by the canvas
+ // section container (mouseIsInside, button-state checks). Invoke
+ // the marker's handlers directly so the coordinate computation is
+ // exercised against the live core kit.
+ cy.getFrameWindow().then(function(win) {
+ var marker = win.app.sectionContainer.getSectionWithName('auto fill marker');
+ expect(marker, 'autofill marker section').to.exist;
+
+ var halfWidth = Math.floor(marker.size[0] / 2);
+ var halfHeight = Math.floor(marker.size[1] / 2);
+ var cellHeight = win.app.calc.cellCursorRectangle.pHeight;
+
+ var localStart = win.cool.SimplePoint.fromCorePixels([halfWidth, halfHeight]);
+ marker.onMouseDown(localStart, new win.MouseEvent('mousedown', { button: 0 }));
+
+ var localEnd = win.cool.SimplePoint.fromCorePixels([halfWidth, halfHeight + cellHeight * 2]);
+ marker.onMouseMove(localEnd, [0, cellHeight * 2], new win.MouseEvent('mousemove'));
+ marker.onMouseUp(localEnd, new win.MouseEvent('mouseup', { button: 0 }));
+ });
+
+ cy.getFrameWindow().then(function(win) {
+ helper.processToIdle(win);
+ });
+
+ helper.typeIntoInputField(helper.addressInputSelector, 'A2');
+ cy.cGet('#sc_input_window.formulabar .ui-custom-textarea-text-layer')
+ .should('have.text', expectedText);
+
+ helper.typeIntoInputField(helper.addressInputSelector, 'A3');
+ cy.cGet('#sc_input_window.formulabar .ui-custom-textarea-text-layer')
+ .should('have.text', expectedText);
+ });
+});
cypress_test/integration_tests/mobile/calc/autofill_marker_spec.js | 57 ++++++++++
2 files changed, 71 insertions(+)
New commits:
commit 1f134fe3134baba4cdcf823bfbdb1582d0032ae1
Author: Gökay Şatır <gokay...@collabora.com>
AuthorDate: Thu May 7 17:17:55 2026 +0300
Commit: Caolán McNamara <caolan....@collabora.com>
CommitDate: Mon May 18 10:54:52 2026 +0000
Calc: Fix autofillmarker behaviour on mobile.
Issue: Autofill marker doesn't work on mobile view.
Cause: Since the position of marker is different than the desktop view, we need to alter it before sending it core side.
Fix: Alter the position before sending it to core side.
A test is also added and I confirmed that the new test doesn't pass without the fix.
Signed-off-by: Gökay Şatır <gokay...@collabora.com>
Change-Id: I6bb85a47064c46dcf9651ea50e787c85c69c3322
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/2138
Tested-by: Caolán McNamara <caolan....@collabora.com>
Reviewed-by: Caolán McNamara <caolan....@collabora.com>
Tested-by: Jenkins CPCI <rel...@collaboraoffice.com>
diff --git a/browser/src/canvas/sections/AutoFillBaseSection.ts b/browser/src/canvas/sections/AutoFillBaseSection.ts
index 6b3b8c27ae9d..9dd24c7caf88 100644
--- a/browser/src/canvas/sections/AutoFillBaseSection.ts
+++ b/browser/src/canvas/sections/AutoFillBaseSection.ts
@@ -210,6 +210,16 @@ class AutoFillBaseSection extends CanvasSectionObject {
return p2;
}
+ // On mobile, setMarkerPosition shifts the marker left by half the cell
+ // width so it sits visually under the cell. Core's autofill hit-test is at
+ // the cell's bottom-right corner, so undo that shift before posting events.
+ private adjustForMobileCenterOffset(p: cool.SimplePoint): void {
+ if (!(<any>window).mode.isDesktop()) {
+ Util.ensureValue(app.calc.cellCursorRectangle);
+ p.pX += app.calc.cellCursorRectangle.pWidth * 0.5;
+ }
+ }
+
protected autoScroll(point: cool.SimplePoint) {
Util.ensureValue(app.activeDocument);
const viewedRectangle = app.activeDocument.activeLayout.viewedRectangle;
@@ -237,6 +247,7 @@ class AutoFillBaseSection extends CanvasSectionObject {
return; // No dragging or no event handling or auto fill marker is not visible.
const p2 = this.getDocumentPositionFromLocal(point);
+ this.adjustForMobileCenterOffset(p2);
app.map._docLayer._postMouseEvent('move', p2.x, p2.y, 1, 1, 0);
if (
@@ -248,6 +259,7 @@ class AutoFillBaseSection extends CanvasSectionObject {
public onMouseUp(point: cool.SimplePoint, e: MouseEvent) {
const p2 = this.getDocumentPositionFromLocal(point);
+ this.adjustForMobileCenterOffset(p2);
app.map._docLayer._postMouseEvent('buttonup', p2.x, p2.y, 1, 1, 0);
}
@@ -255,6 +267,7 @@ class AutoFillBaseSection extends CanvasSectionObject {
// revert coordinates to global and fire event again with position in the center
// inverse of convertPositionToCanvasLocale
const p2 = this.getCenterRegardingDocument();
+ this.adjustForMobileCenterOffset(p2);
app.map._docLayer._postMouseEvent('buttondown', p2.x, p2.y, 1, 1, 0);
}
@@ -269,6 +282,7 @@ class AutoFillBaseSection extends CanvasSectionObject {
public onDoubleClick(point: cool.SimplePoint, e: MouseEvent) {
const pos = this.getCenterRegardingDocument();
+ this.adjustForMobileCenterOffset(pos);
this.sectionProperties.docLayer._postMouseEvent(
'buttondown',
pos.x,
diff --git a/cypress_test/integration_tests/mobile/calc/autofill_marker_spec.js b/cypress_test/integration_tests/mobile/calc/autofill_marker_spec.js
new file mode 100644
index 000000000000..18191871b544
--- /dev/null
+++ b/cypress_test/integration_tests/mobile/calc/autofill_marker_spec.js
@@ -0,0 +1,57 @@
+/* global describe it cy beforeEach require expect */
+
+var helper = require('../../common/helper');
+var calcHelper = require('../../common/calc_helper');
+var mobileHelper = require('../../common/mobile_helper');
+
+describe(['tagmobile', 'tagnextcloud'], 'Calc autofill marker on mobile.', function() {
+
+ beforeEach(function() {
+ helper.setupAndLoadDocument('calc/formulabar.ods');
+ mobileHelper.enableEditingMobile();
+ });
+
+ // On mobile the autofill marker is rendered at the bottom-centre of the
+ // selected cell instead of the bottom-right corner like on desktop. The
+ // mouse events posted to core must still be at the bottom-right corner
+ // where core's autofill hit-test lives, otherwise the drag has no effect
+ // and no cells get filled.
+ it('Drag autofill marker fills cells below the source', function() {
+ var expectedText = 'long line long line long line';
+
+ calcHelper.clickOnFirstCell();
+ cy.cGet('[id="test-div-auto fill marker"]').should('exist');
+
+ // Cypress synthetic DOM events are guarded against by the canvas
+ // section container (mouseIsInside, button-state checks). Invoke
+ // the marker's handlers directly so the coordinate computation is
+ // exercised against the live core kit.
+ cy.getFrameWindow().then(function(win) {
+ var marker = win.app.sectionContainer.getSectionWithName('auto fill marker');
+ expect(marker, 'autofill marker section').to.exist;
+
+ var halfWidth = Math.floor(marker.size[0] / 2);
+ var halfHeight = Math.floor(marker.size[1] / 2);
+ var cellHeight = win.app.calc.cellCursorRectangle.pHeight;
+
+ var localStart = win.cool.SimplePoint.fromCorePixels([halfWidth, halfHeight]);
+ marker.onMouseDown(localStart, new win.MouseEvent('mousedown', { button: 0 }));
+
+ var localEnd = win.cool.SimplePoint.fromCorePixels([halfWidth, halfHeight + cellHeight * 2]);
+ marker.onMouseMove(localEnd, [0, cellHeight * 2], new win.MouseEvent('mousemove'));
+ marker.onMouseUp(localEnd, new win.MouseEvent('mouseup', { button: 0 }));
+ });
+
+ cy.getFrameWindow().then(function(win) {
+ helper.processToIdle(win);
+ });
+
+ helper.typeIntoInputField(helper.addressInputSelector, 'A2');
+ cy.cGet('#sc_input_window.formulabar .ui-custom-textarea-text-layer')
+ .should('have.text', expectedText);
+
+ helper.typeIntoInputField(helper.addressInputSelector, 'A3');
+ cy.cGet('#sc_input_window.formulabar .ui-custom-textarea-text-layer')
+ .should('have.text', expectedText);
+ });
+});
"Mike Kaganski (via cogerrit)"
May 19, 2026, 2:38:46 AM (7 days ago) May 19
to collaboraon...@googlegroups.com
browser/src/map/handler/Map.WOPI.js | 12 +++
cypress_test/integration_tests/desktop/calc4/threaded_comment_spec.js | 19 ++++++
cypress_test/integration_tests/desktop/draw/annotation_spec.js | 31 ++++++++++
3 files changed, 62 insertions(+)
New commits:
commit 04525a6fc39afbfb71bf470bd00181c771e9d9f9
Author: Mike Kaganski <mike.k...@collabora.com>
AuthorDate: Tue May 19 09:46:32 2026 +0500
Commit: Miklos Vajna <vmi...@collabora.com>
CommitDate: Tue May 19 06:37:55 2026 +0000
Handle no-arg "insert comment" commands in browser
Without that, the command either is a no-op (calc), or adds an empty
comment at 0,0 (draw/pdf).
Signed-off-by: Mike Kaganski <mike.k...@collabora.com>
Change-Id: I266b0c10eb1531742afc09a245add4acddfe81eb
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/2864
index 4ae9276a8c04..b598e3bb5054 100644
--- a/browser/src/map/handler/Map.WOPI.js
+++ b/browser/src/map/handler/Map.WOPI.js
@@ -513,6 +513,18 @@ window.L.Map.WOPI = window.L.Handler.extend({
return;
}
else if (msg.MessageId === 'Send_UNO_Command' && msg.Values && msg.Values.Command) {
+ // Commands to insert comments without args initiate interactive insertion,
+ // showing in-place editor, and in PDF, entering a click-to-place mode.
+ if (!msg.Values.Args) {
+ if (msg.Values.Command === '.uno:InsertAnnotation') {
+ this._map.insertComment();
+ return;
+ }
+ if (msg.Values.Command === '.uno:InsertThreadedComment') {
+ this._map.insertThreadedComment();
+ return;
+ }
+ }
this._map.sendUnoCommand(msg.Values.Command, msg.Values.Args || '');
return;
}
diff --git a/cypress_test/integration_tests/desktop/calc4/threaded_comment_spec.js b/cypress_test/integration_tests/desktop/calc4/threaded_comment_spec.js
index 943adad013b9..2008d5e232ee 100644
--- a/cypress_test/integration_tests/desktop/calc4/threaded_comment_spec.js
+++ b/cypress_test/integration_tests/desktop/calc4/threaded_comment_spec.js
@@ -15,6 +15,25 @@ describe(['tagdesktop'], 'Threaded Comment', function() {
});
});
+ // A WOPI host can request a new threaded comment by posting Send_UNO_Command
+ // with .uno:InsertThreadedComment. That should enter the in-browser handling,
+ // instead of sending the command to engine.
+ it('Send_UNO_Command .uno:InsertThreadedComment opens a new comment editor', function() {
+ cy.getFrameWindow().then(function(win) {
+ const message = {
+ MessageId: 'Send_UNO_Command',
+ Values: { Command: '.uno:InsertThreadedComment' }
+ };
+ win.postMessage(JSON.stringify(message), '*');
+ });
+
+ // Same expectation as with the toolbar button: the new-comment editor
+ // placeholder appears so the user can type before the slot is dispatched.
+ cy.cGet('#comment-container-new').should('exist');
+ cy.cGet('.cool-annotation').last().find('#annotation-modify-textarea-new')
+ .should('exist');
+ });
+
it('Insert, resolve, unresolve, and remove threaded comment', function() {
// Click the "Insert Comment" button on the Insert tab.
cy.cGet('#Insert-tab-label').click();
diff --git a/cypress_test/integration_tests/desktop/draw/annotation_spec.js b/cypress_test/integration_tests/desktop/draw/annotation_spec.js
index 862e3c933002..add8809d8e79 100644
--- a/cypress_test/integration_tests/desktop/draw/annotation_spec.js
+++ b/cypress_test/integration_tests/desktop/draw/annotation_spec.js
@@ -320,6 +320,37 @@ describe(['tagdesktop'], 'PDF Threaded Comments', function() {
});
});
+ // A WOPI host can request a new comment in PDF by posting Send_UNO_Command
+ // with .uno:InsertAnnotation. That should enter the in-browser handling,
+ // instead of sending the command to engine.
+ it('Send_UNO_Command .uno:InsertAnnotation enters click-to-place mode', { 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;
+
+ const message = {
+ MessageId: 'Send_UNO_Command',
+ Values: { Command: '.uno:InsertAnnotation' }
+ };
+ win.postMessage(JSON.stringify(message), '*');
+ });
+
+ 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);
+ });
+ });
+
// 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
cypress_test/integration_tests/desktop/calc4/threaded_comment_spec.js | 19 ++++++
cypress_test/integration_tests/desktop/draw/annotation_spec.js | 31 ++++++++++
3 files changed, 62 insertions(+)
New commits:
commit 04525a6fc39afbfb71bf470bd00181c771e9d9f9
Author: Mike Kaganski <mike.k...@collabora.com>
AuthorDate: Tue May 19 09:46:32 2026 +0500
Commit: Miklos Vajna <vmi...@collabora.com>
CommitDate: Tue May 19 06:37:55 2026 +0000
Handle no-arg "insert comment" commands in browser
Without that, the command either is a no-op (calc), or adds an empty
comment at 0,0 (draw/pdf).
Signed-off-by: Mike Kaganski <mike.k...@collabora.com>
Change-Id: I266b0c10eb1531742afc09a245add4acddfe81eb
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/2864
Tested-by: Jenkins CPCI <rel...@collaboraoffice.com>
Reviewed-by: Miklos Vajna <vmi...@collabora.com>
diff --git a/browser/src/map/handler/Map.WOPI.js b/browser/src/map/handler/Map.WOPI.js
Reviewed-by: Miklos Vajna <vmi...@collabora.com>
index 4ae9276a8c04..b598e3bb5054 100644
--- a/browser/src/map/handler/Map.WOPI.js
+++ b/browser/src/map/handler/Map.WOPI.js
@@ -513,6 +513,18 @@ window.L.Map.WOPI = window.L.Handler.extend({
return;
}
else if (msg.MessageId === 'Send_UNO_Command' && msg.Values && msg.Values.Command) {
+ // Commands to insert comments without args initiate interactive insertion,
+ // showing in-place editor, and in PDF, entering a click-to-place mode.
+ if (!msg.Values.Args) {
+ if (msg.Values.Command === '.uno:InsertAnnotation') {
+ this._map.insertComment();
+ return;
+ }
+ if (msg.Values.Command === '.uno:InsertThreadedComment') {
+ this._map.insertThreadedComment();
+ return;
+ }
+ }
this._map.sendUnoCommand(msg.Values.Command, msg.Values.Args || '');
return;
}
diff --git a/cypress_test/integration_tests/desktop/calc4/threaded_comment_spec.js b/cypress_test/integration_tests/desktop/calc4/threaded_comment_spec.js
index 943adad013b9..2008d5e232ee 100644
--- a/cypress_test/integration_tests/desktop/calc4/threaded_comment_spec.js
+++ b/cypress_test/integration_tests/desktop/calc4/threaded_comment_spec.js
@@ -15,6 +15,25 @@ describe(['tagdesktop'], 'Threaded Comment', function() {
});
});
+ // A WOPI host can request a new threaded comment by posting Send_UNO_Command
+ // with .uno:InsertThreadedComment. That should enter the in-browser handling,
+ // instead of sending the command to engine.
+ it('Send_UNO_Command .uno:InsertThreadedComment opens a new comment editor', function() {
+ cy.getFrameWindow().then(function(win) {
+ const message = {
+ MessageId: 'Send_UNO_Command',
+ Values: { Command: '.uno:InsertThreadedComment' }
+ };
+ win.postMessage(JSON.stringify(message), '*');
+ });
+
+ // Same expectation as with the toolbar button: the new-comment editor
+ // placeholder appears so the user can type before the slot is dispatched.
+ cy.cGet('#comment-container-new').should('exist');
+ cy.cGet('.cool-annotation').last().find('#annotation-modify-textarea-new')
+ .should('exist');
+ });
+
it('Insert, resolve, unresolve, and remove threaded comment', function() {
// Click the "Insert Comment" button on the Insert tab.
cy.cGet('#Insert-tab-label').click();
diff --git a/cypress_test/integration_tests/desktop/draw/annotation_spec.js b/cypress_test/integration_tests/desktop/draw/annotation_spec.js
index 862e3c933002..add8809d8e79 100644
--- a/cypress_test/integration_tests/desktop/draw/annotation_spec.js
+++ b/cypress_test/integration_tests/desktop/draw/annotation_spec.js
@@ -320,6 +320,37 @@ describe(['tagdesktop'], 'PDF Threaded Comments', function() {
});
});
+ // A WOPI host can request a new comment in PDF by posting Send_UNO_Command
+ // with .uno:InsertAnnotation. That should enter the in-browser handling,
+ // instead of sending the command to engine.
+ it('Send_UNO_Command .uno:InsertAnnotation enters click-to-place mode', { 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;
+
+ const message = {
+ MessageId: 'Send_UNO_Command',
+ Values: { Command: '.uno:InsertAnnotation' }
+ };
+ win.postMessage(JSON.stringify(message), '*');
+ });
+
+ 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);
+ });
+ });
+
// 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
"Jaume Pujantell (via cogerrit)"
May 19, 2026, 7:03:02 AM (6 days ago) May 19
to collaboraon...@googlegroups.com
browser/src/canvas/sections/CommentListSection.ts | 17 +++
browser/src/canvas/sections/CommentMarkerSubSection.ts | 3
browser/src/canvas/sections/CommentSection.ts | 50 ++++++----
cypress_test/integration_tests/multiuser/writer/annotation_spec.js | 18 +++
4 files changed, 69 insertions(+), 19 deletions(-)
New commits:
commit 7a94e7f5ba4cae6c46d92180d01f7820d7e5f86a
Author: Jaume Pujantell <jaume.p...@collabora.com>
AuthorDate: Thu Apr 23 10:48:18 2026 +0000
Commit: Miklos Vajna <vmi...@collabora.com>
CommitDate: Tue May 19 11:02:40 2026 +0000
comments: only the author can modify
Before this change, the rule "only the author can modify their comment"
was active only in the readonly-with-comments mode. In normal editing
mode, any user could edit or delete any comment.
Now the comment rules are:
- Modify and promote: only the author of the comment.
- Remove, resolve and unresolve: the author, or anyone who can edit
the document.
The context menu hides the entries that the current user is not allowed
to use.
Signed-off-by: Jaume Pujantell <jaume.p...@collabora.com>
Change-Id: I0c9f6cbf66adcdc56e69d0f57d008fa184f9aa8d
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/1593
Reviewed-by: Miklos Vajna <vmi...@collabora.com>
Tested-by: Jenkins CPCI <rel...@collaboraoffice.com>
diff --git a/browser/src/canvas/sections/CommentListSection.ts b/browser/src/canvas/sections/CommentListSection.ts
index 29ecf02c3b75..161ddd14bbd8 100644
--- a/browser/src/canvas/sections/CommentListSection.ts
+++ b/browser/src/canvas/sections/CommentListSection.ts
@@ -1137,6 +1137,8 @@ export class CommentSection extends CanvasSectionObject {
}
public modify (annotation: any): void {
+ if (!annotation.isAuthor())
+ return;
if (cool.Comment.isAnyEdit()) {
this.navigateAndFocusComment(cool.Comment.isAnyEdit());
return;
@@ -1453,6 +1455,10 @@ export class CommentSection extends CanvasSectionObject {
}
public remove (id: any): void {
+ var removedComment = this.getComment(id);
+ if (removedComment && !removedComment.canRemove())
+ return;
+
const comment = {
Id: {
type: 'string',
@@ -1460,7 +1466,6 @@ export class CommentSection extends CanvasSectionObject {
}
};
- var removedComment = this.getComment(id);
if (removedComment) {
removedComment.sectionProperties.selfRemoved = true;
}
@@ -1482,6 +1487,10 @@ export class CommentSection extends CanvasSectionObject {
}
public removeThread (id: any): void {
+ const rootComment = this.getComment(id);
+ if (rootComment && !rootComment.canRemove())
+ return;
+
const comment = {
Id: {
type: 'string',
@@ -1494,6 +1503,8 @@ export class CommentSection extends CanvasSectionObject {
}
public resolve (annotation: any): void {
+ if (!annotation.canModerate())
+ return;
const comment = {
Id: {
type: 'string',
@@ -1504,6 +1515,8 @@ export class CommentSection extends CanvasSectionObject {
}
public resolveThread (annotation: any): void {
+ if (!annotation.canModerate())
+ return;
const comment = {
Id: {
type: 'string',
@@ -1514,6 +1527,8 @@ export class CommentSection extends CanvasSectionObject {
}
public promote(annotation: any): void {
+ if (!annotation.isAuthor())
+ return;
const comment = {
Id: {
type: 'string',
diff --git a/browser/src/canvas/sections/CommentMarkerSubSection.ts b/browser/src/canvas/sections/CommentMarkerSubSection.ts
index 5bd58a313d35..916c2680b803 100644
--- a/browser/src/canvas/sections/CommentMarkerSubSection.ts
+++ b/browser/src/canvas/sections/CommentMarkerSubSection.ts
@@ -69,6 +69,7 @@ class CommentMarkerSubSection extends HTMLObjectSection {
e: MouseEvent,
): void {
if (this.sectionProperties.parentSection === null) return;
+ if (!this.sectionProperties.parentSection.isAuthor()) return;
if (app.sectionContainer.isDraggingSomething()) {
if (this.sectionProperties.parent === null) return;
@@ -86,6 +87,8 @@ class CommentMarkerSubSection extends HTMLObjectSection {
onDragEnd(): void {
this.sectionProperties.dragStartPosition = null;
+ if (!this.sectionProperties.parentSection.isAuthor()) return;
+
const twips = [
this.position[0] * app.pixelsToTwips,
this.position[1] * app.pixelsToTwips,
diff --git a/browser/src/canvas/sections/CommentSection.ts b/browser/src/canvas/sections/CommentSection.ts
index fb077490f6ed..60e1db5da432 100644
--- a/browser/src/canvas/sections/CommentSection.ts
+++ b/browser/src/canvas/sections/CommentSection.ts
@@ -368,13 +368,15 @@ export class Comment extends CanvasSectionObject {
private createMenu (): void {
var tdMenu = window.L.DomUtil.create('td', 'cool-annotation-menubar', this.sectionProperties.authorRow);
this.sectionProperties.menuBarCell = tdMenu;
- const edit = window.L.DomUtil.create('div', 'cool-annotation-menu-edit', tdMenu);
- edit.id = 'comment-annotation-menu-edit-' + this.sectionProperties.data.id;
- edit.tabIndex = 0;
- edit.onclick = this.onEditComment.bind(this);
- edit.onkeypress = this.editOnKeyPress.bind(this);
- edit.dataset.title = Comment.editCommentLabel;
- edit.setAttribute('aria-label', Comment.editCommentLabel);
+ if (this.isAuthor()) {
+ const edit = window.L.DomUtil.create('div', 'cool-annotation-menu-edit', tdMenu);
+ edit.id = 'comment-annotation-menu-edit-' + this.sectionProperties.data.id;
+ edit.tabIndex = 0;
+ edit.onclick = this.onEditComment.bind(this);
+ edit.onkeypress = this.editOnKeyPress.bind(this);
+ edit.dataset.title = Comment.editCommentLabel;
+ edit.setAttribute('aria-label', Comment.editCommentLabel);
+ }
this.sectionProperties.menu = window.L.DomUtil.create('div', this.sectionProperties.data.trackchange ? 'cool-annotation-menu-redline' : 'cool-annotation-menu', tdMenu);
this.sectionProperties.menu.id = 'comment-annotation-menu-' + this.sectionProperties.data.id;
@@ -1089,6 +1091,18 @@ export class Comment extends CanvasSectionObject {
this.hidden = true;
}
+ public isAuthor(): boolean {
+ return this.map.getViewName(app.map._docLayer._viewId) === this.sectionProperties.data.author;
+ }
+
+ public canRemove(): boolean {
+ return this.isAuthor() || !this.map.isReadOnlyMode();
+ }
+
+ public canModerate(): boolean {
+ return this.isAuthor() || app.isCommentEditingAllowed();
+ }
+
// check if this is "our" autosaved comment
// core is not aware it's autosaved one so use this simplified detection based on content
public isAutoSaved (): boolean {
@@ -1096,8 +1110,7 @@ export class Comment extends CanvasSectionObject {
if (!autoSavedComment)
return false;
- var authorMatch = this.sectionProperties.data.author === this.map.getViewName(app.map._docLayer._viewId);
- return authorMatch;
+ return this.isAuthor();
}
public hide (): void {
@@ -1159,36 +1172,37 @@ export class Comment extends CanvasSectionObject {
if (data.trackchange) {
entries.push({ text: _('Comment'), type: 'action', id: 'modify', pos: String(pos++) });
} else {
- const blockChangeFromDifferentAuthor = this.map.isReadOnlyMode()
- && docLayer._docType === 'text'
- && this.map.getViewName(docLayer._viewId) !== data.author;
+ const isAuthor = this.isAuthor();
+ const canRemove = this.canRemove();
+ const canModerate = this.canModerate();
- if (!blockChangeFromDifferentAuthor)
+ if (isAuthor)
entries.push({ text: _('Modify'), type: 'action', id: 'modify', pos: String(pos++) });
if (docLayer._docType === 'text')
entries.push({ text: _('Reply'), type: 'action', id: 'reply', pos: String(pos++) });
- if (!blockChangeFromDifferentAuthor)
+ if (canRemove)
entries.push({ text: _('Remove'), type: 'action', id: 'remove', pos: String(pos++) });
- if (docLayer._docType === 'text' && this.isRootComment() && !blockChangeFromDifferentAuthor)
+ if (docLayer._docType === 'text' && this.isRootComment() && canRemove)
entries.push({ text: _('Remove Thread'), type: 'action', id: 'removeThread', pos: String(pos++) });
const isNonWriterComponent = ['spreadsheet', 'drawing', 'presentation'].includes(docLayer._docType);
- if (docLayer._docType === 'text' || (isNonWriterComponent && data.threaded))
+ if (canModerate
+ && (docLayer._docType === 'text' || (isNonWriterComponent && data.threaded)))
entries.push({
text: data.resolved === 'false' ? _('Resolve') : _('Unresolve'),
type: 'action', id: 'resolve', pos: String(pos++),
});
- if (docLayer._docType === 'text' && this.isRootComment())
+ if (docLayer._docType === 'text' && this.isRootComment() && canModerate)
entries.push({
text: listSection.isThreadResolved(this) ? _('Unresolve Thread') : _('Resolve Thread'),
type: 'action', id: 'resolveThread', pos: String(pos++),
});
- if (docLayer._docType === 'text' && !this.isRootComment() && !blockChangeFromDifferentAuthor)
+ if (docLayer._docType === 'text' && !this.isRootComment() && isAuthor)
entries.push({ text: _('Promote to top comment'), type: 'action', id: 'promote', pos: String(pos++) });
if (docLayer._docType === 'text' && !window.mode.isSmallScreenDevice()) {
diff --git a/cypress_test/integration_tests/multiuser/writer/annotation_spec.js b/cypress_test/integration_tests/multiuser/writer/annotation_spec.js
index b59b511a9a64..44d5e35e440f 100644
--- a/cypress_test/integration_tests/multiuser/writer/annotation_spec.js
+++ b/cypress_test/integration_tests/multiuser/writer/annotation_spec.js
@@ -80,6 +80,24 @@ describe(['tagmultiuser'], 'Multiuser Annotation Tests', function () {
cy.cSetActiveFrame('#iframe2');
cy.cGet('.cool-annotation-content-wrapper').should('not.exist');
});
+
+ it('Only author sees Modify', function() {
+ cy.cSetActiveFrame('#iframe1');
+ desktopHelper.insertComment();
+ cy.cGet('.cool-annotation-content-wrapper').should('exist');
+ cy.cGet('#comment-annotation-menu-edit-1').should('exist');
+ cy.cGet('#comment-annotation-menu-1').click();
+ cy.cGet('body').contains('.ui-combobox-entry.jsdialog.ui-grid-cell', 'Modify').should('exist');
+ cy.cGet('body').contains('.ui-combobox-entry.jsdialog.ui-grid-cell', 'Reply').should('exist');
+ cy.cGet('body').type('{esc}');
+
+ cy.cSetActiveFrame('#iframe2');
+ cy.cGet('.cool-annotation-content-wrapper').should('exist');
+ cy.cGet('#comment-annotation-menu-edit-1').should('not.exist');
+ cy.cGet('#comment-annotation-menu-1').click();
+ cy.cGet('body').contains('.ui-combobox-entry.jsdialog.ui-grid-cell', 'Modify').should('not.exist');
+ cy.cGet('body').contains('.ui-combobox-entry.jsdialog.ui-grid-cell', 'Reply').should('exist');
+ });
});
describe(['tagmultiuser'], 'Collapsed Annotation Tests', function() {
browser/src/canvas/sections/CommentMarkerSubSection.ts | 3
browser/src/canvas/sections/CommentSection.ts | 50 ++++++----
cypress_test/integration_tests/multiuser/writer/annotation_spec.js | 18 +++
4 files changed, 69 insertions(+), 19 deletions(-)
New commits:
commit 7a94e7f5ba4cae6c46d92180d01f7820d7e5f86a
Author: Jaume Pujantell <jaume.p...@collabora.com>
AuthorDate: Thu Apr 23 10:48:18 2026 +0000
Commit: Miklos Vajna <vmi...@collabora.com>
CommitDate: Tue May 19 11:02:40 2026 +0000
comments: only the author can modify
Before this change, the rule "only the author can modify their comment"
was active only in the readonly-with-comments mode. In normal editing
mode, any user could edit or delete any comment.
Now the comment rules are:
- Modify and promote: only the author of the comment.
- Remove, resolve and unresolve: the author, or anyone who can edit
the document.
The context menu hides the entries that the current user is not allowed
to use.
Signed-off-by: Jaume Pujantell <jaume.p...@collabora.com>
Change-Id: I0c9f6cbf66adcdc56e69d0f57d008fa184f9aa8d
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/1593
Reviewed-by: Miklos Vajna <vmi...@collabora.com>
Tested-by: Jenkins CPCI <rel...@collaboraoffice.com>
diff --git a/browser/src/canvas/sections/CommentListSection.ts b/browser/src/canvas/sections/CommentListSection.ts
index 29ecf02c3b75..161ddd14bbd8 100644
--- a/browser/src/canvas/sections/CommentListSection.ts
+++ b/browser/src/canvas/sections/CommentListSection.ts
@@ -1137,6 +1137,8 @@ export class CommentSection extends CanvasSectionObject {
}
public modify (annotation: any): void {
+ if (!annotation.isAuthor())
+ return;
if (cool.Comment.isAnyEdit()) {
this.navigateAndFocusComment(cool.Comment.isAnyEdit());
return;
@@ -1453,6 +1455,10 @@ export class CommentSection extends CanvasSectionObject {
}
public remove (id: any): void {
+ var removedComment = this.getComment(id);
+ if (removedComment && !removedComment.canRemove())
+ return;
+
const comment = {
Id: {
type: 'string',
@@ -1460,7 +1466,6 @@ export class CommentSection extends CanvasSectionObject {
}
};
- var removedComment = this.getComment(id);
if (removedComment) {
removedComment.sectionProperties.selfRemoved = true;
}
@@ -1482,6 +1487,10 @@ export class CommentSection extends CanvasSectionObject {
}
public removeThread (id: any): void {
+ const rootComment = this.getComment(id);
+ if (rootComment && !rootComment.canRemove())
+ return;
+
const comment = {
Id: {
type: 'string',
@@ -1494,6 +1503,8 @@ export class CommentSection extends CanvasSectionObject {
}
public resolve (annotation: any): void {
+ if (!annotation.canModerate())
+ return;
const comment = {
Id: {
type: 'string',
@@ -1504,6 +1515,8 @@ export class CommentSection extends CanvasSectionObject {
}
public resolveThread (annotation: any): void {
+ if (!annotation.canModerate())
+ return;
const comment = {
Id: {
type: 'string',
@@ -1514,6 +1527,8 @@ export class CommentSection extends CanvasSectionObject {
}
public promote(annotation: any): void {
+ if (!annotation.isAuthor())
+ return;
const comment = {
Id: {
type: 'string',
diff --git a/browser/src/canvas/sections/CommentMarkerSubSection.ts b/browser/src/canvas/sections/CommentMarkerSubSection.ts
index 5bd58a313d35..916c2680b803 100644
--- a/browser/src/canvas/sections/CommentMarkerSubSection.ts
+++ b/browser/src/canvas/sections/CommentMarkerSubSection.ts
@@ -69,6 +69,7 @@ class CommentMarkerSubSection extends HTMLObjectSection {
e: MouseEvent,
): void {
if (this.sectionProperties.parentSection === null) return;
+ if (!this.sectionProperties.parentSection.isAuthor()) return;
if (app.sectionContainer.isDraggingSomething()) {
if (this.sectionProperties.parent === null) return;
@@ -86,6 +87,8 @@ class CommentMarkerSubSection extends HTMLObjectSection {
onDragEnd(): void {
this.sectionProperties.dragStartPosition = null;
+ if (!this.sectionProperties.parentSection.isAuthor()) return;
+
const twips = [
this.position[0] * app.pixelsToTwips,
this.position[1] * app.pixelsToTwips,
diff --git a/browser/src/canvas/sections/CommentSection.ts b/browser/src/canvas/sections/CommentSection.ts
index fb077490f6ed..60e1db5da432 100644
--- a/browser/src/canvas/sections/CommentSection.ts
+++ b/browser/src/canvas/sections/CommentSection.ts
@@ -368,13 +368,15 @@ export class Comment extends CanvasSectionObject {
private createMenu (): void {
var tdMenu = window.L.DomUtil.create('td', 'cool-annotation-menubar', this.sectionProperties.authorRow);
this.sectionProperties.menuBarCell = tdMenu;
- const edit = window.L.DomUtil.create('div', 'cool-annotation-menu-edit', tdMenu);
- edit.id = 'comment-annotation-menu-edit-' + this.sectionProperties.data.id;
- edit.tabIndex = 0;
- edit.onclick = this.onEditComment.bind(this);
- edit.onkeypress = this.editOnKeyPress.bind(this);
- edit.dataset.title = Comment.editCommentLabel;
- edit.setAttribute('aria-label', Comment.editCommentLabel);
+ if (this.isAuthor()) {
+ const edit = window.L.DomUtil.create('div', 'cool-annotation-menu-edit', tdMenu);
+ edit.id = 'comment-annotation-menu-edit-' + this.sectionProperties.data.id;
+ edit.tabIndex = 0;
+ edit.onclick = this.onEditComment.bind(this);
+ edit.onkeypress = this.editOnKeyPress.bind(this);
+ edit.dataset.title = Comment.editCommentLabel;
+ edit.setAttribute('aria-label', Comment.editCommentLabel);
+ }
this.sectionProperties.menu = window.L.DomUtil.create('div', this.sectionProperties.data.trackchange ? 'cool-annotation-menu-redline' : 'cool-annotation-menu', tdMenu);
this.sectionProperties.menu.id = 'comment-annotation-menu-' + this.sectionProperties.data.id;
@@ -1089,6 +1091,18 @@ export class Comment extends CanvasSectionObject {
this.hidden = true;
}
+ public isAuthor(): boolean {
+ return this.map.getViewName(app.map._docLayer._viewId) === this.sectionProperties.data.author;
+ }
+
+ public canRemove(): boolean {
+ return this.isAuthor() || !this.map.isReadOnlyMode();
+ }
+
+ public canModerate(): boolean {
+ return this.isAuthor() || app.isCommentEditingAllowed();
+ }
+
// check if this is "our" autosaved comment
// core is not aware it's autosaved one so use this simplified detection based on content
public isAutoSaved (): boolean {
@@ -1096,8 +1110,7 @@ export class Comment extends CanvasSectionObject {
if (!autoSavedComment)
return false;
- var authorMatch = this.sectionProperties.data.author === this.map.getViewName(app.map._docLayer._viewId);
- return authorMatch;
+ return this.isAuthor();
}
public hide (): void {
@@ -1159,36 +1172,37 @@ export class Comment extends CanvasSectionObject {
if (data.trackchange) {
entries.push({ text: _('Comment'), type: 'action', id: 'modify', pos: String(pos++) });
} else {
- const blockChangeFromDifferentAuthor = this.map.isReadOnlyMode()
- && docLayer._docType === 'text'
- && this.map.getViewName(docLayer._viewId) !== data.author;
+ const isAuthor = this.isAuthor();
+ const canRemove = this.canRemove();
+ const canModerate = this.canModerate();
- if (!blockChangeFromDifferentAuthor)
+ if (isAuthor)
entries.push({ text: _('Modify'), type: 'action', id: 'modify', pos: String(pos++) });
if (docLayer._docType === 'text')
entries.push({ text: _('Reply'), type: 'action', id: 'reply', pos: String(pos++) });
- if (!blockChangeFromDifferentAuthor)
+ if (canRemove)
entries.push({ text: _('Remove'), type: 'action', id: 'remove', pos: String(pos++) });
- if (docLayer._docType === 'text' && this.isRootComment() && !blockChangeFromDifferentAuthor)
+ if (docLayer._docType === 'text' && this.isRootComment() && canRemove)
entries.push({ text: _('Remove Thread'), type: 'action', id: 'removeThread', pos: String(pos++) });
const isNonWriterComponent = ['spreadsheet', 'drawing', 'presentation'].includes(docLayer._docType);
- if (docLayer._docType === 'text' || (isNonWriterComponent && data.threaded))
+ if (canModerate
+ && (docLayer._docType === 'text' || (isNonWriterComponent && data.threaded)))
entries.push({
text: data.resolved === 'false' ? _('Resolve') : _('Unresolve'),
type: 'action', id: 'resolve', pos: String(pos++),
});
- if (docLayer._docType === 'text' && this.isRootComment())
+ if (docLayer._docType === 'text' && this.isRootComment() && canModerate)
entries.push({
text: listSection.isThreadResolved(this) ? _('Unresolve Thread') : _('Resolve Thread'),
type: 'action', id: 'resolveThread', pos: String(pos++),
});
- if (docLayer._docType === 'text' && !this.isRootComment() && !blockChangeFromDifferentAuthor)
+ if (docLayer._docType === 'text' && !this.isRootComment() && isAuthor)
entries.push({ text: _('Promote to top comment'), type: 'action', id: 'promote', pos: String(pos++) });
if (docLayer._docType === 'text' && !window.mode.isSmallScreenDevice()) {
diff --git a/cypress_test/integration_tests/multiuser/writer/annotation_spec.js b/cypress_test/integration_tests/multiuser/writer/annotation_spec.js
index b59b511a9a64..44d5e35e440f 100644
--- a/cypress_test/integration_tests/multiuser/writer/annotation_spec.js
+++ b/cypress_test/integration_tests/multiuser/writer/annotation_spec.js
@@ -80,6 +80,24 @@ describe(['tagmultiuser'], 'Multiuser Annotation Tests', function () {
cy.cSetActiveFrame('#iframe2');
cy.cGet('.cool-annotation-content-wrapper').should('not.exist');
});
+
+ it('Only author sees Modify', function() {
+ cy.cSetActiveFrame('#iframe1');
+ desktopHelper.insertComment();
+ cy.cGet('.cool-annotation-content-wrapper').should('exist');
+ cy.cGet('#comment-annotation-menu-edit-1').should('exist');
+ cy.cGet('#comment-annotation-menu-1').click();
+ cy.cGet('body').contains('.ui-combobox-entry.jsdialog.ui-grid-cell', 'Modify').should('exist');
+ cy.cGet('body').contains('.ui-combobox-entry.jsdialog.ui-grid-cell', 'Reply').should('exist');
+ cy.cGet('body').type('{esc}');
+
+ cy.cSetActiveFrame('#iframe2');
+ cy.cGet('.cool-annotation-content-wrapper').should('exist');
+ cy.cGet('#comment-annotation-menu-edit-1').should('not.exist');
+ cy.cGet('#comment-annotation-menu-1').click();
+ cy.cGet('body').contains('.ui-combobox-entry.jsdialog.ui-grid-cell', 'Modify').should('not.exist');
+ cy.cGet('body').contains('.ui-combobox-entry.jsdialog.ui-grid-cell', 'Reply').should('exist');
+ });
});
describe(['tagmultiuser'], 'Collapsed Annotation Tests', function() {
"Bayram Çiçek (via cogerrit)"
May 20, 2026, 4:21:51 PM (5 days ago) May 20
to collaboraon...@googlegroups.com
browser/src/control/Control.JSDialogBuilder.js | 6 -
cypress_test/integration_tests/desktop/calc1/a11y_notebookbar_spec.js | 58 ++++++++++
2 files changed, 62 insertions(+), 2 deletions(-)
New commits:
commit ff33648a5cb50b045f306e632e7c919661eb61e2
Author: Bayram Çiçek <bayram...@collabora.com>
AuthorDate: Sun May 17 14:05:48 2026 +0300
Commit: Caolán McNamara <caolan....@collabora.com>
CommitDate: Wed May 20 20:21:32 2026 +0000
Calc a11y: keep focus on the current toolbar button when ArrowDown has no target
- The notebookbar keydown handler had a Tab wrap-around fallback that arrow keys were also falling into. Its [tabindex="-1"] selector matches the unotoolbutton wrapper divs, not the inner <button>s, so ArrowDown on "Position and Size" in the Shape tab moved focus to a wrapper div with no accessible name. The focus ring disappeared, the screen reader fell silent, and subsequent arrow keys did nothing.
- Restrict the fallback to isTab so only Tab and Shift+Tab still wrap, while arrow keys with no target leave focus on the current button.
- Add a cypress test.
Signed-off-by: Bayram Çiçek <bayram...@collabora.com>
Change-Id: I07b49512f04ab6a8898b723c53e713fba11ade4b
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/2845
Tested-by: Jenkins CPCI <rel...@collaboraoffice.com>
Reviewed-by: Caolán McNamara <caolan....@collabora.com>
diff --git a/browser/src/control/Control.JSDialogBuilder.js b/browser/src/control/Control.JSDialogBuilder.js
index c5c701f5ddd9..94e9b3760584 100644
--- a/browser/src/control/Control.JSDialogBuilder.js
+++ b/browser/src/control/Control.JSDialogBuilder.js
@@ -1001,8 +1001,10 @@ window.L.Control.JSDialogBuilder = window.L.Control.extend({
elementToFocus.focus();
else if (elementToFocus)
document.querySelector('.ui-tab.notebookbar.selected').focus();
- else {
- // Nothing found — cycle to first focusable
+ else if (isTab) {
+ // Tab wrap-around only. If the user actually pressed Tab,
+ // wrap to first/last. For arrow keys, leave focus alone so
+ // it stays on the current button when nothing is found.
let visibleContainer = Array.from(container[0].children).find(child =>
!child.classList.contains('hidden') && child.offsetParent !== null
);
diff --git a/cypress_test/integration_tests/desktop/calc1/a11y_notebookbar_spec.js b/cypress_test/integration_tests/desktop/calc1/a11y_notebookbar_spec.js
index b668395f49c1..cfd0b1ccbd73 100644
--- a/cypress_test/integration_tests/desktop/calc1/a11y_notebookbar_spec.js
+++ b/cypress_test/integration_tests/desktop/calc1/a11y_notebookbar_spec.js
@@ -70,6 +70,64 @@ describe(['tagdesktop'], 'Accessibility Calc Notebookbar Tests', { testIsolation
helper.typeIntoDocument('{esc}');
});
+ it('Shape tab: ArrowDown on first toolbar button keeps focus', function () {
+ cy.then(function () {
+ win.app.map.sendUnoCommand('.uno:BasicShapes.octagon');
+ });
+
+ cy.cGet('#test-div-shapeHandlesSection').should('exist');
+
+ cy.cGet('#Shape-tab-label').should('be.visible').click();
+ cy.cGet('#Shape-tab-label').should('have.class', 'selected');
+ cy.then(function () { return helper.processToIdle(win); });
+
+ // Extend notebookbar, collapsed mode hides all tab pages.
+ cy.then(function () {
+ if (win.app.map.uiManager.isNotebookbarCollapsed())
+ win.app.map.uiManager.extendNotebookbar();
+ });
+ cy.then(function () { return helper.processToIdle(win); });
+
+ cy.cGet('[modelid="shape-transform-dialog"] > button')
+ .should('not.be.disabled');
+
+ // Listen for focus changes during dispatch; the clipboard-area
+ // steals focus back instantly and hides any wrong .focus() call.
+ cy.then(function () {
+ var positionAndSizeBtn = win.document.querySelector(
+ '[modelid="shape-transform-dialog"] > button');
+ expect(positionAndSizeBtn,
+ 'Position and Size button must exist').to.not.be.null;
+
+ var stolenFocusOnto = [];
+ var listener = function (e) {
+ if (e.target.classList &&
+ e.target.classList.contains('unotoolbutton')) {
+ stolenFocusOnto.push(e.target.id || '<no id>');
+ }
+ };
+ win.document.addEventListener('focusin', listener, true);
+
+ var event = new win.KeyboardEvent('keydown', {
+ key: 'ArrowDown',
+ bubbles: true,
+ cancelable: true,
+ });
+ positionAndSizeBtn.dispatchEvent(event);
+
+ win.document.removeEventListener('focusin', listener, true);
+
+ // Regression: ArrowDown used to focus a wrapper div.
+ expect(stolenFocusOnto,
+ 'ArrowDown must not focus a unotoolbutton wrapper, saw: ' +
+ JSON.stringify(stolenFocusOnto))
+ .to.be.empty;
+ });
+
+ // exit shape mode
+ helper.typeIntoDocument('{esc}');
+ });
+
it('Notebookbar tab: Picture (context)', function () {
cy.viewport(1920, 1080);
cypress_test/integration_tests/desktop/calc1/a11y_notebookbar_spec.js | 58 ++++++++++
2 files changed, 62 insertions(+), 2 deletions(-)
New commits:
commit ff33648a5cb50b045f306e632e7c919661eb61e2
Author: Bayram Çiçek <bayram...@collabora.com>
AuthorDate: Sun May 17 14:05:48 2026 +0300
Commit: Caolán McNamara <caolan....@collabora.com>
CommitDate: Wed May 20 20:21:32 2026 +0000
Calc a11y: keep focus on the current toolbar button when ArrowDown has no target
- The notebookbar keydown handler had a Tab wrap-around fallback that arrow keys were also falling into. Its [tabindex="-1"] selector matches the unotoolbutton wrapper divs, not the inner <button>s, so ArrowDown on "Position and Size" in the Shape tab moved focus to a wrapper div with no accessible name. The focus ring disappeared, the screen reader fell silent, and subsequent arrow keys did nothing.
- Restrict the fallback to isTab so only Tab and Shift+Tab still wrap, while arrow keys with no target leave focus on the current button.
- Add a cypress test.
Signed-off-by: Bayram Çiçek <bayram...@collabora.com>
Change-Id: I07b49512f04ab6a8898b723c53e713fba11ade4b
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/2845
Tested-by: Jenkins CPCI <rel...@collaboraoffice.com>
Reviewed-by: Caolán McNamara <caolan....@collabora.com>
diff --git a/browser/src/control/Control.JSDialogBuilder.js b/browser/src/control/Control.JSDialogBuilder.js
index c5c701f5ddd9..94e9b3760584 100644
--- a/browser/src/control/Control.JSDialogBuilder.js
+++ b/browser/src/control/Control.JSDialogBuilder.js
@@ -1001,8 +1001,10 @@ window.L.Control.JSDialogBuilder = window.L.Control.extend({
elementToFocus.focus();
else if (elementToFocus)
document.querySelector('.ui-tab.notebookbar.selected').focus();
- else {
- // Nothing found — cycle to first focusable
+ else if (isTab) {
+ // Tab wrap-around only. If the user actually pressed Tab,
+ // wrap to first/last. For arrow keys, leave focus alone so
+ // it stays on the current button when nothing is found.
let visibleContainer = Array.from(container[0].children).find(child =>
!child.classList.contains('hidden') && child.offsetParent !== null
);
diff --git a/cypress_test/integration_tests/desktop/calc1/a11y_notebookbar_spec.js b/cypress_test/integration_tests/desktop/calc1/a11y_notebookbar_spec.js
index b668395f49c1..cfd0b1ccbd73 100644
--- a/cypress_test/integration_tests/desktop/calc1/a11y_notebookbar_spec.js
+++ b/cypress_test/integration_tests/desktop/calc1/a11y_notebookbar_spec.js
@@ -70,6 +70,64 @@ describe(['tagdesktop'], 'Accessibility Calc Notebookbar Tests', { testIsolation
helper.typeIntoDocument('{esc}');
});
+ it('Shape tab: ArrowDown on first toolbar button keeps focus', function () {
+ cy.then(function () {
+ win.app.map.sendUnoCommand('.uno:BasicShapes.octagon');
+ });
+
+ cy.cGet('#test-div-shapeHandlesSection').should('exist');
+
+ cy.cGet('#Shape-tab-label').should('be.visible').click();
+ cy.cGet('#Shape-tab-label').should('have.class', 'selected');
+ cy.then(function () { return helper.processToIdle(win); });
+
+ // Extend notebookbar, collapsed mode hides all tab pages.
+ cy.then(function () {
+ if (win.app.map.uiManager.isNotebookbarCollapsed())
+ win.app.map.uiManager.extendNotebookbar();
+ });
+ cy.then(function () { return helper.processToIdle(win); });
+
+ cy.cGet('[modelid="shape-transform-dialog"] > button')
+ .should('not.be.disabled');
+
+ // Listen for focus changes during dispatch; the clipboard-area
+ // steals focus back instantly and hides any wrong .focus() call.
+ cy.then(function () {
+ var positionAndSizeBtn = win.document.querySelector(
+ '[modelid="shape-transform-dialog"] > button');
+ expect(positionAndSizeBtn,
+ 'Position and Size button must exist').to.not.be.null;
+
+ var stolenFocusOnto = [];
+ var listener = function (e) {
+ if (e.target.classList &&
+ e.target.classList.contains('unotoolbutton')) {
+ stolenFocusOnto.push(e.target.id || '<no id>');
+ }
+ };
+ win.document.addEventListener('focusin', listener, true);
+
+ var event = new win.KeyboardEvent('keydown', {
+ key: 'ArrowDown',
+ bubbles: true,
+ cancelable: true,
+ });
+ positionAndSizeBtn.dispatchEvent(event);
+
+ win.document.removeEventListener('focusin', listener, true);
+
+ // Regression: ArrowDown used to focus a wrapper div.
+ expect(stolenFocusOnto,
+ 'ArrowDown must not focus a unotoolbutton wrapper, saw: ' +
+ JSON.stringify(stolenFocusOnto))
+ .to.be.empty;
+ });
+
+ // exit shape mode
+ helper.typeIntoDocument('{esc}');
+ });
+
it('Notebookbar tab: Picture (context)', function () {
cy.viewport(1920, 1080);
"Szymon Kłos (via cogerrit)"
May 21, 2026, 2:01:56 AM (5 days ago) May 21
to collaboraon...@googlegroups.com
browser/src/control/Control.MobileWizardWindow.js | 4 ++++
cypress_test/integration_tests/mobile/impress/apply_paragraph_props_shape_spec.js | 4 ++--
cypress_test/integration_tests/mobile/impress/apply_paragraph_props_text_spec.js | 4 ++--
3 files changed, 8 insertions(+), 4 deletions(-)
New commits:
commit 46d8e50171249340154ed3d949a9b53dbbdea2a8
Author: Szymon Kłos <szymo...@collabora.com>
AuthorDate: Mon May 18 06:32:26 2026 +0000
Commit: Szymon Kłos <szymo...@collabora.com>
CommitDate: Thu May 21 06:01:40 2026 +0000
jsdialog: remove hidden class on level-down
Sidebar panels arrive with the inner content carrying the "hidden"
class (visible: false from the server). jQuery .show() can't override
that because .hidden has display: none !important, so the panel stayed
collapsed after clicking its header.
Strip the class before calling .show() in goLevelDown.
Also scope the bulleting / paragraph property panel selectors in the
mobile cypress tests to the panel that was just opened. The previous
selector matched both the open panel and the (hidden) toolbar-down
copy, and chai-jquery's :visible requires every match to be visible.
Signed-off-by: Szymon Kłos <szymo...@collabora.com>
Change-Id: I91212bdf4aa449d562218a7af09457bf079f19c4
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/2929
Tested-by: Jenkins CPCI <rel...@collaboraoffice.com>
Reviewed-by: Mohit Marathe <mohit....@collabora.com>
diff --git a/browser/src/control/Control.MobileWizardWindow.js b/browser/src/control/Control.MobileWizardWindow.js
index d11a4f7f02f9..d55ab98e00ef 100644
--- a/browser/src/control/Control.MobileWizardWindow.js
+++ b/browser/src/control/Control.MobileWizardWindow.js
@@ -263,6 +263,10 @@ window.L.Control.MobileWizardWindow = window.L.Control.extend({
$('#mobile-wizard.funcwizard div#mobile-wizard-content').removeClass('hideHelpBG');
$('#mobile-wizard.funcwizard div#mobile-wizard-content').addClass('showHelpBG');
+ // The content panel may carry the "hidden" class from the server
+ // state (visible: false). Removing it is required for .show() to
+ // take effect, because .hidden has display:none !important.
+ $(contentToShow).children('.ui-content').first().removeClass('hidden');
if (animate)
$(contentToShow).children('.ui-content').first().show('slide', { direction: 'right' }, 'fast');
else
diff --git a/cypress_test/integration_tests/mobile/impress/apply_paragraph_props_shape_spec.js b/cypress_test/integration_tests/mobile/impress/apply_paragraph_props_shape_spec.js
index 236816cf0cef..52bd8db0067f 100644
--- a/cypress_test/integration_tests/mobile/impress/apply_paragraph_props_shape_spec.js
+++ b/cypress_test/integration_tests/mobile/impress/apply_paragraph_props_shape_spec.js
@@ -29,7 +29,7 @@ describe(['tagmobile', 'tagnextcloud', 'tagproxy'], 'Apply paragraph properties
cy.cGet('#ParaPropertyPanel').click();
- cy.cGet('.unoParaLeftToRight').should('be.visible');
+ cy.cGet('#ParaPropertyPanel .unoParaLeftToRight').should('be.visible');
}
function openListsPropertiesPanel() {
@@ -37,7 +37,7 @@ describe(['tagmobile', 'tagnextcloud', 'tagproxy'], 'Apply paragraph properties
cy.cGet('#ListsPropertyPanel').click();
- cy.cGet('.unoDefaultBullet').should('be.visible');
+ cy.cGet('#ListsPropertyPanel .unoDefaultBullet').should('be.visible');
}
it.skip('Apply left/right alignment on text shape.', function() {
diff --git a/cypress_test/integration_tests/mobile/impress/apply_paragraph_props_text_spec.js b/cypress_test/integration_tests/mobile/impress/apply_paragraph_props_text_spec.js
index 87cd71c82364..9836680855a0 100644
--- a/cypress_test/integration_tests/mobile/impress/apply_paragraph_props_text_spec.js
+++ b/cypress_test/integration_tests/mobile/impress/apply_paragraph_props_text_spec.js
@@ -34,13 +34,13 @@ describe(['tagmobile', 'tagnextcloud', 'tagproxy'], 'Apply paragraph properties
function openParagraphPropertiesPanel() {
mobileHelper.openMobileWizard();
cy.cGet('#ParaPropertyPanel').click();
- cy.cGet('.unoParaLeftToRight').should('be.visible');
+ cy.cGet('#ParaPropertyPanel .unoParaLeftToRight').should('be.visible');
}
function openListsPropertiesPanel() {
mobileHelper.openMobileWizard();
cy.cGet('#ListsPropertyPanel').click();
- cy.cGet('.unoDefaultBullet').should('be.visible');
+ cy.cGet('#ListsPropertyPanel .unoDefaultBullet').should('be.visible');
}
it('Apply horizontal alignment on selected text.', function() {
cypress_test/integration_tests/mobile/impress/apply_paragraph_props_shape_spec.js | 4 ++--
cypress_test/integration_tests/mobile/impress/apply_paragraph_props_text_spec.js | 4 ++--
3 files changed, 8 insertions(+), 4 deletions(-)
New commits:
commit 46d8e50171249340154ed3d949a9b53dbbdea2a8
Author: Szymon Kłos <szymo...@collabora.com>
AuthorDate: Mon May 18 06:32:26 2026 +0000
Commit: Szymon Kłos <szymo...@collabora.com>
CommitDate: Thu May 21 06:01:40 2026 +0000
jsdialog: remove hidden class on level-down
Sidebar panels arrive with the inner content carrying the "hidden"
class (visible: false from the server). jQuery .show() can't override
that because .hidden has display: none !important, so the panel stayed
collapsed after clicking its header.
Strip the class before calling .show() in goLevelDown.
Also scope the bulleting / paragraph property panel selectors in the
mobile cypress tests to the panel that was just opened. The previous
selector matched both the open panel and the (hidden) toolbar-down
copy, and chai-jquery's :visible requires every match to be visible.
Signed-off-by: Szymon Kłos <szymo...@collabora.com>
Change-Id: I91212bdf4aa449d562218a7af09457bf079f19c4
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/2929
Tested-by: Jenkins CPCI <rel...@collaboraoffice.com>
Reviewed-by: Mohit Marathe <mohit....@collabora.com>
diff --git a/browser/src/control/Control.MobileWizardWindow.js b/browser/src/control/Control.MobileWizardWindow.js
index d11a4f7f02f9..d55ab98e00ef 100644
--- a/browser/src/control/Control.MobileWizardWindow.js
+++ b/browser/src/control/Control.MobileWizardWindow.js
@@ -263,6 +263,10 @@ window.L.Control.MobileWizardWindow = window.L.Control.extend({
$('#mobile-wizard.funcwizard div#mobile-wizard-content').removeClass('hideHelpBG');
$('#mobile-wizard.funcwizard div#mobile-wizard-content').addClass('showHelpBG');
+ // The content panel may carry the "hidden" class from the server
+ // state (visible: false). Removing it is required for .show() to
+ // take effect, because .hidden has display:none !important.
+ $(contentToShow).children('.ui-content').first().removeClass('hidden');
if (animate)
$(contentToShow).children('.ui-content').first().show('slide', { direction: 'right' }, 'fast');
else
diff --git a/cypress_test/integration_tests/mobile/impress/apply_paragraph_props_shape_spec.js b/cypress_test/integration_tests/mobile/impress/apply_paragraph_props_shape_spec.js
index 236816cf0cef..52bd8db0067f 100644
--- a/cypress_test/integration_tests/mobile/impress/apply_paragraph_props_shape_spec.js
+++ b/cypress_test/integration_tests/mobile/impress/apply_paragraph_props_shape_spec.js
@@ -29,7 +29,7 @@ describe(['tagmobile', 'tagnextcloud', 'tagproxy'], 'Apply paragraph properties
cy.cGet('#ParaPropertyPanel').click();
- cy.cGet('.unoParaLeftToRight').should('be.visible');
+ cy.cGet('#ParaPropertyPanel .unoParaLeftToRight').should('be.visible');
}
function openListsPropertiesPanel() {
@@ -37,7 +37,7 @@ describe(['tagmobile', 'tagnextcloud', 'tagproxy'], 'Apply paragraph properties
cy.cGet('#ListsPropertyPanel').click();
- cy.cGet('.unoDefaultBullet').should('be.visible');
+ cy.cGet('#ListsPropertyPanel .unoDefaultBullet').should('be.visible');
}
it.skip('Apply left/right alignment on text shape.', function() {
diff --git a/cypress_test/integration_tests/mobile/impress/apply_paragraph_props_text_spec.js b/cypress_test/integration_tests/mobile/impress/apply_paragraph_props_text_spec.js
index 87cd71c82364..9836680855a0 100644
--- a/cypress_test/integration_tests/mobile/impress/apply_paragraph_props_text_spec.js
+++ b/cypress_test/integration_tests/mobile/impress/apply_paragraph_props_text_spec.js
@@ -34,13 +34,13 @@ describe(['tagmobile', 'tagnextcloud', 'tagproxy'], 'Apply paragraph properties
function openParagraphPropertiesPanel() {
mobileHelper.openMobileWizard();
cy.cGet('#ParaPropertyPanel').click();
- cy.cGet('.unoParaLeftToRight').should('be.visible');
+ cy.cGet('#ParaPropertyPanel .unoParaLeftToRight').should('be.visible');
}
function openListsPropertiesPanel() {
mobileHelper.openMobileWizard();
cy.cGet('#ListsPropertyPanel').click();
- cy.cGet('.unoDefaultBullet').should('be.visible');
+ cy.cGet('#ListsPropertyPanel .unoDefaultBullet').should('be.visible');
}
it('Apply horizontal alignment on selected text.', function() {
"Jaume Pujantell (via cogerrit)"
May 21, 2026, 4:36:43 AM (5 days ago) May 21
to collaboraon...@googlegroups.com
browser/src/layer/tile/WriterTileLayer.js | 27 ++++++----
cypress_test/integration_tests/desktop/writer3/reconnect_spec.js | 10 +--
2 files changed, 22 insertions(+), 15 deletions(-)
New commits:
commit fcf291766d6c089f3f03a3c72feb2a5c135403ef
Author: Jaume Pujantell <jaume.p...@collabora.com>
AuthorDate: Fri May 15 19:11:52 2026 +0200
Commit: Miklos Vajna <vmi...@collabora.com>
CommitDate: Thu May 21 08:35:44 2026 +0000
follow-up to maintain view position on reconnection
This is a follow-up to patch 3b137c5bca4377def4f07196b7aa6dd95c02f45b
(maintain view position on reconnection) with some corrections. The view
jump is very flaky and difficult to reproduce. The previous still
produced view jumps in some cases.
Signed-off-by: Jaume Pujantell <jaume.p...@collabora.com>
Change-Id: I2d7ab2e81e6512d23259fff31526ec263a0f6a18
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/2485
index 523b228853a7..c9b52faa9233 100644
--- a/browser/src/layer/tile/WriterTileLayer.js
+++ b/browser/src/layer/tile/WriterTileLayer.js
@@ -109,14 +109,17 @@ window.L.WriterTileLayer = window.L.CanvasTileLayer.extend({
_setNewSize: function (/*cool.SimplePoint*/ size) {
app.activeDocument.fileSize = size;
- app.activeDocument.activeLayout.viewSize = app.activeDocument.fileSize.clone();
+ app.activeDocument.activeLayout.viewSize = size.clone();
this._updateMaxBounds(true);
},
_releaseReconnectFileSize: function () {
- this._setNewSize(this._reconnectFileSize);
this._reconnectFileSize = null;
this._reconnectFileSizeTimer = null;
+ var last = this._reconnectLatestStatus;
+ this._reconnectLatestStatus = null;
+ if (last && (last.x !== app.activeDocument.fileSize.x || last.y !== app.activeDocument.fileSize.y))
+ this._setNewSize(last);
},
_onStatusMsg: function (textMsg) {
@@ -139,20 +142,24 @@ window.L.WriterTileLayer = window.L.CanvasTileLayer.extend({
if (statusJSON.readonly && !this._documentInfo)
this._map.setPermission('readonly');
- var sizeChanged = statusJSON.width !== app.activeDocument.fileSize.x || statusJSON.height !== app.activeDocument.fileSize.y;
- if (sizeChanged && (app.socket._reconnecting || this._reconnectFileSize)) {
+ // Suppress shrinking sizes during reconnect's incremental reload
+ // so setMaxBounds doesn't pan the view; timer covers real shrinks.
+ if (app.socket._reconnecting && !this._reconnectFileSize && app.activeDocument.fileSize.y > 0)
+ this._reconnectFileSize = app.activeDocument.fileSize.clone();
+ if (this._reconnectFileSize) {
if (this._reconnectFileSizeTimer)
clearTimeout(this._reconnectFileSizeTimer);
- if (statusJSON.width >= app.activeDocument.fileSize.x && statusJSON.height >= app.activeDocument.fileSize.y) {
+ if (statusJSON.width >= this._reconnectFileSize.x && statusJSON.height >= this._reconnectFileSize.y) {
- // Suppress shrinking sizes during reconnection incremental reload
- // The timer avoids this supression being permanent
- sizeChanged = false;
- this._reconnectFileSize = new cool.SimplePoint(statusJSON.width, statusJSON.height);
- this._reconnectFileSizeTimer = setTimeout(this._releaseReconnectFileSize.bind(this), 5000);
+ const RECONNECT_FILE_SIZE_RELEASE_MS = 5000;
+ this._reconnectLatestStatus = new cool.SimplePoint(statusJSON.width, statusJSON.height);
+ this._reconnectFileSizeTimer = setTimeout(this._releaseReconnectFileSize.bind(this), RECONNECT_FILE_SIZE_RELEASE_MS);
}
}
+ var sizeChanged = !this._reconnectFileSize &&
+ (statusJSON.width !== app.activeDocument.fileSize.x || statusJSON.height !== app.activeDocument.fileSize.y);
if (statusJSON.viewid !== undefined) {
this._viewId = statusJSON.viewid;
diff --git a/cypress_test/integration_tests/desktop/writer3/reconnect_spec.js b/cypress_test/integration_tests/desktop/writer3/reconnect_spec.js
index e23e14260f78..c693d2c350ef 100644
--- a/cypress_test/integration_tests/desktop/writer3/reconnect_spec.js
+++ b/cypress_test/integration_tests/desktop/writer3/reconnect_spec.js
@@ -16,20 +16,18 @@ describe(['tagdesktop'], 'WebSocket reconnection', function () {
cy.getFrameWindow().then(function (win) {
preDisconnectY1 = win.app.activeDocument.activeLayout.viewedRectangle.y1;
expect(preDisconnectY1).to.be.greaterThan(0);
- });
-
- // Close the raw WebSocket to trigger automatic reconnection
- cy.getFrameWindow().then(function (win) {
win.app.socket.socket.close();
});
// Can't use processToIdle with the socket closed
cy.wait(500);
+ // Wait for reconnection to complete
helper.processToIdle(win);
});
+ // Verify the page position is preserved after reconnection
desktopHelper.assertVisiblePage(12, 12, 12);
cy.cGet('#document-canvas').click(200, 200);
@@ -46,9 +45,10 @@ describe(['tagdesktop'], 'WebSocket reconnection', function () {
desktopHelper.assertVisiblePage(12, 12, 12);
cy.getFrameWindow().then(function (win) {
+ const DRIFT_TOLERANCE_TWIPS = 5000;
var postClickY1 = win.app.activeDocument.activeLayout.viewedRectangle.y1;
+ expect(drift).to.be.lessThan(DRIFT_TOLERANCE_TWIPS);
});
});
});
cypress_test/integration_tests/desktop/writer3/reconnect_spec.js | 10 +--
2 files changed, 22 insertions(+), 15 deletions(-)
New commits:
commit fcf291766d6c089f3f03a3c72feb2a5c135403ef
Author: Jaume Pujantell <jaume.p...@collabora.com>
AuthorDate: Fri May 15 19:11:52 2026 +0200
Commit: Miklos Vajna <vmi...@collabora.com>
CommitDate: Thu May 21 08:35:44 2026 +0000
follow-up to maintain view position on reconnection
This is a follow-up to patch 3b137c5bca4377def4f07196b7aa6dd95c02f45b
(maintain view position on reconnection) with some corrections. The view
jump is very flaky and difficult to reproduce. The previous still
produced view jumps in some cases.
Signed-off-by: Jaume Pujantell <jaume.p...@collabora.com>
Change-Id: I2d7ab2e81e6512d23259fff31526ec263a0f6a18
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/2485
Reviewed-by: Miklos Vajna <vmi...@collabora.com>
Tested-by: Jenkins CPCI <rel...@collaboraoffice.com>
diff --git a/browser/src/layer/tile/WriterTileLayer.js b/browser/src/layer/tile/WriterTileLayer.js
Tested-by: Jenkins CPCI <rel...@collaboraoffice.com>
index 523b228853a7..c9b52faa9233 100644
--- a/browser/src/layer/tile/WriterTileLayer.js
+++ b/browser/src/layer/tile/WriterTileLayer.js
@@ -109,14 +109,17 @@ window.L.WriterTileLayer = window.L.CanvasTileLayer.extend({
_setNewSize: function (/*cool.SimplePoint*/ size) {
app.activeDocument.fileSize = size;
- app.activeDocument.activeLayout.viewSize = app.activeDocument.fileSize.clone();
+ app.activeDocument.activeLayout.viewSize = size.clone();
this._updateMaxBounds(true);
},
_releaseReconnectFileSize: function () {
- this._setNewSize(this._reconnectFileSize);
this._reconnectFileSize = null;
this._reconnectFileSizeTimer = null;
+ var last = this._reconnectLatestStatus;
+ this._reconnectLatestStatus = null;
+ if (last && (last.x !== app.activeDocument.fileSize.x || last.y !== app.activeDocument.fileSize.y))
+ this._setNewSize(last);
},
_onStatusMsg: function (textMsg) {
@@ -139,20 +142,24 @@ window.L.WriterTileLayer = window.L.CanvasTileLayer.extend({
if (statusJSON.readonly && !this._documentInfo)
this._map.setPermission('readonly');
- var sizeChanged = statusJSON.width !== app.activeDocument.fileSize.x || statusJSON.height !== app.activeDocument.fileSize.y;
- if (sizeChanged && (app.socket._reconnecting || this._reconnectFileSize)) {
+ // Suppress shrinking sizes during reconnect's incremental reload
+ // so setMaxBounds doesn't pan the view; timer covers real shrinks.
+ if (app.socket._reconnecting && !this._reconnectFileSize && app.activeDocument.fileSize.y > 0)
+ this._reconnectFileSize = app.activeDocument.fileSize.clone();
+ if (this._reconnectFileSize) {
if (this._reconnectFileSizeTimer)
clearTimeout(this._reconnectFileSizeTimer);
- if (statusJSON.width >= app.activeDocument.fileSize.x && statusJSON.height >= app.activeDocument.fileSize.y) {
+ if (statusJSON.width >= this._reconnectFileSize.x && statusJSON.height >= this._reconnectFileSize.y) {
this._reconnectFileSize = null;
+ this._reconnectFileSizeTimer = null;
} else {
+ this._reconnectFileSizeTimer = null;
- // Suppress shrinking sizes during reconnection incremental reload
- // The timer avoids this supression being permanent
- sizeChanged = false;
- this._reconnectFileSize = new cool.SimplePoint(statusJSON.width, statusJSON.height);
- this._reconnectFileSizeTimer = setTimeout(this._releaseReconnectFileSize.bind(this), 5000);
+ const RECONNECT_FILE_SIZE_RELEASE_MS = 5000;
+ this._reconnectLatestStatus = new cool.SimplePoint(statusJSON.width, statusJSON.height);
+ this._reconnectFileSizeTimer = setTimeout(this._releaseReconnectFileSize.bind(this), RECONNECT_FILE_SIZE_RELEASE_MS);
}
}
+ var sizeChanged = !this._reconnectFileSize &&
+ (statusJSON.width !== app.activeDocument.fileSize.x || statusJSON.height !== app.activeDocument.fileSize.y);
if (statusJSON.viewid !== undefined) {
this._viewId = statusJSON.viewid;
index e23e14260f78..c693d2c350ef 100644
--- a/cypress_test/integration_tests/desktop/writer3/reconnect_spec.js
+++ b/cypress_test/integration_tests/desktop/writer3/reconnect_spec.js
@@ -16,20 +16,18 @@ describe(['tagdesktop'], 'WebSocket reconnection', function () {
helper.typeIntoDocument('{ctrl}{End}');
desktopHelper.assertVisiblePage(12, 12, 12);
desktopHelper.assertVisiblePage(12, 12, 12);
+ // Close the raw WebSocket to trigger automatic reconnection
var preDisconnectY1;
cy.getFrameWindow().then(function (win) {
preDisconnectY1 = win.app.activeDocument.activeLayout.viewedRectangle.y1;
expect(preDisconnectY1).to.be.greaterThan(0);
- });
-
- // Close the raw WebSocket to trigger automatic reconnection
- cy.getFrameWindow().then(function (win) {
win.app.socket.socket.close();
});
// Can't use processToIdle with the socket closed
cy.wait(500);
cy.cGet('#document-canvas').should('be.visible');
cy.getFrameWindow().its('app.socket._reconnecting')
.should('eq', false);
@@ -37,6 +35,7 @@ describe(['tagdesktop'], 'WebSocket reconnection', function () {
cy.getFrameWindow().its('app.socket._reconnecting')
.should('eq', false);
helper.processToIdle(win);
});
+ // Verify the page position is preserved after reconnection
desktopHelper.assertVisiblePage(12, 12, 12);
cy.cGet('#document-canvas').click(200, 200);
@@ -46,9 +45,10 @@ describe(['tagdesktop'], 'WebSocket reconnection', function () {
desktopHelper.assertVisiblePage(12, 12, 12);
cy.getFrameWindow().then(function (win) {
+ const DRIFT_TOLERANCE_TWIPS = 5000;
var postClickY1 = win.app.activeDocument.activeLayout.viewedRectangle.y1;
var drift = Math.abs(postClickY1 - preDisconnectY1);
- expect(drift).to.be.lessThan(preDisconnectY1 / 2);
+ expect(drift).to.be.lessThan(DRIFT_TOLERANCE_TWIPS);
});
});
});
"Méven Car (via cogerrit)"
6:28 AM (11 hours ago) 6:28 AM
to collaboraon...@googlegroups.com
browser/src/control/Control.UIManager.ts | 12 --
browser/src/control/jsdialog/Widget.OverflowManager.ts | 22 ++++-
cypress_test/integration_tests/desktop/writer4/compact_toolbar_overflow_spec.js | 41 ++++++++++
3 files changed, 65 insertions(+), 10 deletions(-)
New commits:
commit ced359d25f1a7b9850bfa3d6cc93b61df97e938f
Author: Méven Car <me...@meven-thinkpad.home>
AuthorDate: Thu May 14 12:21:30 2026 +0000
Commit: Méven <meve...@collabora.com>
CommitDate: Mon May 25 10:27:23 2026 +0000
overflow manager: don't fold groups before the parent has been measured
In classic mode the first onRefresh fires before the toolbar's parent
container has been laid out; scrollWidth is 0 and the 'unknown width
-> assume overflow' safety branch in hasOverflow() folds every group.
Until the user resizes the window (which schedules a fresh onRefresh
after layout has settled) the menu stays compressed.
- hasOverflow() now returns false when requiredWidth is 0, so an
unmeasurable pass leaves the toolbar fully expanded rather than
fully collapsed.
- The OverflowManager installs a one-shot ResizeObserver on its
parent. The first time scrollWidth becomes non-zero it disconnects
and forces an onRefresh, so the real fold decision happens once
the browser actually has dimensions to give us.
- Drop the explicit post-mode-change refreshoverflows fire from
UIManager.onChangeUIMode: the freshly built OverflowManager now
reconciles itself via its constructor-time observer.
Notebookbar mode was already fine because tab activation forces a
layout before the refresh fires; the tab-click handler still fires
refreshoverflows to cover the "previously hidden tab is shown after
a window resize" case the constructor-time observer doesn't catch.
Add a cypress regression test that loads writer/top_toolbar.odt in
compact mode at 1920x1080 and asserts every #toolbar-up
.ui-overflow-group is unfolded (class kept, more-button hidden) on
initial load, plus a narrow-viewport sanity that the manager still
folds when the toolbar genuinely doesn't fit.
Signed-off-by: Méven Car <meve...@collabora.com>
Change-Id: Icb8616b1e0fccf5ef6c75e3b14d4a8cde7dc4f31
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/2606
Tested-by: Jenkins CPCI <rel...@collaboraoffice.com>
Reviewed-by: Parth Raiyani <parth....@collabora.com>
diff --git a/browser/src/control/Control.UIManager.ts b/browser/src/control/Control.UIManager.ts
index 712ced2b7d53..2b2f7d8571d4 100644
--- a/browser/src/control/Control.UIManager.ts
+++ b/browser/src/control/Control.UIManager.ts
@@ -1064,14 +1064,10 @@ class UIManager extends window.L.Control {
// be sure we hide old scrollable markers
JSDialog.RefreshScrollables();
- // Recalculate overflow layout after all DOM modifications.
- // RefreshScrollables dispatches a resize event, but the
- // OverflowManager skips resize events when the window size
- // is unchanged. Fire refreshoverflows explicitly with layouting service
- // so the browser has time to lay out the new DOM before overflow is measured.
- app.layoutingService.appendLayoutingTask(() => {
- this.map.fire('refreshoverflows', { force: true });
- });
+ // Overflow layout reconciles itself: every newly built
+ // OverflowManager installs a one-shot ResizeObserver on its
+ // parent and fires refreshoverflows once the container has a
+ // measurable width.
}
// UI modification
diff --git a/browser/src/control/jsdialog/Widget.OverflowManager.ts b/browser/src/control/jsdialog/Widget.OverflowManager.ts
index a7a51940a0c1..6444514161c1 100644
--- a/browser/src/control/jsdialog/Widget.OverflowManager.ts
+++ b/browser/src/control/jsdialog/Widget.OverflowManager.ts
@@ -20,6 +20,7 @@ class OverflowManager {
data: ContainerWidgetJSON;
lastMaxWidth: number = -1;
scheduledRefresh: TaskId = '';
+ initialSizeResizeObserver: ResizeObserver | null = null;
constructor(parentContainer: Element, data: ContainerWidgetJSON) {
this.parentContainer = parentContainer as HTMLElement;
@@ -28,6 +29,20 @@ class OverflowManager {
window.addEventListener('resize', this.onResize.bind(this));
if (app.map) app.map.on('refreshoverflows', this.onRefresh, this);
else app.console.error('OverflowManager: no app.map available');
+
+ // In classic mode the first onRefresh can fire before the parent has
+ // been laid out, leaving scrollWidth at 0 and every group folded.
+ // Re-run once the container actually has a measurable width.
+ if (typeof ResizeObserver !== 'undefined') {
+ this.initialSizeResizeObserver = new ResizeObserver(() => {
+ if (this.parentContainer.scrollWidth > 0) {
+ this.initialSizeResizeObserver?.disconnect();
+ this.initialSizeResizeObserver = null;
+ this.onRefresh({ force: true } as Event & { force?: boolean });
+ }
+ });
+ this.initialSizeResizeObserver.observe(this.parentContainer);
+ }
}
calculateMaxWidth(): number {
@@ -60,8 +75,11 @@ class OverflowManager {
requiredWidth,
);
- // not yet known width -> do not assume it is small to prevent scrollbars
- if (requiredWidth === 0) return true;
+ // Width not known yet -> defer the decision. The ResizeObserver
+ // installed in the constructor will retrigger onRefresh once the
+ // container has been measured; folding now would leave every group
+ // collapsed until the next window resize.
+ if (requiredWidth === 0) return false;
return maxWidth < requiredWidth;
}
diff --git a/cypress_test/integration_tests/desktop/writer4/compact_toolbar_overflow_spec.js b/cypress_test/integration_tests/desktop/writer4/compact_toolbar_overflow_spec.js
new file mode 100644
index 000000000000..9a71a4b2db7d
--- /dev/null
+++ b/cypress_test/integration_tests/desktop/writer4/compact_toolbar_overflow_spec.js
@@ -0,0 +1,41 @@
+/* global describe it cy beforeEach require */
+
+// Regression for the OverflowManager initial-measurement bug: in compact
+// mode the first onRefresh used to run before the toolbar's parent had
+// been laid out, scrollWidth was 0, hasOverflow() took its "unknown
+// width -> assume overflow" branch and every group folded. The fix
+// (one-shot ResizeObserver + hasOverflow returning false on
+// requiredWidth==0) is what these tests guard.
+describe(['tagdesktop'], 'Compact toolbar overflow groups', function() {
+
+ beforeEach(function() {
+ cy.viewport(1920, 1080);
+ helper.setupAndLoadDocument('writer/top_toolbar.odt');
+ desktopHelper.switchUIToCompact();
+ cy.cGet('#toolbar-up').should('be.visible');
+ });
+
+ it('starts with every overflow group unfolded at a wide viewport', function() {
+ // There must be at least one overflow group to validate.
+ cy.cGet('#toolbar-up .ui-overflow-group').should('have.length.gte', 1);
+
+ cy.cGet('#toolbar-up .ui-overflow-group').each(($el) => {
+ cy.wrap($el)
+ .should('have.class', 'ui-overflow-group-container-with-label');
+ cy.wrap($el).find('[id^="overflow-button-"]')
+ .should('have.css', 'display', 'none');
+ });
+ });
+
+ it('folds groups at a narrow viewport (observer drives real fold)', function() {
+ // Sanity: after the constructor-time observer fires onRefresh with
+ // a measurable width, a viewport too small for the toolbar must
+ // produce at least one folded group.
+ cy.viewport(640, 1080);
+ cy.cGet('#toolbar-up [id^="overflow-button-"]:visible')
+ .should('have.length.gte', 1);
+ });
+});
browser/src/control/jsdialog/Widget.OverflowManager.ts | 22 ++++-
cypress_test/integration_tests/desktop/writer4/compact_toolbar_overflow_spec.js | 41 ++++++++++
3 files changed, 65 insertions(+), 10 deletions(-)
New commits:
commit ced359d25f1a7b9850bfa3d6cc93b61df97e938f
Author: Méven Car <me...@meven-thinkpad.home>
AuthorDate: Thu May 14 12:21:30 2026 +0000
Commit: Méven <meve...@collabora.com>
CommitDate: Mon May 25 10:27:23 2026 +0000
overflow manager: don't fold groups before the parent has been measured
In classic mode the first onRefresh fires before the toolbar's parent
container has been laid out; scrollWidth is 0 and the 'unknown width
-> assume overflow' safety branch in hasOverflow() folds every group.
Until the user resizes the window (which schedules a fresh onRefresh
after layout has settled) the menu stays compressed.
- hasOverflow() now returns false when requiredWidth is 0, so an
unmeasurable pass leaves the toolbar fully expanded rather than
fully collapsed.
- The OverflowManager installs a one-shot ResizeObserver on its
parent. The first time scrollWidth becomes non-zero it disconnects
and forces an onRefresh, so the real fold decision happens once
the browser actually has dimensions to give us.
- Drop the explicit post-mode-change refreshoverflows fire from
UIManager.onChangeUIMode: the freshly built OverflowManager now
reconciles itself via its constructor-time observer.
Notebookbar mode was already fine because tab activation forces a
layout before the refresh fires; the tab-click handler still fires
refreshoverflows to cover the "previously hidden tab is shown after
a window resize" case the constructor-time observer doesn't catch.
Add a cypress regression test that loads writer/top_toolbar.odt in
compact mode at 1920x1080 and asserts every #toolbar-up
.ui-overflow-group is unfolded (class kept, more-button hidden) on
initial load, plus a narrow-viewport sanity that the manager still
folds when the toolbar genuinely doesn't fit.
Signed-off-by: Méven Car <meve...@collabora.com>
Change-Id: Icb8616b1e0fccf5ef6c75e3b14d4a8cde7dc4f31
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/2606
Tested-by: Jenkins CPCI <rel...@collaboraoffice.com>
Reviewed-by: Parth Raiyani <parth....@collabora.com>
diff --git a/browser/src/control/Control.UIManager.ts b/browser/src/control/Control.UIManager.ts
index 712ced2b7d53..2b2f7d8571d4 100644
--- a/browser/src/control/Control.UIManager.ts
+++ b/browser/src/control/Control.UIManager.ts
@@ -1064,14 +1064,10 @@ class UIManager extends window.L.Control {
// be sure we hide old scrollable markers
JSDialog.RefreshScrollables();
- // Recalculate overflow layout after all DOM modifications.
- // RefreshScrollables dispatches a resize event, but the
- // OverflowManager skips resize events when the window size
- // is unchanged. Fire refreshoverflows explicitly with layouting service
- // so the browser has time to lay out the new DOM before overflow is measured.
- app.layoutingService.appendLayoutingTask(() => {
- this.map.fire('refreshoverflows', { force: true });
- });
+ // Overflow layout reconciles itself: every newly built
+ // OverflowManager installs a one-shot ResizeObserver on its
+ // parent and fires refreshoverflows once the container has a
+ // measurable width.
}
// UI modification
diff --git a/browser/src/control/jsdialog/Widget.OverflowManager.ts b/browser/src/control/jsdialog/Widget.OverflowManager.ts
index a7a51940a0c1..6444514161c1 100644
--- a/browser/src/control/jsdialog/Widget.OverflowManager.ts
+++ b/browser/src/control/jsdialog/Widget.OverflowManager.ts
@@ -20,6 +20,7 @@ class OverflowManager {
data: ContainerWidgetJSON;
lastMaxWidth: number = -1;
scheduledRefresh: TaskId = '';
+ initialSizeResizeObserver: ResizeObserver | null = null;
constructor(parentContainer: Element, data: ContainerWidgetJSON) {
this.parentContainer = parentContainer as HTMLElement;
@@ -28,6 +29,20 @@ class OverflowManager {
window.addEventListener('resize', this.onResize.bind(this));
if (app.map) app.map.on('refreshoverflows', this.onRefresh, this);
else app.console.error('OverflowManager: no app.map available');
+
+ // In classic mode the first onRefresh can fire before the parent has
+ // been laid out, leaving scrollWidth at 0 and every group folded.
+ // Re-run once the container actually has a measurable width.
+ if (typeof ResizeObserver !== 'undefined') {
+ this.initialSizeResizeObserver = new ResizeObserver(() => {
+ if (this.parentContainer.scrollWidth > 0) {
+ this.initialSizeResizeObserver?.disconnect();
+ this.initialSizeResizeObserver = null;
+ this.onRefresh({ force: true } as Event & { force?: boolean });
+ }
+ });
+ this.initialSizeResizeObserver.observe(this.parentContainer);
+ }
}
calculateMaxWidth(): number {
@@ -60,8 +75,11 @@ class OverflowManager {
requiredWidth,
);
- // not yet known width -> do not assume it is small to prevent scrollbars
- if (requiredWidth === 0) return true;
+ // Width not known yet -> defer the decision. The ResizeObserver
+ // installed in the constructor will retrigger onRefresh once the
+ // container has been measured; folding now would leave every group
+ // collapsed until the next window resize.
+ if (requiredWidth === 0) return false;
return maxWidth < requiredWidth;
}
diff --git a/cypress_test/integration_tests/desktop/writer4/compact_toolbar_overflow_spec.js b/cypress_test/integration_tests/desktop/writer4/compact_toolbar_overflow_spec.js
new file mode 100644
index 000000000000..9a71a4b2db7d
--- /dev/null
+++ b/cypress_test/integration_tests/desktop/writer4/compact_toolbar_overflow_spec.js
@@ -0,0 +1,41 @@
+/* global describe it cy beforeEach require */
+
+var helper = require('../../common/helper');
+var desktopHelper = require('../../common/desktop_helper');
+var helper = require('../../common/helper');
+
+// Regression for the OverflowManager initial-measurement bug: in compact
+// mode the first onRefresh used to run before the toolbar's parent had
+// been laid out, scrollWidth was 0, hasOverflow() took its "unknown
+// width -> assume overflow" branch and every group folded. The fix
+// (one-shot ResizeObserver + hasOverflow returning false on
+// requiredWidth==0) is what these tests guard.
+describe(['tagdesktop'], 'Compact toolbar overflow groups', function() {
+
+ beforeEach(function() {
+ cy.viewport(1920, 1080);
+ helper.setupAndLoadDocument('writer/top_toolbar.odt');
+ desktopHelper.switchUIToCompact();
+ cy.cGet('#toolbar-up').should('be.visible');
+ });
+
+ it('starts with every overflow group unfolded at a wide viewport', function() {
+ // There must be at least one overflow group to validate.
+ cy.cGet('#toolbar-up .ui-overflow-group').should('have.length.gte', 1);
+
+ cy.cGet('#toolbar-up .ui-overflow-group').each(($el) => {
+ cy.wrap($el)
+ .should('have.class', 'ui-overflow-group-container-with-label');
+ cy.wrap($el).find('[id^="overflow-button-"]')
+ .should('have.css', 'display', 'none');
+ });
+ });
+
+ it('folds groups at a narrow viewport (observer drives real fold)', function() {
+ // Sanity: after the constructor-time observer fires onRefresh with
+ // a measurable width, a viewport too small for the toolbar must
+ // produce at least one folded group.
+ cy.viewport(640, 1080);
+ cy.cGet('#toolbar-up [id^="overflow-button-"]:visible')
+ .should('have.length.gte', 1);
+ });
+});
Reply all
Reply to author
Forward
0 new messages