Skip to content

feat(Block Editor): Restore internal dotCMS Page search in the link popover#35907

Merged
rjvelazco merged 7 commits into
mainfrom
issue-35890-block-editor-20-restore-internal-dotcms-page-search-in-the-link-popover
Jun 3, 2026
Merged

feat(Block Editor): Restore internal dotCMS Page search in the link popover#35907
rjvelazco merged 7 commits into
mainfrom
issue-35890-block-editor-20-restore-internal-dotcms-page-search-in-the-link-popover

Conversation

@rjvelazco

@rjvelazco rjvelazco commented Jun 2, 2026

Copy link
Copy Markdown
Member

This PR fixes: #35890

Proposed Changes

Restores feature parity with the legacy Block Editor by bringing internal dotCMS page search back to the Block Editor 2.0 link popover.

  • Page search in the URL field — replaced the plain URL text input with a PrimeNG p-autoComplete. Typing a partial title/path runs a debounced (250 ms) search and shows a dropdown of matching pages.
  • Reuses existing data-access — the search goes through DotContentSearchService from @dotcms/data-access (the same service the slash-menu drill-down uses); no new HTTP service. The Lucene query is built inline at the call site, mirroring the legacy editor (basetype:5 pages + URL-mapped content, matched by title/path/urlmap prefix).
  • Custom result template — each suggestion shows the contentlet name and its path, plus a thumbnail (/dA/{inode}) when available, with ellipsis truncation to fit the popover.
  • External URLs never blocked — when the value parses as a valid http(s) URL the overlay is suppressed entirely (no search, no empty message), so pasting/typing an external link works unchanged. Manual entry is always allowed.
  • Empty state — shows "No results found in dotCMS" when a search returns nothing.
  • Works in create and edit flows — selecting a result populates the link href; opening an existing link prefills the field without firing a spurious search.
  • Selection highlight while editing — in insert mode, focusing the URL input no longer hides which text will become the link: the target range is painted with a ProseMirror decoration that survives the input blur and is cleared when the popover closes.
  • Popover shell fix — whitelisted .p-autocomplete-overlay in the popover's click-outside handler so picking a suggestion (body-appended overlay) doesn't dismiss the popover.
  • rel select fix — the rel control now defaults to null instead of '', so PrimeNG's [showClear] no longer renders a stray "X" on the empty dropdown.

Checklist

  • Translations — added dot.block.editor.dialog.link.search.empty and updated the URL placeholder in Language.properties.
  • Security Implications Contemplated — no new endpoint; reuses the existing /api/content/_search. Query-term interpolation matches the legacy popover's behavior (search scoped to working/non-deleted content for the active language).

Additional Info

The search-result set (pages via basetype:5 plus URL-mapped content, scoped to the active languageId, working + non-deleted) reproduces the legacy link popover's behavior.

Screenshots

video.mov

This PR fixes: #35890

@github-actions github-actions Bot added Area : Backend PR changes Java/Maven backend code Area : Frontend PR changes Angular/TypeScript frontend code labels Jun 2, 2026
@claude

claude Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Claude finished @rjvelazco's task in 4m 23s —— View job


Claude PR Review

  • Read changed files
  • Analyze logic, edge cases, security, design
  • Post review findings

Status since prior review

Prior finding Status
🔴 size="'small'" literal string Fixed — now [size]="'small'" (correct Angular property binding)
🟠 Lucene injection Fixed#escapeLuceneTerm escapes all reserved chars
🟠 url: c.path || c.urlMap can be undefined Fixed — falls back to '' and .filter((r) => !!r.url) removes blanks
🟢 rel: null default Fixed (prior cycle)
🟡 Mutating PrimeNG private fields Not addressed — now has a pinning comment, still brittle
🟡 No host filter (+conhost:) Not addressed
🟡 +working:true vs +live:true Not addressed
🟡 No unit tests Not addressed

Remaining findings

🟠 Multi-word search terms silently mis-parse (new finding)

link-popover.component.ts:339

return `... +(title:${safe}* OR path:*${safe}* OR urlmap:*${safe}*)`;

#escapeLuceneTerm does not escape or quote spaces. Typing "about us" produces title:about us*, which Lucene query_string parses as two independent terms: title:about and an unfielded us*. The unfielded wildcard matches any field and makes results unpredictable. Wrap the term in escaped quotes: title:"${safe}" (phrase match, no trailing wildcard needed for prefix matching).

Fix this →


🟠 Leading wildcards in path: and urlmap: branches

link-popover.component.ts:339

+(title:${safe}* OR path:*${safe}* OR urlmap:*${safe}*)

path:*${safe}* and urlmap:*${safe}* use a leading wildcard, which Lucene/ES rejects unless AllowLeadingWildcard is explicitly enabled (it's off by default). If disabled, these branches silently drop, so only title:${safe}* matches — a title-only prefix search, not the path/urlmap search the query intends. Verify the production ES config permits leading wildcards; if not, consider path:${safe}* (prefix only, no leading wildcard).


🟡 PrimeNG private API — now documented, still fragile

link-popover.component.ts:299-303

ac.loading = false;
ac.overlayVisible = false;
ac.cd.markForCheck();

The added comment calls this out and pins the PrimeNG version — that's the right move. Just confirm the comment makes it into the upgrade checklist so it isn't lost.


🟡 No host filter on internal page search

Query has no +conhost: constraint. On multi-site installs, pages from other sites appear in the dropdown. If the legacy editor scoped to the current site, this is a regression; if not, it's parity. Confirm intent.


🟡 +working:true exposes unpublished pages

Selecting a page that hasn't gone live yet produces a link that 404s for end-users until publish. +live:true would scope to published pages only. Both choices are defensible — flag for product confirmation.


🟡 No unit tests

#buildPageSearchQuery, onComplete (external URL short-circuit, debounce path), onSelectPage, and the prefill-without-search path are all untested. The core-web/CLAUDE.md requires Jest + Spectator. These don't block the PR, but they leave the debounce and overlay-hide logic completely uncovered.


🟢 Non-substantive observation

editor-popover.component.ts change is a pure line-wrap reformat (no logic change) — picked up by nx format:write. Fine.
· Branch

@rjvelazco

Copy link
Copy Markdown
Member Author

@claude review these changes

@claude

claude Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Claude finished @rjvelazco's task in 1m 37s —— View job


Claude PR Review (re-review)

  • Read changed files
  • Diff against prior review (commits a67863c + 657bf4b added since)
  • Analyze logic, edge cases, security, design
  • Post review findings

Status since prior review

Prior finding Status
🔴 size="'small'" literal string Still broken — commit a67863c2 set this value but with the same wrong syntax
🟢 rel: null default Fixed in commit 657bf4bd
🟠 Lucene injection Not addressed
🟠 Leading wildcards (*${term}*) Not addressed
🟠 url: c.path || c.urlMap can be undefined Not addressed
🟡 Mutating PrimeNG private fields (ac.loading, ac.overlayVisible, ac.cd) Not addressed
🟡 No host filter (+conhost:) Not addressed
🟡 +working:true vs +live:true Not addressed
🟡 No unit tests Not addressed
🟢 .claude/settings.json reformat noise Still in diff

Remaining findings

🔴 Bug — size="'small'" is still a literal string (commit a67863c2 did not fix it)

link-popover.component.html:28

<p-autoComplete
    inputId="link-url"
    appendTo="body"
    optionLabel="url"
    size="'small'"         literal value 'small' (with the single quotes)

The fix commit attempted to set size="small" but landed size="'small'". PrimeNG's size input accepts "small" / "large" — not "'small'". As written, AutoComplete falls back to the default size and remains visually inconsistent with the sibling controls. Compare with link-popover.component.html:159, where <p-select [size]="'small'"> is correctly bound. Use either size="small" (string attribute) or [size]="'small'" (binding).

Fix this →


🟠 Lucene query — unescaped user input + leading wildcards (unchanged)

link-popover.component.ts:321

return `+languageId:${languageId || 1} +deleted:false +working:true +(urlmap:* OR basetype:5) +(title:${term}* OR path:*${term}* OR urlmap:*${term}*)`;

Two compounding issues:

  1. term is interpolated raw. Reserved Lucene chars (+ - && || ! ( ) { } [ ] ^ " ~ * ? : \ /) will either change the query semantics or break parsing. catchError swallows the failure, so the user just sees an empty dropdown the moment they type : (extremely likely — they're searching for URLs). Escape the term (or strip non-alphanumerics) before composing.

  2. Leading wildcards (path:*${term}*, urlmap:*${term}*) are rejected by Lucene unless AllowLeadingWildcard is enabled, and are very expensive when allowed. Worth verifying the production ES config permits them — otherwise the substring branches silently degrade to the title: prefix branch only.


🟠 url: c.path \|\| c.urlMap can yield undefined

link-popover.component.ts:190

url: c.path || c.urlMap,

Both fields come from the [key: string]: any index. If neither is populated, the suggestion's url is undefined, which:

  • Renders as the literal string "undefined" under optionLabel="url" (visible in the suggestion list).
  • Gets written into the link href on selection.

Guard the mapping: skip the contentlet, fall back to c.url/c.identifier, or at minimum ?? '' and short-circuit if empty.


🟡 Reaching into PrimeNG internals (onComplete external-URL branch)

link-popover.component.ts:290-295

const ac = this.autoComplete();
if (ac) {
    ac.loading = false;
    ac.overlayVisible = false;
    ac.cd.markForCheck();
}

Mutating loading / overlayVisible / cd is brittle across PrimeNG upgrades — none of these are public API. A cleaner alternative: gate [suggestions] upstream so PrimeNG never opens the overlay for an external URL, or short-circuit in completeMethod before AutoComplete opens. At minimum, add a comment pinning the assumed PrimeNG version and surface this in the upgrade checklist.


🟡 No host filter on internal page search

The Lucene query has no +conhost: constraint. On multi-site installs, authors editing site A will see pages from sites B/C/D mixed into the dropdown. Confirm legacy behavior — if it scoped to the current site, this is a regression.


🟡 +working:true lets authors pick unpublished pages

Authors can select a page that isn't live yet, producing a link that 404s for end users until the page goes live. Intentional? +live:true would scope to published pages. Both choices have trade-offs — flag for owner confirmation.


🟡 No unit tests

200 lines of new behavior — debounce, external-URL short-circuit, sync between linkModelform.controls.href, edit-mode prefill without firing a search, popover close cleanup — and zero coverage. core-web/CLAUDE.md mandates Jest + Spectator. Minimum cases worth covering:

  • typing → DotContentSearchService.get called with built query, suggestions populated
  • typing a valid http(s) URL → no service call, overlay forced closed
  • selecting a suggestion → href control updated synchronously before onInsert
  • prefilling in edit mode → no search fires (emitEvent:false path)
  • popover close → suggestions / linkModel / isExternalUrl all reset

🟢 Other observations (non-blocking)

  • editor-popover.component.ts:145 allow-list extension to .p-autocomplete-overlay is correct and necessary — without it, clicking a body-portaled suggestion would dismiss the popover.
  • rel: null change in link-popover.component.ts:149 correctly aligns the initial control state with the nullable type and [showClear] behavior. ✅
  • core-web/.claude/settings.json is a pure reformat (4→2 space indent + key reorder). No behavior change — reads as accidental editor formatting noise. Consider reverting to keep the PR focused on the feature.
  • linkModel (signal) ↔ form.controls.href (reactive form) is two sources of truth kept in sync via an ngModelChange handler. Works, but a ControlValueAccessor-backed wrapper around p-autoComplete would let the reactive form be the single source. Not blocking.
  • effect((onCleanup) => { ... if (this.manager.linkPayload()?.linkEl) return; ... }) at link-popover.component.ts:260-268 runs on every manager.linkPayload() change. If the payload mutates while the popover is open (e.g. linkEl resolves after first open), the decoration will re-toggle. Not currently a real risk, but worth a sanity check.
  • The new search executes off this.#store.languageId() read non-reactively inside switchMap. If the editor's language changes while the popover is open, the next query will use the new value — which is probably correct, but flag it for the author to confirm.

· Branch

- Escape Lucene/ES reserved chars in the search term so typing ':' or '/'
  (common in URLs) no longer breaks the query into an empty dropdown
- Guard suggestion URL: fall back to '' and filter out results with no
  resolvable URL so "undefined" is never written into the link href
- Fix the AutoComplete size to [size]="'small'" (was the literal "'small'")
- Document that the external-URL overlay-close branch mutates non-public
  PrimeNG fields (pinned to primeng 21.x) for the upgrade checklist
- Revert incidental .claude/settings.json reformat to keep the PR focused

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-dotcms-page-search-in-the-link-popover

# Conflicts:
#	core-web/libs/new-block-editor/src/lib/editor/components/link-popover/link-popover.component.html
Reformats the two block-editor files flagged by CI's `nx format:check`
(Tailwind class ordering + line wrap) that were missed because the prior
merge commit used --no-verify.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@rjvelazco rjvelazco added this pull request to the merge queue Jun 3, 2026
Merged via the queue into main with commit 0a2aff2 Jun 3, 2026
58 checks passed
@rjvelazco rjvelazco deleted the issue-35890-block-editor-20-restore-internal-dotcms-page-search-in-the-link-popover branch June 3, 2026 19:07
fmontes pushed a commit that referenced this pull request Jun 4, 2026
…opover (#35907)

This PR fixes: #35890

### Proposed Changes

Restores feature parity with the legacy Block Editor by bringing
internal dotCMS **page search** back to the Block Editor 2.0 link
popover.

* **Page search in the URL field** — replaced the plain URL text input
with a PrimeNG `p-autoComplete`. Typing a partial title/path runs a
debounced (250 ms) search and shows a dropdown of matching pages.
* **Reuses existing data-access** — the search goes through
`DotContentSearchService` from `@dotcms/data-access` (the same service
the slash-menu drill-down uses); no new HTTP service. The Lucene query
is built inline at the call site, mirroring the legacy editor
(`basetype:5` pages + URL-mapped content, matched by title/path/urlmap
prefix).
* **Custom result template** — each suggestion shows the contentlet name
and its path, plus a thumbnail (`/dA/{inode}`) when available, with
ellipsis truncation to fit the popover.
* **External URLs never blocked** — when the value parses as a valid
`http(s)` URL the overlay is suppressed entirely (no search, no empty
message), so pasting/typing an external link works unchanged. Manual
entry is always allowed.
* **Empty state** — shows "No results found in dotCMS" when a search
returns nothing.
* **Works in create and edit flows** — selecting a result populates the
link `href`; opening an existing link prefills the field without firing
a spurious search.
* **Selection highlight while editing** — in insert mode, focusing the
URL input no longer hides which text will become the link: the target
range is painted with a ProseMirror decoration that survives the input
blur and is cleared when the popover closes.
* **Popover shell fix** — whitelisted `.p-autocomplete-overlay` in the
popover's click-outside handler so picking a suggestion (body-appended
overlay) doesn't dismiss the popover.
* **`rel` select fix** — the `rel` control now defaults to `null`
instead of `''`, so PrimeNG's `[showClear]` no longer renders a stray
"X" on the empty dropdown.

### Checklist
- [x] Translations — added `dot.block.editor.dialog.link.search.empty`
and updated the URL placeholder in `Language.properties`.
- [x] Security Implications Contemplated — no new endpoint; reuses the
existing `/api/content/_search`. Query-term interpolation matches the
legacy popover's behavior (search scoped to working/non-deleted content
for the active language).

### Additional Info
The search-result set (pages via `basetype:5` plus URL-mapped content,
scoped to the active `languageId`, working + non-deleted) reproduces the
legacy link popover's behavior.

### Screenshots


https://github.com/user-attachments/assets/453d14be-f426-4971-a7f8-a81ce0f8fd53

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Block Editor 2.0: restore internal dotCMS page search in the link popover

2 participants