Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(web): prevents selection-clear for pure layer-switching multitaps #11232

Merged
merged 2 commits into from
Apr 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions common/web/input-processor/src/text/inputProcessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,20 @@ export default class InputProcessor {
if(keyEvent.baseTranscriptionToken) {
const transcription = this.contextCache.get(keyEvent.baseTranscriptionToken);
if(transcription) {
// Restores full context, including deadkeys in their exact pre-keystroke state.
outputTarget.restoreTo(transcription.preInput);
// Has there been a context change at any point during the multitap? If so, we need
// to revert it. If not, we assume it's a layer-change multitap, in which case
// no such reset is needed.
if(!isEmptyTransform(transcription.transform) || !transcription.preInput.isEqual(Mock.from(outputTarget))) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why both?

  1. transcription.transform (and in fact, transcription in general) does not provide any indication of deadkey changes.
  2. While certainly "bad keyboard design", there's nothing preventing a multitap component of a layer-switch key from manipulating the context. Should that occur, the correct thing to do is still to rewind the context.

Note that it is technically possible to have altered the context since the base tap... but if the current context state matches the original one anyway, there's no need to perform a reset - we've already done the rewind in one manner or other.


We're only safe to bypass the context-rewind mechanism if there truly has been no change to the context whatsoever. Deadkey changes are context changes, and to handle them requires the some of the special handling newly added by this PR.

// Restores full context, including deadkeys in their exact pre-keystroke state.
outputTarget.restoreTo(transcription.preInput);
}
/*
else:
1. We don't need to restore the original context, as it's already
in-place.
2. Restoring anyway would obliterate any selected text, which is bad
if this is a purely-layer-switching multitap. (#11230)
*/
} else {
console.warn('The base context for the multitap could not be found');
}
Expand Down
22 changes: 22 additions & 0 deletions common/web/keyboard-processor/src/text/deadkeys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ export class Deadkey {
return dk;
}

equal(other: Deadkey) {
return this.d == other.d && this.p == other.d && this.o == other.o;
}

/**
* Sorts the deadkeys in reverse order.
*/
Expand Down Expand Up @@ -151,6 +155,24 @@ export class DeadkeyTracker {
}
}

equal(other: DeadkeyTracker) {
if(this.dks.length != other.dks.length) {
return false;
}

const otherDks = other.dks;
const matchedDks: Deadkey[] = [];

for(let dk of this.dks) {
const match = otherDks.find((otherDk) => dk.equal(otherDk));
if(!match) {
return false;
}
}

return matchedDks.length == otherDks.length;
}

count(): number {
return this.dks.length;
}
Expand Down
12 changes: 12 additions & 0 deletions common/web/keyboard-processor/src/text/outputTarget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,18 @@ export class Mock extends OutputTarget {
this.text = this.getTextBeforeCaret() + s;
}

/**
* Indicates if this Mock represents an identical context to that of another Mock.
* @param other
* @returns
*/
isEqual(other: Mock) {
return this.text == other.text
&& this.selStart == other.selStart
&& this.selEnd == other.selEnd
&& this.deadkeys().equal(other.deadkeys());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If someone goes and makes a multitap key that only does deadkey stuff, the final check is required to be able to rewind deadkey manipulations. Otherwise, we'd get a lot of excess deadkeys during such a multitap due to not erasing deadkeys from taps before the final one.

}

doInputEvent() {
// Mock isn't backed by an element, so it won't have any event listeners.
}
Expand Down
Loading