From 7ed46ab2258476a35c3c1290dc0e6fbd7ae99aa5 Mon Sep 17 00:00:00 2001 From: Anthony LC Date: Fri, 19 Sep 2025 16:45:40 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=82(frontend)=20block=20editing=20titl?= =?UTF-8?q?e=20when=20not=20allowed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We had a case where the title input was editable even when the user did not have the right to edit it because of websocket problem during collaboration. We fixed this issue by checking the collaboration status before allowing the edition of the title. --- CHANGELOG.md | 12 ++---- .../__tests__/app-impress/doc-editor.spec.ts | 43 ++++++++----------- .../__tests__/app-impress/doc-header.spec.ts | 12 ++++++ .../app-impress/doc-member-create.spec.ts | 4 +- .../app-impress/doc-visibility.spec.ts | 8 ++-- .../e2e/__tests__/app-impress/utils-share.ts | 35 +++++++++------ .../docs/doc-header/components/DocTitle.tsx | 6 ++- 7 files changed, 66 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ce4a5ddf68..db660ee10d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,8 +14,9 @@ and this project adheres to - ♻️(frontend) replace Arial font-family with token font #1411 - ♿(frontend) improve accessibility: - - #1354 - - #1349 + - ♿(frontend) enable enter key to open documentss #1354 + - ♿(frontend) improve modal a11y: structure, labels, title #1349 + - ♿improve NVDA navigation in DocShareModal #1396 - ♿ improve accessibility by adding landmark roles to layout #1394 - ♿ add document visible in list and openable via enter key #1365 - ♿ add pdf outline property to enable bookmarks display #1368 @@ -29,20 +30,15 @@ and this project adheres to - ♿ add h1 for SR on 40X pages and remove alt texts #1438 - ♿ update labels and shared document icon accessibility #1442 - ### Fixed - 🐛(backend) duplicate sub docs as root for reader users - ⚗️(service-worker) remove index from cache first strategy #1395 - 🐛(frontend) fix 404 page when reload 403 page #1402 - 🐛(frontend) fix legacy role computation #1376 +- 🛂(frontend) block editing title when not allowed #1412 - 🐛(frontend) scroll back to top when navigate to a document #1406 -### Changed - -- ♿(frontend) improve accessibility: - - ♿improve NVDA navigation in DocShareModal #1396 - ## [3.7.0] - 2025-09-12 ### Added diff --git a/src/frontend/apps/e2e/__tests__/app-impress/doc-editor.spec.ts b/src/frontend/apps/e2e/__tests__/app-impress/doc-editor.spec.ts index fcb4374d5c..47a318b657 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/doc-editor.spec.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/doc-editor.spec.ts @@ -1,7 +1,7 @@ /* eslint-disable playwright/no-conditional-expect */ import path from 'path'; -import { chromium, expect, test } from '@playwright/test'; +import { expect, test } from '@playwright/test'; import cs from 'convert-stream'; import { @@ -12,6 +12,7 @@ import { verifyDocName, } from './utils-common'; import { getEditor, openSuggestionMenu, writeInEditor } from './utils-editor'; +import { connectOtherUserToDoc, updateShareLink } from './utils-share'; import { createRootSubPage, navigateToPageFromTree } from './utils-sub-pages'; test.beforeEach(async ({ page }) => { @@ -560,20 +561,7 @@ test.describe('Doc Editor', () => { await page.getByRole('button', { name: 'Share' }).click(); - await page.getByTestId('doc-visibility').click(); - - await page - .getByRole('menuitem', { - name: 'Public', - }) - .click(); - - await expect( - page.getByText('The document visibility has been updated.'), - ).toBeVisible(); - - await page.getByTestId('doc-access-mode').click(); - await page.getByRole('menuitem', { name: 'Editing' }).click(); + await updateShareLink(page, 'Public', 'Editing'); // Close the modal await page.getByRole('button', { name: 'close' }).first().click(); @@ -597,17 +585,12 @@ test.describe('Doc Editor', () => { * We open another browser that will connect to the collaborative server * and will block the current browser to edit the doc. */ - const otherBrowser = await chromium.launch({ headless: true }); - const otherContext = await otherBrowser.newContext({ - locale: 'en-US', - timezoneId: 'Europe/Paris', - permissions: [], - storageState: { - cookies: [], - origins: [], - }, + const { otherPage } = await connectOtherUserToDoc({ + browserName, + docUrl: urlChildDoc, + docTitle: childTitle, + withoutSignIn: true, }); - const otherPage = await otherContext.newPage(); const webSocketPromise = otherPage.waitForEvent( 'websocket', @@ -648,6 +631,11 @@ test.describe('Doc Editor', () => { await expect(editor).toHaveAttribute('contenteditable', 'false'); + await expect( + page.getByRole('textbox', { name: 'Document title' }), + ).toBeHidden(); + await expect(page.getByRole('heading', { name: childTitle })).toBeVisible(); + await page.goto(urlParentDoc); await verifyDocName(page, parentTitle); @@ -664,6 +652,11 @@ test.describe('Doc Editor', () => { await expect(editor).toHaveAttribute('contenteditable', 'true'); + await expect( + page.getByRole('textbox', { name: 'Document title' }), + ).toContainText(childTitle); + await expect(page.getByRole('heading', { name: childTitle })).toBeHidden(); + await expect( card.getByText('Others are editing. Your network prevent changes.'), ).toBeHidden(); diff --git a/src/frontend/apps/e2e/__tests__/app-impress/doc-header.spec.ts b/src/frontend/apps/e2e/__tests__/app-impress/doc-header.spec.ts index 5476f1aa34..8e05c5e6c6 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/doc-header.spec.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/doc-header.spec.ts @@ -142,6 +142,10 @@ test.describe('Doc Header', () => { await goToGridDoc(page); + await expect( + page.getByRole('textbox', { name: 'Document title' }), + ).toContainText('Mocked document'); + await expect( page.getByRole('button', { name: 'Export the document' }), ).toBeVisible(); @@ -218,6 +222,10 @@ test.describe('Doc Header', () => { await goToGridDoc(page); + await expect( + page.getByRole('textbox', { name: 'Document title' }), + ).toContainText('Mocked document'); + await expect( page.getByRole('button', { name: 'Export the document' }), ).toBeVisible(); @@ -286,6 +294,10 @@ test.describe('Doc Header', () => { await goToGridDoc(page); + await expect( + page.getByRole('heading', { name: 'Mocked document' }), + ).toBeVisible(); + await expect( page.getByRole('button', { name: 'Export the document' }), ).toBeVisible(); diff --git a/src/frontend/apps/e2e/__tests__/app-impress/doc-member-create.spec.ts b/src/frontend/apps/e2e/__tests__/app-impress/doc-member-create.spec.ts index 724d04ed40..d426ac50d9 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/doc-member-create.spec.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/doc-member-create.spec.ts @@ -229,11 +229,11 @@ test.describe('Document create member', () => { .last() .fill('Hello World'); - const urlDoc = page.url(); + const docUrl = page.url(); // Other user will request access const { otherPage, otherBrowserName, cleanup } = - await connectOtherUserToDoc(browserName, urlDoc); + await connectOtherUserToDoc({ browserName, docUrl }); await expect( otherPage.getByText('Insufficient access rights to view the document.'), diff --git a/src/frontend/apps/e2e/__tests__/app-impress/doc-visibility.spec.ts b/src/frontend/apps/e2e/__tests__/app-impress/doc-visibility.spec.ts index 7027ccc114..0f761c3fd7 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/doc-visibility.spec.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/doc-visibility.spec.ts @@ -157,12 +157,12 @@ test.describe('Doc Visibility: Restricted', () => { .last() .fill('Hello World'); - const urlDoc = page.url(); + const docUrl = page.url(); - const { otherBrowserName, otherPage } = await connectOtherUserToDoc( + const { otherBrowserName, otherPage } = await connectOtherUserToDoc({ browserName, - urlDoc, - ); + docUrl, + }); await expect( otherPage.getByText('Insufficient access rights to view the document.'), diff --git a/src/frontend/apps/e2e/__tests__/app-impress/utils-share.ts b/src/frontend/apps/e2e/__tests__/app-impress/utils-share.ts index 700ad8056b..3adfd034de 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/utils-share.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/utils-share.ts @@ -9,7 +9,7 @@ import { export type Role = 'Administrator' | 'Owner' | 'Member' | 'Editor' | 'Reader'; export type LinkReach = 'Private' | 'Connected' | 'Public'; -export type LinkRole = 'Reading' | 'Edition'; +export type LinkRole = 'Reading' | 'Editing'; export const addNewMember = async ( page: Page, @@ -88,11 +88,17 @@ export const updateRoleUser = async ( * @param docTitle The title of the document (optional). * @returns An object containing the other browser, context, and page. */ -export const connectOtherUserToDoc = async ( - browserName: BrowserName, - docUrl: string, - docTitle?: string, -) => { +export const connectOtherUserToDoc = async ({ + browserName, + docUrl, + docTitle, + withoutSignIn, +}: { + browserName: BrowserName; + docUrl: string; + docTitle?: string; + withoutSignIn?: boolean; +}) => { const otherBrowserName = BROWSERS.find((b) => b !== browserName); if (!otherBrowserName) { throw new Error('No alternative browser found'); @@ -111,15 +117,16 @@ export const connectOtherUserToDoc = async ( const otherPage = await otherContext.newPage(); await otherPage.goto(docUrl); - await otherPage - .getByRole('main', { name: 'Main content' }) - .getByLabel('Login') - .click({ - timeout: 15000, - }); - - await keyCloakSignIn(otherPage, otherBrowserName, false); + if (!withoutSignIn) { + await otherPage + .getByRole('main', { name: 'Main content' }) + .getByLabel('Login') + .click({ + timeout: 15000, + }); + await keyCloakSignIn(otherPage, otherBrowserName, false); + } if (docTitle) { await verifyDocName(otherPage, docTitle); } diff --git a/src/frontend/apps/impress/src/features/docs/doc-header/components/DocTitle.tsx b/src/frontend/apps/impress/src/features/docs/doc-header/components/DocTitle.tsx index c1e9fb2449..42fea42895 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-header/components/DocTitle.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-header/components/DocTitle.tsx @@ -11,6 +11,7 @@ import { KEY_DOC, KEY_LIST_DOC, useDocStore, + useIsCollaborativeEditable, useTrans, useUpdateDoc, } from '@/docs/doc-management'; @@ -21,7 +22,10 @@ interface DocTitleProps { } export const DocTitle = ({ doc }: DocTitleProps) => { - if (!doc.abilities.partial_update) { + const { isEditable, isLoading } = useIsCollaborativeEditable(doc); + const readOnly = !doc.abilities.partial_update || !isEditable || isLoading; + + if (readOnly) { return ; }