keymanapp / keymanapp/keyman

bug(developer): KMP Compiler Path Flattening Breaks Documentation and KPS Addition Silently Skips Same-name Files

Open
#15,223 3 comments 1 reaction 1 assignee Claimed by @mcdurdin View on GitHub
bug developer/
Dominant language
Pascal
Stars
534
Forks
143
Avg merge
2d 10h
Merged PRs (30d)
113

Description

## Summary

I was working on getting welcome.htm to be available locally in App Builders, and in exploring a KMP file, I found a Keyman bug. The KMP package compiler flattens all file paths to the root directory when creating `.kmp` files, which causes two critical issues:

1. **Duplicate basenames from different source folders silently don't get added** If you added ./qwerty/image.png and ./azerty/image.png, only the first would get added to the kps.
2. **HTML/CSS documentation files cannot reference resources in any other directory after compilation into KMP**, breaking image and asset references

These issues affect **at least 25 keyboards** in the keyboards repository, causing broken documentation in production packages.

## Issue 1: Silent File Overwrites on Basename Conflicts

### Current Behavior

When a KPS file references multiple files with the same basename but different paths, the compiler:
- **Silently skips** adding the new ones to the KPS
- Provides **no warning** to the developer

**Code Location:** [`developer/src/kmc-package/src/compiler/kmp-compiler.ts:528-539`](https://github.com/keymanapp/keyman/blob/master/developer/src/kmc-package/src/compiler/kmp-compiler.ts#L528-L539)

```typescript
const basename = this.callbacks.path.basename(filename);
// ...
zip.file(basename, memberFileData); // Silently overwrites if basename exists
```

### UI Prevention vs. Compiler Prevention

The Keyman Developer UI prevents adding duplicate basenames ([`UfrmPackageEditor.pas:710`] to a KPS (https://github.com/keymanapp/keyman/blob/master/developer/src/tike/child/UfrmPackageEditor.pas#L710)) by silently skipping them:

```pascal
if pack.Files.FromFileNameEx(FileName) <> nil then Exit; // Already added
```

However:
- This only prevents the issue in the UI, not when editing KPS files directly or via other tools
- It provides no warning to the user
- It doesn't help with the subdirectory reference issue (Issue 2)

If you tried to bundle 2 keyboards with same-name documentation files, you'll only get one of each.

## Issue 2: HTML passing through KMP Cannot Reference Files in Subdirectories

### Current Behavior

HTML documentation files that reference resources in sub or parent directories **fail** because:
1. The KPS lists imports files with their relative paths (e.g., `welcome\image\default.png`)
2. The compiler flattens all files to the root of the KMP (e.g., `default.png`)
3. The HTML files are copied **verbatim** with no path transformation
4. References like `` become broken links

**There is NO existing HTML path transformation** in the compiler.

### Confirmed Production Bug: khmer_angkor

https://github.com/keymanapp/keyboards/issues/3793

**KPS File:**
```xml
welcome\image\default.png
welcome\image\shift_flick.png

```

**HTML File (`welcome/welcome.htm`):**
```html
default
shift_with_flick
```

**Compiled KMP Contents:**
```
default.png (flattened from welcome/image/default.png)
shift_flick.png (flattened from welcome/image/shift_flick.png)
welcome.htm (unchanged - still references image/default.png)
```

**Result:** All 6 documentation images fail to load in the official release package.

**Verification:** Downloaded official `khmer_angkor.kmp` from keyman.com - images are broken.

## Affected Keyboards

By analyzing HTML files that reference subdirectories or files outside their own directory (the regex was `(src|href)="\w+/`), the following keyboards have broken documentation if those files are compiled into the KMP:

### Broken Image/Asset References (Subdirectories)
- `keyboards/release/k/khmer_angkor/source/welcome/welcome.htm` ✓ **CONFIRMED BROKEN**
- `keyboards/release/p/pukapuka/source/welcome/welcome.htm`

### Broken File References (Different Directories)
- `keyboards/legacy/k/khmer10/source/khmer10.html`
- `keyboards/release/l/lao_2008_rapid/source/readme.htm`
- `keyboards/release/l/lao_pali/source/readme.htm`
- `keyboards/release/a/arabic_w_o_dots/source/welcome.htm`
- `keyboards/release/e/easy_arabic/source/welcome.htm`
- `keyboards/release/p/programmer_dvorak/source/welcome.htm`
- `keyboards/release/w/wakhi/source/welcome.htm`

### SIL Senegal Keyboards (13 keyboards)
All reference shared assets in parent directories:
- `keyboards/release/sil/sil_senegal_bqj_azerty/source/readme.htm`
- `keyboards/release/sil/sil_senegal_cou_azerty/source/readme.htm`
- `keyboards/release/sil/sil_senegal_cou_qwerty/source/readme.htm`
- `keyboards/release/sil/sil_senegal_csk_azerty/source/readme.htm`
- `keyboards/release/sil/sil_senegal_dyo_azerty/source/readme.htm`
- `keyboards/release/sil/sil_senegal_dyo_azerty/source/sil_senegal_dyo_azerty-help.htm`
- `keyboards/release/sil/sil_senegal_gsl_azerty/source/readme.htm`
- `keyboards/release/sil/sil_senegal_gsl_azerty/source/sil_senegal_gsl_azerty-help.htm`
- `keyboards/release/sil/sil_senegal_knf_azerty/source/readme.htm`
- `keyboards/release/sil/sil_senegal_krx_azerty/source/readme.htm`
- `keyboards/release/sil/sil_senegal_krx_azerty/source/sil_senegal_krx_azerty-help.htm`
- `keyboards/release/sil/sil_senegal_krx_qwerty/source/readme.htm`
- `keyboards/release/sil/sil_senegal_srr_azerty/source/readme.htm`
- `keyboards/release/sil/sil_senegal_srr_azerty/source/sil_senegal_srr_azerty-help.htm`
- `keyboards/release/sil/sil_senegal_wo_azerty/source/readme.htm`
- `keyboards/release/sil/sil_senegal_wo_azerty/source/sil_senegal_wo_azerty.htm`

**Total: At least 25 keyboards** with broken documentation in production.

### ./help directory is not a problem.
The ./help directory is not affected by this bug because those files are never committed to the KMP.

## Root Cause Analysis

### Historical Design Decision

The KMP format was probably designed with a flat file structure for simplicity and cross-platform compatibility. The compiler has always flattened paths, but:

1. The community has organically avoided using subdirectories in documentation
2. Most keyboards use only basename references in HTML (e.g., `src="layout.png"`)
3. The few keyboards that do use subdirectories have shipped with broken documentation

### Why This Went Unnoticed

- No automated validation or testing of HTML file references during compilation
- Documentation is viewed in browsers after installation, but broken images often fail silently
- Small sample size - only a handful of keyboards use subdirectories

## Options:
### 1. Basename Conflict Detection
- Before adding files to ZIP, check if basename already exists
- On conflict, auto-rename files with numeric suffixes:
- `modular/sri.png` → `sri_1.png`
- `typewriter/sri.png` → `sri_2.png`
- Maintain a mapping of original paths → renamed basenames

This would require HTML/CSS Path Rewriting
- Parse HTML, CSS, and JavaScript files before adding to KMP
- Detect resource references: `src=`, `href=`, `url()`, etc.
- Rewrite paths based on the file mapping:
- `` → ``
- `` → ``
- Handle both relative paths and subdirectory references

### 2. Developer Warnings
Keep flattening as is, but warn the user.

- Warn when different-folder basename conflicts are found
- Error if an HTML href/src references cannot be resolved to a file in the KPS

### 3. User/Curator Training

Fix the problem files and in the future, warn people not to allow multiple files with the same name or to cross-reference any files in other folders from HTML/CSS.

## References

- **KMP Compiler:** [`developer/src/kmc-package/src/compiler/kmp-compiler.ts`](https://github.com/keymanapp/keyman/blob/master/developer/src/kmc-package/src/compiler/kmp-compiler.ts)
- **UI File Handler:** [`developer/src/tike/child/UfrmPackageEditor.pas`](https://github.com/keymanapp/keyman/blob/master/developer/src/tike/child/UfrmPackageEditor.pas)
- **Example Broken Package:** [`keyboards/release/k/khmer_angkor`](https://github.com/keymanapp/keyboards/tree/master/release/k/khmer_angkor)

## Steps to Reproduce

1. Download official `khmer_angkor.kmp` from keyman.com
2. Install the package
3. When it shows the welcome page, scroll down.
4. Observe that some images fail to load (broken `image/` path references)
5. In this case, they weren't added to the KPS, but even if they were, the HTML links would still be broken after extraction.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.