online.git: browser/src cypress_test/data cypress_test/integration_tests

["Mike Kaganski (via cogerrit)"]

browser/src/canvas/sections/CommentListSection.ts | 43 +-
browser/src/canvas/sections/CommentSection.ts | 4
browser/src/map/handler/Map.WOPI.js | 79 +++-
cypress_test/data/desktop/draw/threaded_comments.pdf | 54 ++
cypress_test/integration_tests/desktop/draw/annotation_spec.js | 192 ++++++++++
5 files changed, 358 insertions(+), 14 deletions(-)

New commits:
commit 75ec7d60cc118259f633ab39de8b9be4001014df
Author: Mike Kaganski <mike.k...@collabora.com>
AuthorDate: Wed Apr 22 19:22:53 2026 +0500
Commit: Miklos Vajna <vmi...@collabora.com>
CommitDate: Mon Apr 27 08:10:58 2026 +0000

Support Action_GoToComment / Action_ResolveComment for Draw/PDF

Route the two WOPI commands to a new _goToDrawComment handler that
covers Draw/Impress PDF threaded comments. Same-part navigation reuses
_goToComment; cross-part waits for ImpressTileLayer's 'setpart' event
so _scrollViewToPartPosition has already settled the viewport before
we show the dialog.

Extend the doNotHideCommentTimer guard - previously used only by Calc -
to keep the selection stable across the async settling window:
CommentListSection.onPartChange and importComments both honor it now.
Without this, a status-msg-triggered 'updateparts' cancels the
freshly-selected comment, or the annotations refresh that the same
status message kicks off silently drops it.

Fix scrollCommentIntoView. It had a wrong sign of the scroll offset;
using ViewLayoutBase.scroll turned out problematic in general - use
scrollTo instead, which allows to pass absolute document coordinates
instead of offsets. The comment height wasn't converted from CSS units
to pixels previously. The logic was improved to do minimal scrolling
taking scroll direction into account, and add a 5% viewport margin so
the anchor is not jammed against the edge.

Make sure to add the Threaded flag to the Get_Comments response, like
Calc does already.

Show the resolve menu entry for threaded Draw/Impress comments, and
accept Action_ResolveComment for drawing.

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

diff --git a/browser/src/canvas/sections/CommentListSection.ts b/browser/src/canvas/sections/CommentListSection.ts
index 9c6f9d9fc817..8fc6b423e073 100644
--- a/browser/src/canvas/sections/CommentListSection.ts
+++ b/browser/src/canvas/sections/CommentListSection.ts
@@ -1046,18 +1046,26 @@ export class CommentSection extends CanvasSectionObject {
.getPrintTwipsPointFromTile(new cool.Point(0, topTwips)).y;
}
const topVisible = app.isYVisibleInTheDisplayedArea(topTwips);
+ const commentHeight = this.cssToCorePixels(rootComment.getCommentHeight());
const bottomVisible = app.isYVisibleInTheDisplayedArea(
- topTwips + Math.round(rootComment.getCommentHeight() * app.pixelsToTwips)
+ topTwips + Math.round(commentHeight * app.pixelsToTwips)
);

- const topBottom = this.getScreenTopBottom();
-
if (!topVisible || !bottomVisible) {
const topPixels = topTwips * app.twipsToPixels;
- if (!topVisible)
- app.activeDocument.activeLayout.scroll(0, topBottom[0] - topPixels);
- else if (!bottomVisible)
- app.activeDocument.activeLayout.scroll(0, (topPixels + rootComment.getCommentHeight() - topBottom[1]));
+ const topBottom = this.getScreenTopBottom();
+ const viewHeight = topBottom[1] - topBottom[0];
+ const needsScrollDown = topPixels > topBottom[0];
+ // Have a margin of 5% of view height, to avoid edge effects
+ // (e.g. when top of comment aligned with anchor will not fit to view)
+ let scrollPos = topPixels - viewHeight * 0.05;
+ if (needsScrollDown) {
+ // Only consider when the comment can fit into view
+ const bottomViewPos = topPixels + commentHeight - viewHeight * 0.95;
+ if (bottomViewPos < scrollPos)
+ scrollPos = bottomViewPos;
+ }
+ app.activeDocument.activeLayout.scrollTo(0, scrollPos);

if (app.map._docLayer._docType === 'spreadsheet' && rootComment) {
rootComment.positionCalcComment();
@@ -1825,7 +1833,11 @@ export class CommentSection extends CanvasSectionObject {
for (var i: number = 0; i < this.sectionProperties.commentList.length; i++) {
this.showHideComment(this.sectionProperties.commentList[i]);
}
- if (this.sectionProperties.selectedComment)
+ // Honor the doNotHideCommentTimer guard: when Action_GoToComment has
+ // just navigated to a comment on another part, a status-msg-triggered
+ // 'updateparts' can fire right after the select() and would otherwise
+ // cancel the freshly-selected comment.
+ if (this.sectionProperties.selectedComment && !this.sectionProperties.doNotHideCommentTimer)
this.sectionProperties.selectedComment.onCancelClick(null);

this.checkSize();
@@ -1865,6 +1877,9 @@ export class CommentSection extends CanvasSectionObject {

comment.parthash = comment.parthash ? comment.parthash: null;

+ if (comment.parentId)
+ comment.parent = String(comment.parentId);
+
var viewId = this.map.getViewId(comment.author);
var color = viewId >= 0 ? app.LOUtil.rgbToHex(this.map.getViewColor(viewId)) : '#43ACE8';
comment.color = color;
@@ -2556,6 +2571,16 @@ export class CommentSection extends CanvasSectionObject {
return;
}

+ // If a navigation (e.g. Action_GoToComment) just selected a comment
+ // and set the doNotHideCommentTimer guard, remember the id so we can
+ // re-select the rebuilt comment at the end of the import. Without
+ // this, a status-msg-triggered annotations refresh arriving right
+ // after the navigation would silently drop the selection.
+ const preserveSelectedId = this.sectionProperties.doNotHideCommentTimer
+ && this.sectionProperties.selectedComment
+ ? this.sectionProperties.selectedComment.sectionProperties.data.id
+ : null;
+
CommentSection.importingComments = true;
let drawPaused = false;
if (app.map._docLayer._docType === 'spreadsheet') {
@@ -2617,6 +2642,8 @@ export class CommentSection extends CanvasSectionObject {
if (drawPaused) {
this.containerObject.resumeDrawing();
}
+ if (preserveSelectedId !== null)
+ this.selectById(preserveSelectedId);
}

// Accepts redlines/changes comments.
diff --git a/browser/src/canvas/sections/CommentSection.ts b/browser/src/canvas/sections/CommentSection.ts
index 14f030604924..3c87353aafab 100644
--- a/browser/src/canvas/sections/CommentSection.ts
+++ b/browser/src/canvas/sections/CommentSection.ts
@@ -1095,8 +1095,8 @@ export class Comment extends CanvasSectionObject {
if (docLayer._docType === 'text' && this.isRootComment() && !blockChangeFromDifferentAuthor)
entries.push({ text: _('Remove Thread'), type: 'action', id: 'removeThread', pos: String(pos++) });

- if (docLayer._docType === 'text'
- || (docLayer._docType === 'spreadsheet' && data.threaded))
+ const isNonWriterComponent = ['spreadsheet', 'drawing', 'presentation'].includes(docLayer._docType);
+ if (docLayer._docType === 'text' || (isNonWriterComponent && data.threaded))
entries.push({
text: data.resolved === 'false' ? _('Resolve') : _('Unresolve'),
type: 'action', id: 'resolve', pos: String(pos++),
diff --git a/browser/src/map/handler/Map.WOPI.js b/browser/src/map/handler/Map.WOPI.js
index 948fba0e89f7..c44dbd81ee11 100644
--- a/browser/src/map/handler/Map.WOPI.js
+++ b/browser/src/map/handler/Map.WOPI.js
@@ -754,14 +754,19 @@ window.L.Map.WOPI = window.L.Handler.extend({
const data = commentList[i].sectionProperties.data;
if (data.trackchange)
continue;
- commentsResp.push({
+ const entry = {
Id: data.id,
Author: data.author,
DateTime: data.dateTime,
Text: commentList[i].sectionProperties.contentText.textContent,
Resolved: data.resolved,
Parent: data.parent,
- });
+ };
+ // Flag threaded comments so consumers can tell which ones support resolve.
+ // Writer comments don't set data.threaded; Draw PDF comments do.
+ if (data.threaded)
+ entry.Threaded = 'true';
+ commentsResp.push(entry);
}
}
}
@@ -824,10 +829,11 @@ window.L.Map.WOPI = window.L.Handler.extend({
}
else if (msg.MessageId === 'Action_ResolveComment') {
var docType = this._map._docLayer._docType;
- if (msg.Values && (docType === 'text' || docType === 'spreadsheet')) {
+ if (msg.Values && ['text', 'spreadsheet', 'drawing'].includes(docType)) {
const commentSection = app.sectionContainer.getSectionWithName(app.CSections.CommentList.name);
if (commentSection) {
const comment = commentSection.getComment(msg.Values.Id);
+ // Writer allows resolve on every comment; Calc and Draw only on threaded ones.
if (comment && comment.sectionProperties.data.resolved !== 'true'
&& (docType === 'text' || comment.sectionProperties.data.threaded)) {
commentSection.resolve(comment);
@@ -847,8 +853,10 @@ window.L.Map.WOPI = window.L.Handler.extend({
this._goToCalcComment(commentSection, msg.Values.Id);
else if (docType === 'text')
this._goToComment(commentSection, msg.Values.Id);
+ else if (['drawing', 'presentation'].includes(docType))
+ this._goToDrawComment(commentSection, msg.Values.Id);
else
- this._sendGoToCommentResp(msg.Values.Id, false, 'Unsupported document type'); // TODO: support Draw/Impress
+ this._sendGoToCommentResp(msg.Values.Id, false, 'Unsupported document type');
}
}
else if (msg.sender === 'EIDEASY_SINGLE_METHOD_SIGNATURE') {
@@ -966,6 +974,69 @@ window.L.Map.WOPI = window.L.Handler.extend({
map.setPart(targetTab);
},

+ _goToDrawComment: function(commentSection, commentId) {
+ // Draw and Impress share Writer's per-page annotation shape, but comments
+ // live on specific parts (slides or pages). If the target is on a different
+ // part than the current one, switch parts first so the annotation becomes
+ // reachable.
+ const comment = commentSection.getComment(commentId)
+ || commentSection.getComment(parseInt(commentId));
+ if (!comment) {
+ this._sendGoToCommentResp(commentId, false, 'Comment not found');
+ return;
+ }
+
+ const map = this._map;
+ const self = this;
+
+ function finish() {
+ // _goToComment -> navigateAndFocusComment -> scrollCommentIntoView
+ // scrolls the anchor into view for every docType.
+ self._goToComment(commentSection, commentId);
+ // Guard the freshly-selected comment against async events that
+ // would otherwise cancel it during the settling window. Calc uses
+ // this same flag to keep the comment visible across
+ // _onSetPartMsg / onNewDocumentTopLeft / onCellAddressChanged;
+ // for Draw/Impress on a cross-part navigation, a status-msg-driven
+ // 'updateparts' fires right after setPart and would hit
+ // onPartChange which cancels selectedComment, and an annotations
+ // refresh rebuilds the comment list and drops the selection too.
+ const props = commentSection.sectionProperties;
+ if (props.doNotHideCommentTimer)
+ clearTimeout(props.doNotHideCommentTimer);
+ props.doNotHideCommentTimer = setTimeout(function() {
+ props.doNotHideCommentTimer = null;
+ }, 2000);
+ }
+
+ const targetPart = comment.sectionProperties.partIndex;
+ if (targetPart < 0 || targetPart === map._docLayer._selectedPart) {
+ finish();
+ return;
+ }
+
+ // Cross-part: setPart triggers an async round-trip with the server.
+ // ImpressTileLayer._onSetPartMsg fires 'setpart' after
+ // _scrollViewToPartPosition has settled the viewport. Wait for that,
+ // then run finish() - the doNotHideCommentTimer set there prevents a
+ // later status-msg-driven 'updateparts' from canceling the selection.
+ let safetyTimer = null;
+ function cleanup() {
+ clearTimeout(safetyTimer);
+ map.off('setpart', onPart);
+ }
+ function onPart() {
+ cleanup();
+ finish();
+ }
+ safetyTimer = setTimeout(function() {
+ cleanup();
+ self._sendGoToCommentResp(commentId, false, 'Timed out waiting for server');
+ }, 10000);
+ map.once('setpart', onPart);
+ map.setPart(targetPart);
+ },
+
_sendGoToCommentResp: function(commentId, success, errorMsg) {
var args = { Id: String(commentId), success: success };
if (errorMsg)
diff --git a/cypress_test/data/desktop/draw/threaded_comments.pdf b/cypress_test/data/desktop/draw/threaded_comments.pdf
new file mode 100644
index 000000000000..afb5dcf2d6e4
--- /dev/null
+++ b/cypress_test/data/desktop/draw/threaded_comments.pdf
@@ -0,0 +1,54 @@
+%PDF-1.5
+%����
+1 0 obj
+<</Type/Catalog/Pages 2 0 R>>
+endobj
+2 0 obj
+<</Type/Pages/Kids[3 0 R 4 0 R 11 0 R]/Count 3>>
+endobj
+3 0 obj
+<</Type/Page/Parent 2 0 R/MediaBox[0 0 612 792]/Resources<<>>/Annots[5 0 R 6 0 R 7 0 R 8 0 R 9 0 R]>>
+endobj
+4 0 obj
+<</Type/Page/Parent 2 0 R/MediaBox[0 0 612 792]/Resources<<>>/Annots[10 0 R]>>
+endobj
+5 0 obj
+<</Type/Annot/Subtype/Text/Rect[10 170 30 190]/Contents(Root comment)/T(Alice)/M(D:20260420120000+00'00')>>
+endobj
+6 0 obj
+<</Type/Annot/Subtype/Text/Rect[10 140 30 160]/Contents(Reply from Bob)/T(Bob)/M(D:20260420130000+00'00')/IRT 5 0 R>>
+endobj
+7 0 obj
+<</Type/Annot/Subtype/Text/Rect[10 110 30 130]/F 30/Contents(Completed set by Charlie)/T(Charlie)/M(D:20260420140000+00'00')/IRT 5 0 R/State(Completed)/StateModel(Review)>>
+endobj
+8 0 obj
+<</Type/Annot/Subtype/Text/Rect[10 80 30 100]/Contents(Self-state)/T(Dave)/M(D:20260420150000+00'00')/State(Completed)/StateModel(Review)>>
+endobj
+9 0 obj
+<</Type/Annot/Subtype/Text/Rect[10 50 30 70]/Contents(Reply-with-state)/T(Eve)/M(D:20260420160000+00'00')/IRT 5 0 R/State(Completed)/StateModel(Review)>>
+endobj
+10 0 obj
+<</Type/Annot/Subtype/Text/Rect[10 170 30 190]/Contents(Page 2 comment)/T(Zoe)/M(D:20260420170000+00'00')>>
+endobj
+11 0 obj
+<</Type/Page/Parent 2 0 R/MediaBox[0 0 612 792]/Resources<<>>>>
+endobj
+xref
+0 12
+0000000000 65535 f
+0000000015 00000 n
+0000000060 00000 n
+0000000124 00000 n
+0000000241 00000 n
+0000000335 00000 n
+0000000458 00000 n
+0000000591 00000 n
+0000000779 00000 n
+0000000934 00000 n
+0000001103 00000 n
+0000001227 00000 n
+trailer
+<</Size 12/Root 1 0 R>>
+startxref
+1307
+%%EOF
diff --git a/cypress_test/integration_tests/desktop/draw/annotation_spec.js b/cypress_test/integration_tests/desktop/draw/annotation_spec.js
new file mode 100644
index 000000000000..bafe28b029ed
--- /dev/null
+++ b/cypress_test/integration_tests/desktop/draw/annotation_spec.js
@@ -0,0 +1,192 @@
+/* -*- js-indent-level: 8 -*- */
+/*
+ * Copyright the Collabora Online contributors.
+ *
+ * SPDX-License-Identifier: MPL-2.0
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+/* global describe it cy require beforeEach expect */
+
+const helper = require('../../common/helper');
+
+describe(['tagdesktop'], 'PDF Threaded Comments', function() {
+
+ beforeEach(function() {
+ // Lock the viewport so scrolling is actually required: Alice's anchor
+ // is ~803px down page 1 and Zoe's is on page 2, both outside a 600-tall
+ // viewport. Without this, a larger default could make the anchors
+ // visible without any scroll, hiding regressions in the scroll logic.
+ cy.viewport(1000, 600);
+ helper.setupAndLoadDocument('draw/threaded_comments.pdf');
+ cy.getFrameWindow().then(function(win) {
+ helper.processToIdle(win);
+ });
+ });
+
+ function findCommentByAuthor(win, author) {
+ const commentSection = win.app.sectionContainer.getSectionWithName(
+ win.app.CSections.CommentList.name);
+ return commentSection.sectionProperties.commentList.find(
+ function(c) { return c.sectionProperties.data.author === author; });
+ }
+
+ it('Get_Comments returns threaded fields for PDF comments', { env: { 'pdf-view': true } }, function() {
+ cy.getFrameWindow().then(function(win) {
+ cy.stub(win.parent, 'postMessage').as('postMessage');
+ win.postMessage(JSON.stringify({ MessageId: 'Get_Comments' }), '*');
+ });
+
+ cy.get('@postMessage').should(function(stub) {
+ const calls = stub.getCalls().filter(function(call) {
+ try {
+ const msg = typeof call.args[0] === 'string'
+ ? JSON.parse(call.args[0]) : call.args[0];
+ return msg.MessageId === 'Get_Comments_Resp';
+ } catch (e) { return false; }
+ });
+ expect(calls.length, 'Get_Comments_Resp was not posted').to.be.greaterThan(0);
+ const resp = typeof calls[0].args[0] === 'string'
+ ? JSON.parse(calls[0].args[0]) : calls[0].args[0];
+ const comments = resp.Values.Comments;
+ // Alice, Bob, Dave (page 1) + Zoe (page 2). Charlie collapsed on import
+ // into Alice's resolved flag; Eve dropped as a malformed state-change.
+ expect(comments.length).to.equal(4);
+ const byAuthor = {};
+ comments.forEach(function(c) { byAuthor[c.Author] = c; });
+ expect(byAuthor.Alice, 'Alice missing').to.exist;
+ expect(byAuthor.Bob, 'Bob missing').to.exist;
+ expect(byAuthor.Dave, 'Dave missing').to.exist;
+ expect(byAuthor.Zoe, 'Zoe missing').to.exist;
+ expect(byAuthor.Alice.Threaded).to.equal('true');
+ expect(byAuthor.Alice.Resolved).to.equal('true');
+ expect(byAuthor.Bob.Threaded).to.equal('true');
+ expect(byAuthor.Bob.Resolved).to.equal('false');
+ expect(byAuthor.Bob.Parent).to.equal(String(byAuthor.Alice.Id));
+ expect(byAuthor.Dave.Threaded).to.equal('true');
+ expect(byAuthor.Dave.Resolved).to.equal('false');
+ expect(byAuthor.Zoe.Threaded).to.equal('true');
+ expect(byAuthor.Zoe.Resolved).to.equal('false');
+ });
+ });
+
+ it('Resolve menu item flips an unresolved comment to resolved', { env: { 'pdf-view': true } }, function() {
+ let daveId;
+ cy.getFrameWindow().then(function(win) {
+ const dave = findCommentByAuthor(win, 'Dave');
+ expect(dave, 'Dave not found').to.exist;
+ expect(dave.sectionProperties.data.resolved).to.not.equal('true');
+ daveId = dave.sectionProperties.data.id;
+ });
+
+ cy.then(function() {
+ cy.cGet('#comment-annotation-menu-' + daveId).click({ force: true });
+ cy.cGet('#comment-menu-' + daveId + '-dropdown')
+ .contains('.ui-combobox-entry.jsdialog.ui-grid-cell', 'Resolve')
+ .should('exist')
+ .click({ force: true });
+ });
+
+ cy.getFrameWindow().then(function(win) {
+ const dave = findCommentByAuthor(win, 'Dave');
+ expect(dave.sectionProperties.data.resolved).to.equal('true');
+ });
+ });
+
+ // Verifies that after Action_GoToComment:
+ // (a) the anchor's Y coordinate is inside the scrolled-to document area
+ // (b) the comment dialog is actually on-screen (bounding rect intersects
+ // the iframe viewport), not merely display != none - a dialog
+ // positioned off-canvas would otherwise pass a naive display check.
+ function assertCommentShownAndAnchorVisible(win, author) {
+ const comment = findCommentByAuthor(win, author);
+ expect(comment, author + ' not found').to.exist;
+ const container = comment.sectionProperties.container;
+ expect(container, author + ' has no container DOM node').to.exist;
+ expect(getComputedStyle(container).display,
+ author + ' comment dialog display=none').to.not.equal('none');
+
+ const anchorY = comment.sectionProperties.data.anchorPos[1];
+ expect(win.app.isYVisibleInTheDisplayedArea(anchorY),
+ author + ' anchor not scrolled into view (anchorY=' + anchorY + ')').to.be.true;
+
+ const rect = container.getBoundingClientRect();
+ const onScreen = rect.width > 0 && rect.height > 0
+ && rect.bottom > 0 && rect.top < win.innerHeight
+ && rect.right > 0 && rect.left < win.innerWidth;
+ expect(onScreen,
+ author + ' comment dialog is off-screen'
+ + ' (rect=' + JSON.stringify({top: rect.top, left: rect.left, w: rect.width, h: rect.height})
+ + ' viewport=' + win.innerWidth + 'x' + win.innerHeight + ')').to.be.true;
+ }
+
+ it('Action_GoToComment navigates within the current page', { env: { 'pdf-view': true } }, function() {
+ let aliceId;
+ cy.getFrameWindow().then(function(win) {
+ const alice = findCommentByAuthor(win, 'Alice');
+ aliceId = alice.sectionProperties.data.id;
+ expect(win.app.map._docLayer._selectedPart).to.equal(0);
+ });
+
+ cy.then(function() {
+ cy.getFrameWindow().then(function(win) {
+ cy.stub(win.parent, 'postMessage').as('postMessage');
+ win.postMessage(JSON.stringify({
+ MessageId: 'Action_GoToComment',
+ Values: { Id: aliceId },
+ }), '*');
+ helper.processToIdle(win);
+ });
+ });
+
+ cy.get('@postMessage').should(function(stub) {
+ const calls = stub.getCalls().filter(function(call) {
+ try {
+ const msg = typeof call.args[0] === 'string'
+ ? JSON.parse(call.args[0]) : call.args[0];
+ return msg.MessageId === 'Action_GoToComment_Resp';
+ } catch (e) { return false; }
+ });
+ expect(calls.length, 'Action_GoToComment_Resp was not posted').to.be.greaterThan(0);
+ const resp = typeof calls[0].args[0] === 'string'
+ ? JSON.parse(calls[0].args[0]) : calls[0].args[0];
+ expect(resp.Values.success,
+ 'Action_GoToComment_Resp reported error: ' + resp.Values.errorMsg).to.be.true;
+ });
+
+ cy.getFrameWindow().should(function(win) {
+ expect(win.app.map._docLayer._selectedPart,
+ 'current part unexpectedly changed').to.equal(0);
+ assertCommentShownAndAnchorVisible(win, 'Alice');
+ });
+ });
+
+ it('Action_GoToComment switches page for a comment on another page', { env: { 'pdf-view': true } }, function() {
+ let zoeId;
+ cy.getFrameWindow().then(function(win) {
+ const zoe = findCommentByAuthor(win, 'Zoe');
+ zoeId = zoe.sectionProperties.data.id;
+ expect(win.app.map._docLayer._selectedPart,
+ 'test should start on page 1').to.equal(0);
+ });
+
+ cy.then(function() {
+ cy.getFrameWindow().then(function(win) {
+ win.postMessage(JSON.stringify({
+ MessageId: 'Action_GoToComment',
+ Values: { Id: zoeId },
+ }), '*');
+ helper.processToIdle(win);
+ });
+ });
+
+ cy.getFrameWindow().should(function(win) {
+ expect(win.app.map._docLayer._selectedPart,
+ 'current part did not switch to page 2').to.equal(1);
+ assertCommentShownAndAnchorVisible(win, 'Zoe');
+ });
+ });
+});

["Bayram Çiçek (via cogerrit)"]

browser/src/control/jsdialog/Util.StateChange.ts | 22 ++++
cypress_test/data/desktop/calc/manage_names.fods | 21 ++++
cypress_test/integration_tests/desktop/calc/manage_names_spec.js | 45 ++++++++++
3 files changed, 88 insertions(+)

New commits:
commit cf5dc05744e411af422afcbedb532def28c69fe6
Author: Bayram Çiçek <bayram...@collabora.com>
AuthorDate: Thu Apr 23 11:37:50 2026 +0300
Commit: Caolán McNamara <caolan....@collabora.com>
CommitDate: Tue Apr 28 07:16:58 2026 +0000

Calc a11y: focus on prev/next element in the dialog

if an element has disabled.

Signed-off-by: Bayram Çiçek <bayram...@collabora.com>
Change-Id: Ibab18302356865a46f19f3f5c9575ead9e9a7005
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/1437
Reviewed-by: Caolán McNamara <caolan....@collabora.com>
Tested-by: Caolán McNamara <caolan....@collabora.com>
Tested-by: Jenkins CPCI <rel...@collaboraoffice.com>

diff --git a/browser/src/control/jsdialog/Util.StateChange.ts b/browser/src/control/jsdialog/Util.StateChange.ts
index 513a29905269..68d5d5d8c152 100644
--- a/browser/src/control/jsdialog/Util.StateChange.ts
+++ b/browser/src/control/jsdialog/Util.StateChange.ts
@@ -32,6 +32,25 @@ function onStateChange(element: Element, callback: StateChangeCallback) {
enableObserver.observe(element, { attributeFilter: ['disabled'], attributeOldValue: true });
}

+function moveFocusFromDisabledElement(widget: Element) {
+ const dialog = widget.closest('.jsdialog-window');
+ if (!dialog) return;
+ const next = JSDialog.FindFocusableElement(widget.nextElementSibling, 'next');
+ if (next) {
+ next.focus();
+ return;
+ }
+ const prev = JSDialog.FindFocusableElement(widget.previousElementSibling, 'prev');
+ if (prev) {
+ prev.focus();
+ return;
+ }
+ const focusable = JSDialog.GetFocusableElements(dialog);
+ if (focusable && focusable.length) {
+ focusable[0].focus();
+ }
+}
+
function synchronizeDisabledState(source: Element, targets: Array<Element>) {
const enabledCallback = function (enable: boolean) {
app.layoutingService.appendLayoutingTask(() => {
@@ -40,8 +59,11 @@ function synchronizeDisabledState(source: Element, targets: Array<Element>) {
targets[i].removeAttribute('disabled');
targets[i].removeAttribute('aria-disabled');
} else {
+ const hadFocus = targets[i].contains(document.activeElement);
targets[i].setAttribute('disabled', 'true');
targets[i].setAttribute('aria-disabled', 'true');
+ if (hadFocus)
+ moveFocusFromDisabledElement(source);
}
}
});
diff --git a/cypress_test/data/desktop/calc/manage_names.fods b/cypress_test/data/desktop/calc/manage_names.fods
new file mode 100644
index 000000000000..d9741423e1ac
--- /dev/null
+++ b/cypress_test/data/desktop/calc/manage_names.fods
@@ -0,0 +1,21 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<office:document xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0"
+ xmlns:table="urn:oasis:names:tc:opendocument:xmlns:table:1.0"
+ xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0"
+ xmlns:style="urn:oasis:names:tc:opendocument:xmlns:style:1.0"
+ xmlns:fo="urn:oasis:names:tc:opendocument:xmlns:xsl-fo-compatible:1.0"
+ office:version="1.4" office:mimetype="application/vnd.oasis.opendocument.spreadsheet">
+ <office:body>
+ <office:spreadsheet>
+ <table:table table:name="Sheet1">
+ <table:table-column/>
+ <table:table-row>
+ <table:table-cell office:value-type="string"><text:p>hello</text:p></table:table-cell>
+ </table:table-row>
+ </table:table>
+ <table:named-expressions>
+ <table:named-range table:name="TestRange" table:base-cell-address="$Sheet1.$A$1" table:cell-range-address="$Sheet1.$A$1"/>
+ </table:named-expressions>
+ </office:spreadsheet>
+ </office:body>
+</office:document>
diff --git a/cypress_test/integration_tests/desktop/calc/manage_names_spec.js b/cypress_test/integration_tests/desktop/calc/manage_names_spec.js
new file mode 100644
index 000000000000..54130188c02c
--- /dev/null
+++ b/cypress_test/integration_tests/desktop/calc/manage_names_spec.js
@@ -0,0 +1,45 @@

+/* global describe it cy require beforeEach expect */
+

+var helper = require('../../common/helper');
+
+describe(['tagdesktop'], 'Manage Names dialog focus', function() {
+
+ beforeEach(function() {
+ helper.setupAndLoadDocument('calc/manage_names.fods');
+ cy.viewport(1920, 1080);
+ });
+
+ it('Focus moves to another dialog control when delete button becomes disabled', function() {
+ var win;
+
+ cy.getFrameWindow().then(function(frameWindow) {
+ win = frameWindow;
+ win.app.map.sendUnoCommand('.uno:DefineName');
+ });
+
+ cy.cGet('.ui-dialog[role="dialog"]').should('have.length', 1);
+ cy.then(function() {
+ return helper.processToIdle(win);
+ });
+
+ cy.cGet('#delete-button').should('not.be.disabled');
+
+ cy.cGet('#delete-button').focus();
+ cy.cGet('#delete-button').should('have.focus');
+
+ cy.cGet('#delete-button').click();
+
+ cy.then(function() {
+ return helper.processToIdle(win);
+ });
+
+ cy.cGet('#delete-button').should('be.disabled');
+
+ cy.then(function() {
+ var active = win.document.activeElement;
+ var dialog = win.document.querySelector('.ui-dialog[role="dialog"]');
+ var contains = dialog ? dialog.contains(active) : false;
+ expect(contains).to.be.true;