Skip to content

Commit ff4cccb

Browse files
authored
Merge pull request #2593 from intersective/2.4.y.z/CORE-7942/description-wysiwyg-fix
[CORE-7942] 2.4.y.z/description wysiwyg fix
2 parents 4ca3b6b + 1383e52 commit ff4cccb

File tree

3 files changed

+182
-4
lines changed

3 files changed

+182
-4
lines changed
Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
# CORE-7942: Whitespace Stripping Fix in Language Detection
2+
3+
## Problem Summary
4+
5+
When displaying HTML content with the `detectLanguage` pipe in description components, spaces between inline elements (like `<a>` tags) and adjacent text were being stripped, causing text to run together.
6+
7+
### Example
8+
**API Response:**
9+
```html
10+
<a href="...">Privacy Policy</a> click here
11+
```
12+
13+
**Rendered Output (BEFORE FIX):**
14+
```
15+
Privacy Policyclick here
16+
```
17+
(no space between "Policy" and "click")
18+
19+
**Expected Output (AFTER FIX):**
20+
```
21+
Privacy Policy click here
22+
```
23+
(space preserved)
24+
25+
## Root Cause Analysis
26+
27+
The issue was in the `addLanguageAttributes()` method in [utils.service.ts](projects/v3/src/app/services/utils.service.ts#L902-L945), which is called by the `detectLanguage` pipe for WCAG 3.1.2 Language of Parts compliance.
28+
29+
### Technical Details
30+
31+
When processing HTML content:
32+
1. The DOM parser creates separate text nodes for text outside and between elements
33+
2. For `<a>Privacy Policy</a> click here`, there are 2 text nodes:
34+
- Text node 1 (inside `<a>`): `"Privacy Policy"`
35+
- Text node 2 (after `<a>`): `" click here"` (with leading space)
36+
37+
### The Bug (Lines 919-925)
38+
39+
```typescript
40+
const text = node.textContent?.trim() || ''; // ❌ Strips whitespace
41+
if (text.length >= 10) {
42+
const detectedLang = this.detectLanguage(text, baseLang);
43+
if (detectedLang) {
44+
const span = this.document.createElement('span');
45+
span.setAttribute('lang', detectedLang);
46+
span.textContent = text; // ❌ Uses trimmed text, losing leading/trailing spaces
47+
node.parentNode?.replaceChild(span, node);
48+
}
49+
}
50+
```
51+
52+
**What Happened:**
53+
- Line 919: `node.textContent?.trim()` converted `" click here"``"click here"`
54+
- Line 921: Length check: `"click here".length = 10` ✅ passes threshold
55+
- Line 924: Assigned trimmed text to new span element
56+
- Line 925: Replaced original node (with space) with span (without space)
57+
58+
**Result:** Space between "Policy" and "click" disappeared
59+
60+
## Solution Implemented
61+
62+
### 1. Preserve Original Whitespace
63+
64+
Changed line 924 to use `node.textContent` (original) instead of `text` (trimmed):
65+
66+
```typescript
67+
const text = node.textContent?.trim() || ''; // ✅ Use for validation only
68+
if (text.length >= 20) {
69+
const detectedLang = this.detectLanguage(text, baseLang);
70+
if (detectedLang) {
71+
const span = this.document.createElement('span');
72+
span.setAttribute('lang', detectedLang);
73+
span.textContent = node.textContent; // ✅ Preserve original whitespace
74+
node.parentNode?.replaceChild(span, node);
75+
}
76+
}
77+
```
78+
79+
**Strategy:**
80+
- Use `text` (trimmed) for validation and language detection logic
81+
- Use `node.textContent` (original with whitespace) for rendering
82+
83+
### 2. Increase Minimum Length Threshold
84+
85+
Changed minimum length from **10** to **20** characters:
86+
87+
```typescript
88+
// minimum text length for reliable detection (increased from 10 to 20 for better accuracy)
89+
const minLength = 20;
90+
```
91+
92+
**Benefits:**
93+
1. **Better Accuracy**: Language detection algorithms are more reliable with longer text
94+
2. **Avoid False Positives**: Short phrases like "click here" (10 chars) won't trigger detection
95+
3. **Performance**: Reduces unnecessary processing of short text nodes
96+
4. **WCAG Intent**: WCAG 3.1.2 is meant for substantial foreign language content, not individual words
97+
98+
## Impact Assessment
99+
100+
### Fixed Components
101+
All components using the `detectLanguage` pipe will benefit:
102+
- [description.component.html](projects/v3/src/app/components/description/description.component.html)
103+
- [activity-desktop.component.html](projects/v3/src/app/desktop/activity-desktop/activity-desktop.component.html)
104+
- [review-desktop.component.html](projects/v3/src/app/desktop/review-desktop/review-desktop.component.html)
105+
106+
### Edge Cases Considered
107+
- **Text < 20 chars**: Not processed (preserves original spacing by default)
108+
- **Multiple spaces**: Preserved exactly as in original HTML
109+
- **Non-breaking spaces (`&nbsp;`)**: Preserved as HTML entities
110+
- **Mixed content**: Works correctly with inline elements
111+
112+
## Testing Recommendations
113+
114+
1. **Visual Test**: Verify spacing appears correctly in description content with links
115+
2. **Language Detection**: Confirm foreign language passages (>20 chars) still get `lang` attributes
116+
3. **Short Text**: Verify short phrases (<20 chars) don't get incorrectly wrapped in `<span lang="...">`
117+
4. **Regression**: Test existing descriptions with mixed English/foreign content
118+
119+
## Files Modified
120+
121+
- `/projects/v3/src/app/services/utils.service.ts`
122+
- Line 849: Increased `minLength` from 10 to 20
123+
- Lines 918-926: Added comments and fixed whitespace preservation in `addLanguageAttributes()`
124+
125+
## Related Tickets
126+
127+
- CORE-7942: Description WYSIWYG spacing fix
128+
- Original accessibility implementation for WCAG 3.1.2 compliance

package-lock.json

Lines changed: 48 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

projects/v3/src/app/services/utils.service.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -847,8 +847,8 @@ export class UtilsService {
847847
return null;
848848
}
849849

850-
// Minimum text length for reliable detection
851-
const minLength = 10;
850+
// minimum text length for reliable detection (increased from 10 to 20 for better accuracy)
851+
const minLength = 20;
852852
const cleanText = text.trim().replace(/<[^>]*>/g, ' ').replace(/\s+/g, ' ');
853853

854854
if (cleanText.length < minLength) {
@@ -915,13 +915,15 @@ export class UtilsService {
915915

916916
const processNode = (node: Node): void => {
917917
if (node.nodeType === Node.TEXT_NODE) {
918+
// use trimmed text for validation only, preserve original whitespace for rendering
918919
const text = node.textContent?.trim() || '';
919-
if (text.length >= 10) {
920+
if (text.length >= 20) {
920921
const detectedLang = this.detectLanguage(text, baseLang);
921922
if (detectedLang) {
922923
const span = this.document.createElement('span');
923924
span.setAttribute('lang', detectedLang);
924-
span.textContent = text;
925+
// preserve original whitespace from node.textContent (not trimmed)
926+
span.textContent = node.textContent;
925927
node.parentNode?.replaceChild(span, node);
926928
}
927929
}

0 commit comments

Comments
 (0)