# GbRichTextEditor — Deep Analysis

**Component:** `features/gbrichtexteditor/gbrichtexteditor.component.ts`
**Lightbox:** `features/gbrichtexteditor/gbrichtexteditorlightbox/gbrichtexteditorlightbox.component.ts`
**Date:** 2026-02-24
**Analyst:** Claude (auto-saved)

---

## Executive Summary

The `GbRichTextEditorComponent` is a TipTap-powered rich text editor used as a CMS viewer/editor. It is feature-capable but has **critical security vulnerabilities**, **functional data bugs**, and **numerous violations of project coding standards**. It must not be used in production in its current form.

---

## 1. CRITICAL Security Issues (P0 — Fix Before Any Deploy)

### 1.1 `bypassSecurityTrustHtml` — XSS Vector
**Files:** `gbrichtexteditor.component.ts:101,122`

```typescript
// Lines 101-103 and 122-124
this.savedContent = this.sanitizer.bypassSecurityTrustHtml(editor.getHTML());
```

- Angular's built-in XSS sanitizer is completely disabled for this value.
- `savedContent` is passed to the lightbox via `[content]="savedContent"`, then rendered as `[innerHTML]="content"` in the lightbox template — the trusted flag propagates.
- Any HTML or `<script>` tag injected by a user or from stored CMS content executes with full document scope.
- **Fix:** Replace with DOMPurify before binding. Example:
  ```typescript
  import DOMPurify from 'dompurify';
  this.savedContent = DOMPurify.sanitize(editor.getHTML());
  // Pass as plain string — Angular will sanitize [innerHTML] normally
  ```
- This is already a documented P0 in CLAUDE.md and must not regress.

### 1.2 `javascript:` URI in Link / Image (no validation)
**Files:** `gbrichtexteditor.component.ts:252-257`

```typescript
setLink() {
  const url = prompt('Enter URL');
  if (url) this.editor.chain().focus().setLink({ href: url }).run();
}
addImage() {
  const url = prompt('Enter image URL');
  if (url) this.editor.chain().focus().setImage({ src: url }).run();
}
```

- No URL validation is performed. A user can enter `javascript:alert(document.cookie)` as a link href.
- TipTap's Link extension does NOT sanitize `javascript:` URIs by default unless `protocols` is configured.
- **Fix:** Add URL validation before calling TipTap; configure Link with `protocols: ['http', 'https']` and `autolink: false`:
  ```typescript
  Link.configure({
    openOnClick: false,
    protocols: ['http', 'https'],
    validate: href => /^https?:\/\//.test(href),
  })
  ```
  Also validate image URLs against the same rules.

### 1.3 `prompt()` for User Input — UX & Security Antipattern
**Lines:** 252, 256

- `window.prompt()` is a blocking, unstyled, browser-native dialog — inconsistent UX.
- It cannot be styled, localized via Transloco, or validated before submission.
- In some environments (iframes, CSPs) `prompt()` is blocked entirely.
- **Fix:** Replace with a proper inline dialog using Angular Material `MatDialog` or an inline input pop-over within the toolbar.

---

## 2. Functional Bugs

### 2.1 CRITICAL: `html` field in every block contains ENTIRE document text — wrong data
**File:** `gbrichtexteditor.component.ts:195-198`

```typescript
html: this.editor.state.doc.textBetween(
  0,
  this.editor.state.doc.content.size,
  ''
),
```

- This uses the **full document's start (0) to end** range for **every block**.
- Every block in the emitted `blocks` array has the **same `html`** value — the plain text of the entire document.
- It also calls `textBetween` not `getHTML()`, so the value is **plain text**, not HTML.
- This makes the `html` field on each block completely useless and misleading.
- **Fix:** Each block should emit its own HTML. Either serialize `node.raw` via TipTap's serializer, or remove the redundant `html` field from individual blocks since the top-level envelope already has `html: editor.getHTML()`.

### 2.2 `extractText()` joins children with a space — breaks inline formatting
**Lines:** 181-187

```typescript
if (Array.isArray(n.content)) {
  return n.content.map((child: any) => extractText(child)).join(' ');
}
```

- Inline marks (bold, italic, links) are separate `text` nodes in TipTap's JSON — joining with `' '` adds spurious spaces.
- E.g., `"Hello **World**"` emits `"Hello World"` with an extra space between them.
- **Fix:** Join with `''` (empty string) for inline content; only insert spaces between block-level nodes.

### 2.3 Block IDs are positional (`block-${index}`) — not stable
**Line:** 191

```typescript
id: `block-${index}`,
```

- If a block is inserted before index 2, all subsequent blocks get new IDs.
- CMS systems relying on block IDs for change tracking or targeting break completely.
- **Fix:** Use a stable UUID, or use TipTap's `UniqueID` extension to assign persistent node IDs.

### 2.4 `getChangedBlock()` misses deleted/appended blocks
**Lines:** 205-218

```typescript
for (let i = 0; i < newBlocks.length; i++) {
  const oldBlock = this.previousBlocks[i];
  ...
}
```

- Only iterates over `newBlocks` length. If the last block was deleted, the deletion is never detected.
- Only returns the first changed block. If multiple blocks change (e.g., clear formatting), only the first is emitted.
- **Fix:** Also check if `previousBlocks.length !== newBlocks.length` as a change signal; collect all changed blocks.

### 2.5 `editor.getHTML()` called 3 times in `onUpdate`
**Lines:** 91, 97, 101

```typescript
const html = editor.getHTML();         // line 91 — emitted
...
html: editor.getHTML(),                // line 97 — in newContent (second call)
...
this.savedContent = this.sanitizer.bypassSecurityTrustHtml(editor.getHTML()); // line 101 (third call)
```

- Each `getHTML()` traverses and serializes the ProseMirror document. Three calls on every keystroke.
- **Fix:** Call once, cache the result: `const html = editor.getHTML();` and reuse.

### 2.6 `Color` and `TextStyle` extensions loaded but no toolbar UI
- `Color` and `TextStyle` extensions are registered (lines 82-83) but there is no color picker button in the toolbar.
- Users cannot change text color from the UI even though the extension is loaded.
- `TextStyle` also supports font-size and font-family — no UI for these either.

### 2.7 Toggle editor uses `display:none` — TipTap stays active
**Template line 209:**

```html
[ngStyle]="{ display: isEditorOpen ? 'block' : 'none' }"
```

- TipTap editor remains fully instantiated and listening for events even when hidden.
- For a viewer/preview mode, the editor should be suspended or the view switched to a read-only render.
- **Fix:** Use `*ngIf` to destroy the editor when hidden, or set `editor.setEditable(false)` for viewer mode. Or expose a proper `@Input() mode: 'edit' | 'view'` that controls this.

### 2.8 No read-only / view-only mode input
- The component is described as a "viewer cum editor" but there is no `@Input() readonly` or `mode` property.
- All consumers must always render the full editor toolbar even if they only need to display content.

### 2.9 Initial emit on `ngOnInit` fires before parent can subscribe
**Lines:** 112-127

- `blocksChange`, `changedBlock` are emitted synchronously in `ngOnInit` before the parent's `(blocksChange)` handler is bound.
- Any parent expecting the initial content via `blocksChange` will miss it if they rely on this emit.
- **Fix:** Defer initial emit with `queueMicrotask()` or `setTimeout()`, or document that consumers should not rely on the initial emit.

---

## 3. Performance Issues

### 3.1 Missing `ChangeDetectionStrategy.OnPush` (P0 — violates project standard)
**File:** `gbrichtexteditor.component.ts:24`

- Both `GbRichTextEditorComponent` and `GbRichTextEditorLightboxComponent` use default change detection.
- Every mouse event, keyboard event, or async action in the application triggers re-render of the entire editor and toolbar.
- `isActive()` is called 16+ times per CD cycle — each call queries TipTap's editor state.
- With a busy host page this creates significant unnecessary processing.
- **Fix:** Add `changeDetection: ChangeDetectionStrategy.OnPush` to both components. TipTap integration with OnPush requires calling `cdr.markForCheck()` after editor state changes.

### 3.2 `isActive()` called 16+ times per CD cycle from template — no memoization
**Template lines:** 14, 22, 30, 38, 54, 63, 72, 82, 94, 101, 116, 125, 134, 148, 164

- Every toolbar `[class.active]="isActive(...)"` binding re-evaluates on each CD cycle.
- `editor.isActive()` is not a pure function — it reads the ProseMirror state each time.
- **Fix:** With OnPush + signals, compute active states as a single `computed()` signal on editor update rather than calling `isActive()` from template.

### 3.3 `extractText` inner function recreated on every `extractBlocks()` call
**Line:** 181

```typescript
const extractText = (n: any): string => { ... };
```

- A new closure is created on every single call to `extractBlocks()`.
- **Fix:** Extract as a private class method.

### 3.4 `JSON.stringify` for block comparison — scales poorly
**Line:** 212

```typescript
JSON.stringify(oldBlock.raw) !== JSON.stringify(newBlock.raw)
```

- For large blocks (tables, nested lists), serializing to JSON string for equality is expensive.
- On every keystroke this runs for every block.
- **Fix:** Use a revision counter / TipTap's document history, or use a structural hash.

---

## 4. Memory Issues

### 4.1 `onUpdate` callback holds a closure over `this` — no leak if `destroy()` called
**Lines:** 89-108

- TipTap's `onUpdate` callback is correctly cleaned up when `editor.destroy()` is called in `ngOnDestroy`.
- The `ngOnDestroy` guard (`if (this.editor)`) is correct.
- **Risk:** If `ngOnDestroy` is skipped (e.g., route change without proper Angular teardown), the Editor instance and its ProseMirror view remain alive.

### 4.2 `ngOnChanges` can re-initialize editor without destroying the old one
**Lines:** 61-67**

```typescript
if (this.editor) {
  this.editor.commands.setContent(newContent);
} else {
  this.initializeEditor(newValue); // creates a new Editor instance
}
```

- This path is theoretically safe, but if `initializeEditor` is called (the `else` branch), a new `Editor` instance is created without destroying the previous one.
- Given the timing (`ngOnChanges` before `ngOnInit`), the `else` branch can fire, creating an orphaned editor instance if `ngOnInit` runs immediately after and calls `initializeEditor` again.
- **Fix:** Always call `this.editor?.destroy()` before creating a new instance.

### 4.3 `savedContent` SafeHtml reference never cleared
- Minor: `savedContent` holds a `SafeHtml`-wrapped string. On each update it is reassigned but the old reference is kept alive until GC. Non-critical.

---

## 5. Code Quality / Standards Violations

### 5.1 `any` types everywhere (violates project TypeScript standards)
| Location | Problem |
|---|---|
| `@Input() content: any` | No input type |
| `@Output() blocksChange = new EventEmitter<any[]>()` | Untyped blocks |
| `@Output() changedBlock = new EventEmitter<any>()` | Untyped block |
| `private previousBlocks: any[]` | Untyped array |
| `extractBlocks(): any[]` | Untyped return |
| `getChangedBlock(newBlocks: any[])` | Untyped params |
| `setHeading(level: any)` | Should be `1 \| 2 \| 3` |
| `isActive(name: any, attrs: any)` | Should use TipTap types |
| `@Input() content: any` (lightbox) | Untyped |

- **Fix:** Define interfaces for all block types:
```typescript
interface GbBlock {
  id: string;
  type: string;
  attrs: Record<string, unknown>;
  content: string;
  raw: JSONContent;
}

interface GbEditorContent {
  blocks: GbBlock[];
  html: string;
}

type HeadingLevel = 1 | 2 | 3;
```

### 5.2 `console.log` in production code (violates project standards)
**File:** `gbrichtexteditor.component.ts:155`

```typescript
console.log('📄 Built TipTap doc:', JSON.stringify(tiptapDoc, null, 2));
```

- Must be replaced with `GbConsoleService` or removed entirely.
- This also `JSON.stringify`s potentially large document trees on every content init — a performance cost in addition to the logging violation.

### 5.3 Constructor injection instead of `inject()` (violates project standards)
**Line:** 49

```typescript
constructor(private sanitizer: DomSanitizer) { }
```

- **Fix:** `private sanitizer = inject(DomSanitizer);`

### 5.4 `OnChanges` interface not declared
**Line:** 36

```typescript
export class GbRichTextEditorComponent implements OnInit, OnDestroy {
```

- `ngOnChanges` is implemented but `OnChanges` is not in the `implements` clause.
- While TypeScript won't catch this at runtime, it's a maintainability issue.

### 5.5 Toolbar labels hardcoded in English (violates i18n standards)
- All `title="..."` attributes on toolbar buttons use hardcoded English strings.
- Should use `[title]="'richtexteditor.toolbar.bold' | transloco"` etc.

### 5.6 No `aria-label`, `aria-pressed`, or `role="toolbar"` — accessibility missing
- `title` is not a screen-reader substitute for `aria-label`.
- Active state (`isActive`) is never reflected in `aria-pressed="true/false"`.
- No `role="toolbar"` on the toolbar `<div>`.
- No keyboard navigation between toolbar buttons (Tab/Arrow key handling).

### 5.7 Lightbox SCSS has dead/wrong CSS class
**File:** `gbrichtexteditorlightbox.component.scss:35`

```scss
.editor-preview {        /* <-- THIS */
  width: 100%;
  height: 100%;
  overflow: auto;
}
```

The template uses `.richtexteditor-preview`, not `.editor-preview`. This CSS rule does nothing.

### 5.8 Empty `ngOnInit` in lightbox
**File:** `gbrichtexteditorlightbox.component.ts:13`

```typescript
ngOnInit() {
}
```

Remove. The `OnInit` interface implementation and the empty lifecycle hook add noise.

### 5.9 Hardcoded `max-height: 500px` on editor content area
**File:** `gbrichtexteditor.component.scss:50`

```scss
max-height: 500px;
```

- Fixed pixel height is a responsive design violation per project standards.
- On mobile screens this may be too large or too small.
- **Fix:** Use `@Input() maxHeight` or a CSS custom property so consumers can control height.

### 5.10 No RTL support
- No `[dir="rtl"]` CSS selectors in either component's SCSS.
- For Arabic content (which the platform supports), text alignment, list markers, and blockquote border-side will all be wrong.
- **Fix:** Add `[dir="rtl"] .editor-content { ... }` overrides. Also pass `textAlign: 'justify'` support and configure TipTap for RTL.

---

## 6. Architecture / Structural Recommendations

### 6.1 Split Editor and Viewer into separate components
For CMS use cases, the viewer should not load TipTap editor infrastructure. Suggest:

```
GbRichTextEditorComponent  — edit mode (TipTap)
GbRichTextViewerComponent  — view mode (DOMPurify-sanitized innerHTML, no TipTap)
```

Or expose a single component with `@Input() mode: 'edit' | 'view'` that conditionally renders editor vs viewer.

### 6.2 Migrate to signals / Angular 20 standards
Current code uses the lifecycle hook / EventEmitter pattern. The entire state model can be simplified:

```typescript
// Replace all the onUpdate + previousBlocks complexity:
content = input<GbEditorContent | string>('');
contentChange = output<string>();
blocksChange = output<GbEditorContent[]>();

// Initialize editor reactively
private editorContent = computed(() => this.prepareEditorContent(this.content()));
```

### 6.3 Toolbar should be a separate component
The toolbar has 20+ buttons and is tightly coupled to the editor. For extensibility (CMS blocks, tables, etc.), the toolbar should be:
- A separate `GbRichTextToolbarComponent` with `@Input() editor: Editor`
- Or driven by a configuration object `@Input() toolbarConfig: ToolbarConfig` for selective toolbar display

### 6.4 Extensions should be configurable
Currently all extensions are hardcoded. For different use cases (basic comment box vs. full CMS editor), consumers can't selectively enable/disable features.

```typescript
@Input() features: EditorFeature[] = ['bold', 'italic', 'headings', 'lists', 'links', 'images'];
```

### 6.5 No table, mention, or custom block extension
For CMS usage the following TipTap extensions are commonly needed but missing:
- `@tiptap/extension-table` — table support
- `@tiptap/extension-mention` — @mentions
- `@tiptap/extension-placeholder` — placeholder text
- `@tiptap/extension-character-count` — content length limits
- `@tiptap/extension-code-block-lowlight` — syntax highlighting
- `@tiptap/extension-horizontal-rule` — HR blocks
- `@tiptap/extension-subscript` / `@tiptap/extension-superscript`

### 6.6 Image upload vs URL-only
The current implementation only accepts image URLs (from `prompt()`). For a CMS, images should be uploaded to a server. Recommend adding an `@Output() imageUploadRequest` that the parent handles, or integrating with a file picker.

---

## 7. Priority Fix Order

| Priority | Issue | File:Line |
|---|---|---|
| P0 | `bypassSecurityTrustHtml` — XSS | `ts:101,122` |
| P0 | `javascript:` URI in links/images — XSS | `ts:252,256` |
| P0 | Missing `OnPush` — performance | `ts:24` |
| P1 | `html` field in blocks = wrong full-doc text | `ts:195-198` |
| P1 | `editor.getHTML()` called 3x per keystroke | `ts:91,97,101` |
| P1 | `console.log` of TipTap doc in prod | `ts:155` |
| P1 | Constructor injection instead of `inject()` | `ts:49` |
| P1 | All `any` types — define interfaces | `ts:37-44` |
| P2 | `extractText` joins with `' '` — breaks inline marks | `ts:185` |
| P2 | Block IDs positional — not stable | `ts:191` |
| P2 | `getChangedBlock` misses deletions | `ts:205-218` |
| P2 | Dead CSS class `.editor-preview` in lightbox | `lightbox.scss:35` |
| P2 | Empty `ngOnInit` in lightbox | `lightbox.ts:13` |
| P2 | Hardcoded English `title` attributes — i18n | `html:17-205` |
| P2 | No aria attributes — accessibility | `html:9-205` |
| P3 | No read-only/viewer mode input | — |
| P3 | `Color`/`TextStyle` loaded but no UI | `ts:82-83` |
| P3 | `display:none` keeps TipTap active | `html:209` |
| P3 | Hardcoded `max-height: 500px` | `scss:50` |
| P3 | No RTL CSS | `scss` |
| P3 | No table/placeholder/count extensions | — |

---

## 8. Recommended Interfaces (type-safe replacement for `any`)

```typescript
export type HeadingLevel = 1 | 2 | 3;

export interface GbEditorBlock {
  id: string;
  type: string;
  attrs: Record<string, unknown>;
  content: string;   // plain-text extraction of this block only
  raw: JSONContent;  // TipTap node — remove 'html' from here (it was wrong anyway)
}

export interface GbEditorContent {
  blocks: GbEditorBlock[];
  html: string;
}

export type GbEditorMode = 'edit' | 'view';
export type GbEditorInput = string | JSONContent | GbEditorContent[];
```