From cd9ddac45910ebc73e95e4519f59f6e8d51ed5eb Mon Sep 17 00:00:00 2001 From: 0ko <0ko@noreply.codeberg.org> Date: Thu, 5 Mar 2026 22:01:40 +0100 Subject: [PATCH] [v14.0/forgejo] fix(ui/mde): inputs in table/link insertion modals (#11499) Backport: #11341 Trivial conflict resolution: commit contained changes to org-members.test.e2e not directly related to the fix. The file is exclusive to v15, so these changes were dropped in the backport. --- Fixes #11268 Fixes regression of #9614 Calling `initDisabledInputs` wasn't effective for template contents, so inputs in MDEs spawned by repo-legacy.js on comment editing were broken. Now repo-legacy.js also calls it when it spawns a new MDE. Co-authored-by: Gusted Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11341 Reviewed-by: Gusted Co-authored-by: 0ko <0ko@noreply.codeberg.org> Co-committed-by: 0ko <0ko@noreply.codeberg.org> (cherry picked from commit a0faae27645d0111c940abe6137f2ce2ff95fb52) Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11499 Reviewed-by: Michael Kriese Reviewed-by: Gusted --- tests/e2e/issue-comment.test.e2e.ts | 14 ++- tests/e2e/markdown-editor.test.e2e.ts | 132 ++++++++++++++++++-------- tests/e2e/switch.test.e2e.ts | 40 +++++--- web_src/js/features/common-global.js | 8 +- web_src/js/features/repo-legacy.js | 3 +- web_src/js/index.js | 2 +- 6 files changed, 134 insertions(+), 65 deletions(-) diff --git a/tests/e2e/issue-comment.test.e2e.ts b/tests/e2e/issue-comment.test.e2e.ts index 9477c7e210..7ffb4146a5 100644 --- a/tests/e2e/issue-comment.test.e2e.ts +++ b/tests/e2e/issue-comment.test.e2e.ts @@ -1,3 +1,6 @@ +// Copyright 2024 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + // @watch start // web_src/js/features/comp/** // web_src/js/features/repo-** @@ -108,17 +111,18 @@ test.describe('Button text replaced by JS', () => { await commentField.fill('Blah blah'); await expect(statusButton.getByText('Reopen with comment')).toBeVisible(); await expect(statusButtonIcon).toBeVisible(); - - return true; } test('Issue', async ({page}) => { - // All actual expect() are happening in the helper - expect(await testPage(page, '/user2/repo2/issues/2', 'Close issue')).toBeTruthy(); + await expect(async () => { + await testPage(page, '/user2/repo2/issues/2', 'Close issue'); + }).toPass(); }); test('PR', async ({page}) => { - expect(await testPage(page, '/user2/repo1/pulls/5', 'Close pull request')).toBeTruthy(); + await expect(async () => { + await testPage(page, '/user2/repo1/pulls/5', 'Close pull request'); + }).toPass(); }); }); diff --git a/tests/e2e/markdown-editor.test.e2e.ts b/tests/e2e/markdown-editor.test.e2e.ts index cc8a7be2d1..5be8c1ed67 100644 --- a/tests/e2e/markdown-editor.test.e2e.ts +++ b/tests/e2e/markdown-editor.test.e2e.ts @@ -1,3 +1,6 @@ +// Copyright 2024 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + // @watch start // web_src/js/modules/tab.ts // web_src/css/modules/tab.css @@ -6,7 +9,7 @@ // templates/shared/combomarkdowneditor.tmpl // @watch end -import {expect} from '@playwright/test'; +import {expect, type Page} from '@playwright/test'; import {accessibilityCheck} from './shared/accessibility.ts'; import {test} from './utils_e2e.ts'; import {screenshot} from './shared/screenshots.ts'; @@ -349,53 +352,100 @@ test('Markdown list continuation', async ({page}) => { }); test('Markdown insert table', async ({page}) => { - const response = await page.goto('/user2/repo1/issues/new'); + async function evaluateTableInsertion(page: Page, selector: string, isEditing: boolean) { + const area = page.locator(selector); + + let expectedContent = '| Header | Header |\n|---------|---------|\n| Content | Content |\n| Content | Content |\n| Content | Content |\n'; + + if (isEditing) { + // Preparations for evaluating comment editing + await area.locator('.context-dropdown').click(); + await area.locator('.context-dropdown .edit-content').click(); + expectedContent = `good work!${expectedContent}`; + } + + const newTableButton = area.locator('button[data-md-action="new-table"]'); + await newTableButton.click(); + + const newTableModal = page.locator('[data-modal-name="new-markdown-table"].active'); + await expect(newTableModal).toBeVisible(); + await screenshot(page); + + const rowsInput = newTableModal.locator('input[name="table-rows"]'); + const columnsInput = newTableModal.locator('input[name="table-columns"]'); + + await expect(rowsInput).not.toHaveAttribute('disabled'); + await expect(columnsInput).not.toHaveAttribute('disabled'); + + await rowsInput.fill('3'); + await columnsInput.fill('2'); + + await newTableModal.locator('button[data-selector-name="ok-button"]').click(); + + await expect(newTableModal).toBeHidden(); + + const textarea = area.locator('textarea[name=content]'); + await expect(textarea).toHaveValue(expectedContent); + await screenshot(page); + } + + const response = await page.goto('/user2/repo1/issues/1'); expect(response?.status()).toBe(200); - const newTableButton = page.locator('button[data-md-action="new-table"]'); - await newTableButton.click(); - - const newTableModal = page.locator('div[data-markdown-table-modal-id="0"]'); - await expect(newTableModal).toBeVisible(); - await screenshot(page); - - await newTableModal.locator('input[name="table-rows"]').fill('3'); - await newTableModal.locator('input[name="table-columns"]').fill('2'); - - await newTableModal.locator('button[data-selector-name="ok-button"]').click(); - - await expect(newTableModal).toBeHidden(); - - const textarea = page.locator('textarea[name=content]'); - await expect(textarea).toHaveValue('| Header | Header |\n|---------|---------|\n| Content | Content |\n| Content | Content |\n| Content | Content |\n'); - await screenshot(page); + await expect(async () => { + await evaluateTableInsertion(page, '#comment-form', false); + await evaluateTableInsertion(page, '#issuecomment-2', true); + }).toPass(); }); test('Markdown insert link', async ({page}) => { - const response = await page.goto('/user2/repo1/issues/new'); + async function evaluateLinkInsertion(page: Page, selector: string, isEditing: boolean) { + const url = 'https://example.com'; + const description = 'Where does this lead?'; + + let expectedContent = `[${description}](${url})`; + + const area = page.locator(selector); + + if (isEditing) { + // Preparations for evaluating comment editing + await area.locator('.context-dropdown').click(); + await area.locator('.context-dropdown .edit-content').click(); + expectedContent = `good work!${expectedContent}`; + } + + const newLinkButton = area.locator('button[data-md-action="new-link"]'); + await newLinkButton.click(); + + const newLinkModal = page.locator('[data-modal-name="new-markdown-link"].active'); + await expect(newLinkModal).toBeVisible(); + await accessibilityCheck({page}, ['[data-modal-name="new-markdown-link"].active'], [], []); + await screenshot(page); + + const urlInput = newLinkModal.locator('input[name="link-url"]'); + const descriptionInput = newLinkModal.locator('input[name="link-description"]'); + + await expect(urlInput).not.toHaveAttribute('disabled'); + await expect(descriptionInput).not.toHaveAttribute('disabled'); + + await urlInput.fill(url); + await descriptionInput.fill(description); + + await newLinkModal.locator('button[data-selector-name="ok-button"]').click(); + await expect(newLinkModal).toBeHidden(); + + const textarea = area.locator('textarea[name=content]'); + await expect(textarea).toHaveValue(expectedContent); + await screenshot(page); + } + + const response = await page.goto('/user2/repo1/issues/1'); expect(response?.status()).toBe(200); - const newLinkButton = page.locator('button[data-md-action="new-link"]'); - await newLinkButton.click(); - - const newLinkModal = page.locator('div[data-markdown-link-modal-id="0"]'); - await expect(newLinkModal).toBeVisible(); - await accessibilityCheck({page}, ['[data-modal-name="new-markdown-link"]'], [], []); - await screenshot(page); - - const url = 'https://example.com'; - const description = 'Where does this lead?'; - - await newLinkModal.locator('input[name="link-url"]').fill(url); - await newLinkModal.locator('input[name="link-description"]').fill(description); - - await newLinkModal.locator('button[data-selector-name="ok-button"]').click(); - - await expect(newLinkModal).toBeHidden(); - - const textarea = page.locator('textarea[name=content]'); - await expect(textarea).toHaveValue(`[${description}](${url})`); - await screenshot(page); + await expect(async () => { + await evaluateLinkInsertion(page, '#comment-form', false); + await evaluateLinkInsertion(page, '#issuecomment-2', true); + }).toPass(); }); test('text expander has higher prio then prefix continuation', async ({page}) => { diff --git a/tests/e2e/switch.test.e2e.ts b/tests/e2e/switch.test.e2e.ts index bc4922d198..8489ddb467 100644 --- a/tests/e2e/switch.test.e2e.ts +++ b/tests/e2e/switch.test.e2e.ts @@ -41,8 +41,6 @@ test.describe('Switch CSS properties', () => { } expect((await item.boundingBox()).height).toBeCloseTo(itemHeight); - - return true; } const normalMargin = '0px'; @@ -60,21 +58,33 @@ test.describe('Switch CSS properties', () => { await page.goto('/user2/repo1/pulls'); - expect(await evaluateSwitchItem(page, '#issue-filters .switch > .item:nth-child(1)', true, normalMargin, normalMargin, normalPadding, normalPadding, itemHeight)).toBeTruthy(); - expect(await evaluateSwitchItem(page, '#issue-filters .switch > .item:nth-child(2)', false, specialLeftMargin, normalMargin, specialPadding, normalPadding, itemHeight)).toBeTruthy(); - expect(await evaluateSwitchItem(page, '#issue-filters .switch > .item:nth-child(3)', false, normalMargin, normalMargin, normalPadding, normalPadding, itemHeight)).toBeTruthy(); + await expect(async () => { + await Promise.all([ + evaluateSwitchItem(page, '#issue-filters .switch > .item:nth-child(1)', true, normalMargin, normalMargin, normalPadding, normalPadding, itemHeight), + evaluateSwitchItem(page, '#issue-filters .switch > .item:nth-child(2)', false, specialLeftMargin, normalMargin, specialPadding, normalPadding, itemHeight), + evaluateSwitchItem(page, '#issue-filters .switch > .item:nth-child(3)', false, normalMargin, normalMargin, normalPadding, normalPadding, itemHeight), + ]); + }).toPass(); await page.goto('/user2/repo1/pulls?state=closed'); - expect(await evaluateSwitchItem(page, '#issue-filters .switch > .item:nth-child(1)', false, normalMargin, specialLeftMargin, normalPadding, specialPadding, itemHeight)).toBeTruthy(); - expect(await evaluateSwitchItem(page, '#issue-filters .switch > .item:nth-child(2)', true, normalMargin, normalMargin, normalPadding, normalPadding, itemHeight)).toBeTruthy(); - expect(await evaluateSwitchItem(page, '#issue-filters .switch > .item:nth-child(3)', false, specialLeftMargin, normalMargin, specialPadding, normalPadding, itemHeight)).toBeTruthy(); + await expect(async () => { + await Promise.all([ + evaluateSwitchItem(page, '#issue-filters .switch > .item:nth-child(1)', false, normalMargin, specialLeftMargin, normalPadding, specialPadding, itemHeight), + evaluateSwitchItem(page, '#issue-filters .switch > .item:nth-child(2)', true, normalMargin, normalMargin, normalPadding, normalPadding, itemHeight), + evaluateSwitchItem(page, '#issue-filters .switch > .item:nth-child(3)', false, specialLeftMargin, normalMargin, specialPadding, normalPadding, itemHeight), + ]); + }).toPass(); await page.goto('/user2/repo1/pulls?state=all'); - expect(await evaluateSwitchItem(page, '#issue-filters .switch > .item:nth-child(1)', false, normalMargin, normalMargin, normalPadding, normalPadding, itemHeight)).toBeTruthy(); - expect(await evaluateSwitchItem(page, '#issue-filters .switch > .item:nth-child(2)', false, normalMargin, specialLeftMargin, normalPadding, specialPadding, itemHeight)).toBeTruthy(); - expect(await evaluateSwitchItem(page, '#issue-filters .switch > .item:nth-child(3)', true, normalMargin, normalMargin, normalPadding, normalPadding, itemHeight)).toBeTruthy(); + await expect(async () => { + await Promise.all([ + evaluateSwitchItem(page, '#issue-filters .switch > .item:nth-child(1)', false, normalMargin, normalMargin, normalPadding, normalPadding, itemHeight), + evaluateSwitchItem(page, '#issue-filters .switch > .item:nth-child(2)', false, normalMargin, specialLeftMargin, normalPadding, specialPadding, itemHeight), + evaluateSwitchItem(page, '#issue-filters .switch > .item:nth-child(3)', true, normalMargin, normalMargin, normalPadding, normalPadding, itemHeight), + ]); + }).toPass(); }); // Subtest for areas that can't be reached without JS @@ -91,7 +101,11 @@ test.describe('Switch CSS properties', () => { // Markdown editor has a special rule for a shorter switch const itemHeight = 28; - expect(await evaluateSwitchItem(page, '.review-box-panel .switch > .item:nth-child(1)', true, normalMargin, normalMargin, normalPadding, normalPadding, itemHeight)).toBeTruthy(); - expect(await evaluateSwitchItem(page, '.review-box-panel .switch > .item:nth-child(2)', false, specialLeftMargin, normalMargin, specialPadding, normalPadding, itemHeight)).toBeTruthy(); + await expect(async () => { + await Promise.all([ + evaluateSwitchItem(page, '.review-box-panel .switch > .item:nth-child(1)', true, normalMargin, normalMargin, normalPadding, normalPadding, itemHeight), + evaluateSwitchItem(page, '.review-box-panel .switch > .item:nth-child(2)', false, specialLeftMargin, normalMargin, specialPadding, normalPadding, itemHeight), + ]); + }).toPass(); }); }); diff --git a/web_src/js/features/common-global.js b/web_src/js/features/common-global.js index 25df43a636..93a970caa0 100644 --- a/web_src/js/features/common-global.js +++ b/web_src/js/features/common-global.js @@ -206,10 +206,10 @@ export function initGlobalCommon() { } // Sometimes unrelated inputs are stored in forms for convenience, for example, -// modal inputs. To prevent them from breaking the forms they are in they are -// disabled by default -export function initDisabledInputs() { - for (const el of document.querySelectorAll('input.js-enable[disabled]')) { +// modal inputs. To prevent them from blocking the forms for noJS clients they +// are disabled by default. TypeScript: root is HTMLElement +export function initDisabledInputs(root) { + for (const el of root.querySelectorAll('input.js-enable[disabled]')) { el.removeAttribute('disabled'); } } diff --git a/web_src/js/features/repo-legacy.js b/web_src/js/features/repo-legacy.js index 05766c6c78..4d2f25148c 100644 --- a/web_src/js/features/repo-legacy.js +++ b/web_src/js/features/repo-legacy.js @@ -27,7 +27,7 @@ import {attachRefIssueContextPopup} from './contextpopup.js'; import {POST} from '../modules/fetch.js'; import {MarkdownQuote} from '@github/quote-selection'; import {toAbsoluteUrl} from '../utils.js'; -import {initDropzone, initGlobalShowModal} from './common-global.js'; +import {initDropzone, initGlobalShowModal, initDisabledInputs} from './common-global.js'; export function initRepoCommentForm() { const $commentForm = $('.comment.form'); @@ -370,6 +370,7 @@ async function onEditContent(event) { comboMarkdownEditor = getComboMarkdownEditor(editContentZone.querySelector('.combo-markdown-editor')); if (!comboMarkdownEditor) { editContentZone.innerHTML = document.getElementById('issue-comment-editor-template').innerHTML; + initDisabledInputs(editContentZone); const dropzone = editContentZone.querySelector('.dropzone'); if (!dropzone.dropzone) await initDropzone(dropzone, editContentZone); comboMarkdownEditor = await initComboMarkdownEditor(editContentZone.querySelector('.combo-markdown-editor')); diff --git a/web_src/js/index.js b/web_src/js/index.js index 8701385e92..e2e313aab4 100644 --- a/web_src/js/index.js +++ b/web_src/js/index.js @@ -96,7 +96,7 @@ initDirAuto(); onDomReady(() => { initGlobalCommon(); - initDisabledInputs(); + initDisabledInputs(document); initGlobalTooltips(); initGlobalButtonClickOnEnter(); initGlobalButtons();