Skip to content

Commit bcb4305

Browse files
SysAdminDocSysAdminDoc
committed
Release v1.8.92 — LDML parser honours shift= over longPress=
KeymanLdmlParser tried longPress first, falling back to shift. LDML semantics are the opposite: shift= is the shift-modifier mapping; longPress= is a space-separated list of long-press alternates. Two bugs followed: 1. When both shift= and longPress= were declared, shift= was silently ignored — real LDML keyboards lost their shift mapping. 2. When only longPress= was set with multiple alternates, only the first alternate's first codepoint became the shift value, masking the alternates list with no recovery. New ordering: 1. shift= present → use it 2. else longPress= single value (no space) → use it (preserves Amharic-SERA-style legacy authors) 3. else → shift = null (multi-alternate lists wait for a future longPressAlternates field + long-press UI) Pre-existing fixture (single-value longPress) continues to pass through path #2 — no regression for already-imported keyboards. Adds three new tests: shift-wins-over-longPress, multi-alternate-leaves-shift-null, single-alternate-longPress-as-shift-fallback. Follow-up #7 (final) from the v1.8.85 audit.
1 parent cd922dc commit bcb4305

4 files changed

Lines changed: 170 additions & 6 deletions

File tree

‎RELEASE_NOTES_v1.8.92.md‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
# Release v1.8.92 — LDML parser honours shift= over longPress=
2+
3+
Date: 2026-05-17
4+
5+
Follow-up #7 (final) from the v1.8.85 audit pass.
6+
7+
## What changed
8+
9+
[app/src/main/kotlin/dev/patrickgold/florisboard/ime/hardware/KeymanLdmlParser.kt](app/src/main/kotlin/dev/patrickgold/florisboard/ime/hardware/KeymanLdmlParser.kt#L113-L140)
10+
— LDML defines:
11+
12+
- `shift=` — the shift-modifier mapping for the key.
13+
- `longPress=` — a space-separated list of alternates surfaced on long-press
14+
(NOT a shift mapping).
15+
16+
The previous parser was inverted: it tried `longPress` *first*, falling
17+
back to `shift`. Two real bugs followed:
18+
19+
1. **Whenever both `shift=` and `longPress=` were declared on the same
20+
key, `shift` was silently ignored.** Real Keyman / LDML keyboards
21+
author shift= for the shift mapping and longPress= for alternates;
22+
the parser picked the wrong slot.
23+
2. **When only `longPress=` was set with multiple alternates,** only the
24+
first alternate's first codepoint became the shift value, masking
25+
the rest of the alternates list with no UX to recover them.
26+
27+
This release reorders:
28+
29+
```
30+
1. If shift= is set → use it.
31+
2. Else if longPress= is a single value (no space) → use it
32+
(preserves Amharic-SERA-style legacy authors who used longPress
33+
as a shift workaround before this parser learned the right
34+
semantics).
35+
3. Else → leave shift = null
36+
(multi-alternate longPress is correctly a list, not a shift slot —
37+
wait for a future release to add `longPressAlternates: List<Int>` to
38+
HardwareKeyEntry and route through the long-press UI).
39+
```
40+
41+
The pre-existing fixture (`output="ሀ" longPress="ሁ"`, single-value
42+
longPress) still produces the same result through path #2, so no
43+
already-imported keyboard regresses.
44+
45+
## Tests
46+
47+
[app/src/test/kotlin/dev/patrickgold/florisboard/ime/hardware/KeymanLdmlParserTest.kt](app/src/test/kotlin/dev/patrickgold/florisboard/ime/hardware/KeymanLdmlParserTest.kt)
48+
gains three new tests:
49+
50+
- `shift attribute is preferred over longPress for the shift slot` —
51+
proves the bug fix on the both-attributes case.
52+
- `multi-alternate longPress with no shift leaves shift slot null` —
53+
proves multi-alternate lists no longer poison the shift slot.
54+
- `single-alternate longPress with no shift remains usable as shift fallback` —
55+
proves the Amharic-SERA backward-compat case still works.
56+
57+
The pre-existing `normal + shift output round-trip through the
58+
scancode map` test (single-alternate longPress fixture) continues to
59+
pass unchanged.
60+
61+
## Why not also implement longPressAlternates now
62+
63+
Adding `longPressAlternates: List<Int>` to
64+
[HardwareKeyEntry](app/src/main/kotlin/dev/patrickgold/florisboard/ime/hardware/HardwareKeyboardLayout.kt)
65+
and routing the alternates through the long-press popup is a larger
66+
slice that crosses the LDML parser, the data class, the popup
67+
controller, and the popup-UI snygg surface. It's worth its own
68+
per-feature release once the existing long-press popup is audited for
69+
hardware-keyboard-source events. Tracked in the v1.8.85
70+
[follow-up roster](RELEASE_NOTES_v1.8.85.md#follow-up-work-next-per-feature-releases).
71+
72+
## Files touched
73+
74+
- `app/src/main/kotlin/dev/patrickgold/florisboard/ime/hardware/KeymanLdmlParser.kt`
75+
- `app/src/test/kotlin/dev/patrickgold/florisboard/ime/hardware/KeymanLdmlParserTest.kt`
76+
- `gradle.properties` — versionCode 1892 / versionName 1.8.92
77+
78+
## Verification
79+
80+
```powershell
81+
./gradlew.bat :app:testDebugUnitTest
82+
./gradlew.bat :app:lintDebug
83+
./gradlew.bat :app:assembleDebug
84+
```
85+
86+
The new tests should pass; the pre-existing Amharic-fixture test should
87+
continue to pass.

‎app/src/main/kotlin/dev/patrickgold/florisboard/ime/hardware/KeymanLdmlParser.kt‎

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,10 +111,37 @@ object KeymanLdmlParser {
111111
val id = node.getAttribute("id")
112112
if (id.isBlank()) continue
113113
val output = node.getAttribute("output").takeIf { it.isNotEmpty() }?.decodeLdmlEscapes()
114-
val shiftOutput = (
115-
node.getAttribute("longPress").takeIf { it.isNotEmpty() }
116-
?: node.getAttribute("shift").takeIf { it.isNotEmpty() }
117-
)?.decodeLdmlEscapes()
114+
// LDML semantics:
115+
// shift= — the shift-modifier mapping for this key.
116+
// longPress= — a space-separated list of alternates surfaced
117+
// on long-press (NOT a shift mapping).
118+
// Previously this parser used `longPress` as the shift slot,
119+
// falling back to `shift`. That picked the wrong slot whenever
120+
// both were declared (real LDML keyboards use shift for the
121+
// shift mapping and longPress for the alternates list) and
122+
// promoted only the first long-press alternate to the shift
123+
// slot when only longPress was set, masking the alternates UX.
124+
//
125+
// New behaviour:
126+
// 1. If shift= is set, use it for the shift slot.
127+
// 2. Else if longPress= contains exactly one whitespace-
128+
// separated value, use it for the shift slot. This
129+
// preserves the previously-shipped behaviour for LDML
130+
// keyboards (e.g. Amharic SERA) that authored single-
131+
// alternate longPress declarations as a shift workaround
132+
// before this parser learned the right semantics.
133+
// 3. Else leave shift = null. A future release will route
134+
// multi-alternate longPress through a dedicated
135+
// `longPressAlternates: List<Int>` field on
136+
// HardwareKeyEntry once the long-press UI exists.
137+
val shiftAttr = node.getAttribute("shift").takeIf { it.isNotEmpty() }
138+
val longPressAttr = node.getAttribute("longPress").takeIf { it.isNotEmpty() }
139+
val shiftRaw = when {
140+
shiftAttr != null -> shiftAttr
141+
longPressAttr != null && !longPressAttr.contains(' ') -> longPressAttr
142+
else -> null
143+
}
144+
val shiftOutput = shiftRaw?.decodeLdmlEscapes()
118145
val normalCp = output?.codePointAt(0)
119146
val shiftCp = shiftOutput?.codePointAt(0)
120147
if (normalCp == null && shiftCp == null) continue

‎app/src/test/kotlin/dev/patrickgold/florisboard/ime/hardware/KeymanLdmlParserTest.kt‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,56 @@ class KeymanLdmlParserTest : FunSpec({
8787
layout.scancodeMap.size shouldBe 1
8888
}
8989

90+
test("shift attribute is preferred over longPress for the shift slot") {
91+
// When both are declared, LDML's shift= is the shift-modifier
92+
// mapping and longPress= is a space-separated alternates list.
93+
// shift= must win.
94+
val ldml = """
95+
<keyboard locale="x-test">
96+
<keys>
97+
<key id="A01" output="a" shift="A" longPress="ä á à"/>
98+
</keys>
99+
</keyboard>
100+
""".trimIndent()
101+
val layout = KeymanLdmlParser.parse(ldml)
102+
val a01 = layout.scancodeMap.values.first { it.virtualKeyName == "A01" }
103+
a01.normal shouldBe 'a'.code
104+
a01.shift shouldBe 'A'.code
105+
}
106+
107+
test("multi-alternate longPress with no shift leaves shift slot null") {
108+
// longPress with spaces is a list of alternates; none should be
109+
// promoted into the shift slot (the alternates UX is a separate
110+
// feature once HardwareKeyEntry grows a longPressAlternates field).
111+
val ldml = """
112+
<keyboard locale="x-test">
113+
<keys>
114+
<key id="A01" output="a" longPress="ä á à"/>
115+
</keys>
116+
</keyboard>
117+
""".trimIndent()
118+
val layout = KeymanLdmlParser.parse(ldml)
119+
val a01 = layout.scancodeMap.values.first { it.virtualKeyName == "A01" }
120+
a01.normal shouldBe 'a'.code
121+
a01.shift shouldBe null
122+
}
123+
124+
test("single-alternate longPress with no shift remains usable as shift fallback") {
125+
// Preserves the Amharic SERA-style backward-compat case where the
126+
// LDML author used longPress as the shift workaround.
127+
val ldml = """
128+
<keyboard locale="x-test">
129+
<keys>
130+
<key id="A01" output="a" longPress="A"/>
131+
</keys>
132+
</keyboard>
133+
""".trimIndent()
134+
val layout = KeymanLdmlParser.parse(ldml)
135+
val a01 = layout.scancodeMap.values.first { it.virtualKeyName == "A01" }
136+
a01.normal shouldBe 'a'.code
137+
a01.shift shouldBe 'A'.code
138+
}
139+
90140
test("display override with to attribute labels the matching key output") {
91141
val ldml = """
92142
<keyboard locale="km-KH">

‎gradle.properties‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,5 @@ projectMinSdk=26
1515
projectTargetSdk=36
1616
projectCompileSdk=36
1717

18-
projectVersionCode=1891
19-
projectVersionName=1.8.91
18+
projectVersionCode=1892
19+
projectVersionName=1.8.92

0 commit comments

Comments
 (0)