fix: remove ProjectComments innerHTML usage (#6522)

[Atulsingh1155]

---

Your checklist for this pull request

Please review the contributing guidelines and wiki pages of this repository.

• Include the name and id of the Jira ticket in the PR’s title eg.: SHARE-666 The devils ticket
• Choose the proper base branch (develop)
• Confirm that the changes follow the project’s coding guidelines
• Verify that the changes generate no warnings and errors
• Verify to commit no other files than the intentionally changed ones
• Include reasonable and readable tests verifying the added or changed behavior
• Verify that all tests are passing (CI), if not please state the test cases in the section below
• Perform a self-review of the changes
• Stick to the project’s git workflow (rebase and squash your commits)
• Verify that your changes do not have any conflicts with the base branch
• Put your ticket into the Code Review section in Jira
• Ask for a code reviewer
• Check that your pull request has been successfully deployed to https://web-test-1.catrob.at/

Additional Description

TODO: Add additional information that is not in your commit-message here

[Atulsingh1155]

Please review this PR

[Atulsingh1155]

@dmetzner please review this pr

[Atulsingh1155]

@dmetzner please review this pr?

[dmetzner]

please make sure alle pipelines are green.

[Atulsingh1155]

please make sure alle pipelines are green.

Okay

[Atulsingh1155]

@dmetzner please review this PR... icorrected all the isues

[dmetzner]

Thanks for tackling the innerHTML removal — this is a good security improvement. A few things need fixing before this can merge:

Must Fix

1. Don't hand-edit auto-generated files

src/Api/OpenAPI/Server/Model/CommentResponse.php is auto-generated by yarn run generate-api. Hand-editing it will be overwritten on the next generation.

Fix: Add user_approved to CommentResponse in src/Api/OpenAPI/specification.yaml, then run:

yarn run generate-api
bin/rector process src/Api/OpenAPI/Server/
bin/php-cs-fixer fix src/Api/OpenAPI/Server/ --using-cache=no

Also update the spec to remove the rendered field (or mark it deprecated) since we're no longer populating it.

2. innerHTML still used in createLoadingSpinner()

The PR's goal is to remove innerHTML usage, but createLoadingSpinner() (around line 438) still uses spinnerWrapper.innerHTML = ... with a large HTML string. While this is static content (no user data = no XSS risk), it's inconsistent with the PR's goal. Either:

• Convert to DOM API for consistency, or
• Add a comment explaining why innerHTML is acceptable here (static SVG, no user data)

3. rendered field set to empty string is wasteful

$response->setRendered('');

This sends an empty "rendered": "" in every API response forever. Instead:

• Remove rendered from the OpenAPI spec entirely (breaking change but the right move), or
• Set it to null and mark as deprecated in the spec

Should Fix

4. Missing Behat test updates

Comments are now rendered client-side from API data. Many web-comments Behat tests may need I wait for AJAX to finish steps added (similar to what you did for language_switcher.feature). Otherwise tests could be flaky or fail because they check for elements before AJAX completes.

5. Report button condition review

if (!isDeleted && !isApproved && (isAdmin || !isOwnComment || !isLoggedIn))

The !isLoggedIn means non-logged-in users see report buttons. Is this intentional? If clicking redirects to login, that's fine, but please confirm this matches the original Twig template behavior.

Nitpick

The createCommentElement function is ~200 lines of imperative DOM creation. Consider extracting a small helper like createElement(tag, className, textContent) to reduce repetition. Not blocking, but would improve readability significantly.

---

Overall direction is good — removing Twig rendering from the API response is the right architectural move. The main blocker is fixing the auto-generated file edit via the OpenAPI spec workflow.

[dmetzner]

Scope Concern — PR is too large for the issue

Issue #6522 identifies 2 specific innerHTML lines (312 and 395) that use data.rendered from the API. The suggested fix was either:

1. Build DOM client-side with escapeHtml(), or
2. Audit and document the security contract

This PR goes much further: it removes server-side Twig rendering entirely, rewrites the full comment rendering as 200+ lines of imperative DOM construction, modifies auto-generated API model files, adds new data attributes, and changes the API response shape. That's a full client-side migration of the comment system — a much bigger scope than fixing the innerHTML XSS risk.

Simpler alternative (2-3 line fix)

Replace innerHTML with DOMParser which safely parses HTML without executing scripts:

// Before (XSS risk):
tempDiv.innerHTML = data.rendered
const newComment = tempDiv.firstChild

// After (safe):
const doc = new DOMParser().parseFromString(data.rendered, 'text/html')
const newComment = doc.body.firstChild

This eliminates the innerHTML XSS vector while keeping server-rendered HTML. 2 changes, same result, zero risk of breaking comment rendering.

If the full client-side migration IS desired

That's fine architecturally, but it should be a separate ticket (like the other CSR migrations tracked in #6580-#6584). The current PR has several issues that would need fixing first:

1. Hand-edited auto-generated file (CommentResponse.php) — must go through specification.yaml + yarn run generate-api
2. innerHTML still used in createLoadingSpinner() — inconsistent with PR goal
3. rendered field set to empty string — wasteful, should be removed from spec or set to null
4. Missing Behat test AJAX waits — client-side rendering needs wait steps in web-comments tests
5. 200-line createCommentElement — hard to review/maintain; consider helpers

Recommendation

Either scope this PR down to the DOMParser fix (matching the issue), or split into two PRs: the quick DOMParser fix for #6522, and a separate CSR migration PR with proper OpenAPI spec changes.

[Atulsingh1155]

Okay i will fix everything

[Atulsingh1155]

@dmetzner is this fine now?