Quick actions land on the wrong key on layouts whose third letter row is nine columns or more, including the standard Russian one

[AndrewKirkovski]

What

Undo, cut, copy and paste land on the wrong key on the standard Russian layout, and on most non-qwerty layouts generally. east_slavic, belarusian and ukrainian are all 11/11/9, and on a 9-wide third letter row every quick action sits one key to the left of where it belongs. A row of eleven is off by two.

Observed on an emulator with Quick Actions bound to long press:

key	column	should map to	action before the fix	correct
я	0	nothing	undo (z)	none
ч	1	z	cut (x)	undo
с	2	x	copy (c)	cut
м	3	c	paste (v)	copy

It has gone unnoticed because copy lands on Cyrillic с, which is shaped like Latin c, and because the qwerty family is the one shape where the bug cannot appear.

Why

In v2keyboard/MoreKeysBuilder.kt, three functions map a key's grid coordinate onto a qwerty reference. Two center the column first; actionForCoord indexes with the raw column:

val row = QwertySymbols.getOrNull(keyCoordinate.regularRow)
val letter = row?.getOrNull(keyCoordinate.regularColumn)

symsForCoord centers against the width of the row it is about to index, clamps a negative offset, and rejects a negative column:

val colOffset = (keyCoordinate.measurement.numColumnsByRow[keyCoordinate.regularRow] - row.size) / 2
val centeredCol = keyCoordinate.regularColumn - colOffset.coerceAtLeast(0)
if(centeredCol < 0) return ""

QwertySymbols row sizes are 10 / 10 / 7 (row 1 is "asdfghjkl" plus "r2_e1"). numColumnsByRow counts only keys whose moreKeyMode is All (BaseKey.kt), so shift, delete, enter and the action key do not contribute.

Scope

The two disagree whenever the centered offset is above zero, which is
numColumnsByRow >= QwertySymbols[row].size + 2. Row 2's reference is only 7
wide, so any third letter row of nine or more triggers it. (The bottom: row is a
different thing and is excluded from quick actions altogether, at
MoreKeysBuilder.kt:127.) Layouts that set
moreKeyMode to something other than All never get these keys at all
(BaseKey.kt:89-109), so they are out of scope.

Checked directly:

layout	widths	offsets	quick actions wrong on
qwerty, qwertz	10/9/7	0/0/0	nothing
azerty	10/10/7	0/0/0	nothing
german	11/11/7	0/0/0	nothing
east_slavic, belarusian, ukrainian	11/11/9	0/0/1	row 2
ukrainian_g	12/12/9	1/1/1	all three rows

The standard shapes are exempt because their third letter row is 7 wide, which is why
this survived: qwerty, qwertz, azerty, dvorak, colemak and workman
are all clean. pcqwerty is not, at 13/11/10, and neither is swiftdvorak at
8/10/9. Every ЙЦУКЕН layout has a 9-wide third letter row and is not, so
the standard Russian layout is affected on default settings in the shipped
release. Most non-Latin layouts have a third letter row of nine or more and are in the
same position; a row of exactly eight is unaffected, since (8 - 7) / 2
truncates to zero; I have not tried to put an exact count on it, since it turns
on per-row and per-key moreKeyMode as well as width.

This affects Quick Actions on long press, which is in the shipped default long
press order (LongPressKeyLayoutSetting), so it reaches anyone who has not gone
into long press settings and removed it.

Fix

Copy the three lines from symsForCoord, including the coerceAtLeast(0) and the negative-column guard, so actionForCoord centers the same way. The branch does that and nothing else.

Branch: https://github.com/AndrewKirkovski/android-keyboard/tree/fix/quick-actions-column-offset

A related disagreement I have left alone

getNumForCoordinate centers too, but against a hardcoded 10 rather than a row size, and it has no coerceAtLeast(0). It only runs on row 0, where QwertySymbols is 10 wide, so the hardcoded value is harmless. The missing clamp is not: on a first row of eight or fewer the offset goes negative, so each key resolves one digit higher than it should: an 8-wide row long presses to 2 through 9 rather than 1 through 8, while symbols clamp the offset to zero and stay put. tatar and nganasan are 8 wide and azerbaijani is 7, and the small Cyrillic and Sami layouts are mostly narrower still.

A first row of exactly nine is not affected, because Kotlin truncates integer division toward zero and (9 - 10) / 2 is 0 rather than -1. toki_pona, kazakh_latin, lithuanian_qwerty, sami_ume, belarusian_latin2 and yakut sit on that boundary.

Giving getNumForCoordinate the same clamp would change what every one of those narrow layouts long presses to today, so it is a separate call from this one.

[AndrewKirkovski]

Correcting my own numbers above: I originally wrote that QwertySymbols rows are 10/9/7 and that a 12/11/9 layout is off by one on every row. Both wrong — row 1 is "asdfghjkl" + "r2_e1" = 10, so the offsets come out 1 / 0 / 1 and row 1 is correct by coincidence. The bug and the fix are unchanged; the body now has the accurate table.

[AndrewKirkovski]

Checked this on an emulator rather than only from the source, using a 12/11/9 Cyrillic layout with Quick Actions bound to long-press.

Row 2 is 9 wide against QwertySymbols' 7, so the offset is 1 and every key should map one column to its left:

key	column	centered	qwerty	action before	action after (observed)
я	0	-1	—	undo (z)	none
ч	1	0	z	cut (x)	undo
с	2	1	x	copy (c)	cut
м	3	2	c	paste (v)	copy

This survived unnoticed because, before the patch, copy landed on Cyrillic с, which is shaped like a Latin "c". It looks right.