dotCMS / dotCMS/core

dotAutofocus loses the initial focus race inside PrimeNG dialogs (4 dialogs affected)

Open
#36,824 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

dotCMS : Admin Tools Team : Falcon Type : Task
Dominant language
Java
Stars
970
Forks
486
Avg merge
3d 33m
Merged PRs (30d)
170

Description

Description

The PrimeNG migration made dotAutofocus unreliable inside dialogs, and #36816 fixed only the one dialog that was reported. The same latent defect is very likely present in at least four other dialogs.

Root cause. DotAutofocusDirective (core-web/libs/ui/src/lib/directives/dot-autofocus/dot-autofocus.directive.ts) focuses its host through a fixed setTimeout(..., 100). That timing was written for the legacy, non-animated DotDialogComponent. Since the migration to PrimeNG p-dialog (618fd32060), the dialog defaults focusOnShow to true and, in onAfterEnter(), schedules getFocusableElements(content)[0].focus() using its transition duration (default 150ms). 100ms < 150ms, so PrimeNG always runs last and wins. getFocusableElements walks the DOM in document order and ignores tabindex, so focus lands on whichever element happens to be first in the markup.

The visible symptom is focus appearing on the intended input and then jumping away.

Affected dialogs

Each of these uses <p-dialog> with dotAutofocus on an inner input and no [focusOnShow] override, which is the exact precondition that produced #36816:

  • core-web/apps/dotcms-ui/src/app/portlets/dot-apps/dot-apps-import-export-dialog/dot-apps-import-export-dialog.component.html
  • core-web/apps/dotcms-ui/src/app/portlets/dot-pages/dot-create-page-dialog/dot-create-page-dialog.component.html
  • core-web/apps/dotcms-ui/src/app/portlets/shared/dot-content-types-listing/components/dot-content-type-copy-dialog/dot-content-type-copy-dialog.component.html
  • core-web/apps/dotcms-ui/src/app/portlets/shared/dot-content-types-listing/components/dot-add-to-menu/dot-add-to-menu.component.html

Whether each one visibly misbehaves depends on whether another focusable element precedes the intended input in the DOM. Where the intended input happens to be DOM-first, PrimeNG lands on it anyway and the bug is invisible — but the behavior is accidental, not intended.

Why a shared fix rather than repeating #36816

#36816 solved this per-consumer, by setting focusOnShow to false on that one dialog and moving the focus decision into the component. Repeating that in four more places would spread the same workaround around the app and leave the directive still carrying a timing assumption that no longer holds. The directive is used in ~14 places, so the assumption will keep resurfacing.

Preferred direction, to be confirmed during implementation: stop racing PrimeNG. Either have DotAutofocusDirective cooperate with the dialog lifecycle instead of guessing a delay, or standardize on disabling focusOnShow wherever the app wants to own the initial focus. Both are open to discussion — the point of this issue is to fix the class of bug, not one more instance.

Acceptance criteria

  • Each of the four dialogs above focuses its intended input when opened, and keeps it — no visible jump.
  • The fix is shared, not four copies of the focusOnShow workaround.
  • dotAutofocus no longer depends on out-racing an animation timer, or its remaining uses are documented as safe.
  • Focus trapping and ESC-to-close still work in every touched dialog (focusTrap is an independent PrimeNG input and must stay enabled).
  • Test coverage that fails if PrimeNG reclaims the initial focus. content-types-form-dialog-focus.integration.spec.ts from #36816 is the reference pattern: it renders the real form inside a real p-dialog and includes a test that reproduces the defect when focusOnShow is left on.
  • An audit note listing every remaining dotAutofocus usage and whether it sits inside a p-dialog.

Notes

  • Verified against primeng@21.1.3: focusOnShow default at primeng-dialog.mjs:246, read only in onAfterEnter() (:933-936); focus()/_focus() at :604-628; transitionOptions default '150ms cubic-bezier(0, 0, 0.2, 1)' at :267.
  • focusTrap is a separate code path ([pFocusTrapDisabled]="focusTrap === false"), so disabling focusOnShow does not weaken it.
  • Found while reviewing #36820 (fix for #36816).

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with core-web/libs/ui/src/lib/directives/dot-autofocus/dot-autofocus.directive.ts and inspect the four affected dialog templates listed in the issue. Run or extend content-types-form-dialog-focus.integration.spec.ts to reproduce PrimeNG reclaiming focus, then audit every remaining dotAutofocus usage. Done means all four dialogs retain their intended focus, focus trapping and ESC-to-close still work, and the shared behavior and audit are covered by tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
angular, typescript
Domain
frontend, testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.