Skip to content

fix(common/web): mock deadkey handling after rules manipulating context 📴 #7345

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.

Sign up for GitHub

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

Merged
jahorton merged 9 commits into from
Sep 29, 2022
Merged

fix(common/web): mock deadkey handling after rules manipulating context 📴 #7345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
cf10880
fix(common/web): mocks did not update deadkey positions on context ma…
jahorton Sep 22, 2022
fc105b1
chore(web): updates karma dev-dependency
jahorton Sep 22, 2022
cb8e0bc
chore(common): npm audit fix - ansi-regex update
jahorton Sep 22, 2022
5b2ba7a
chore(common/models): fixes script warning for unstarted action
jahorton Sep 22, 2022
12c1de7
change(common/models): better handling of prior commit
jahorton Sep 22, 2022
dce7ef7
fix(web): fixes for non-focus-related unit-test breaks
jahorton Sep 22, 2022
be6bf14
fix(web): vscode merge got in the way
jahorton Sep 22, 2022
fc7dfb9
Merge pull request #7346 from keymanapp/chore/web/update-toolchain
jahorton Sep 27, 2022
0bbc3e7
chore(web): reverts accidental branch creep
jahorton Sep 27, 2022
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
2 changes: 1 addition & 1 deletion common/predictive-text/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"@keymanapp/resources-gosh": "*",
"@types/node": "^10.17.21",
"chai": "^4.3.4",
"karma": "^6.3.4",
"karma": "^6.4.1",
"karma-browserstack-launcher": "^1.6.0",
"karma-chai": "^0.1.0",
"karma-chrome-launcher": "^2.2.0",
Expand Down
2 changes: 2 additions & 0 deletions common/web/keyboard-processor/src/text/outputTarget.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,12 +439,14 @@ namespace com.keyman.text {
if(dn > this.caretIndex) {
dn = this.caretIndex;
}
this.adjustDeadkeys(-dn);
this.text = this.text.kmwSubstr(0, this.caretIndex - dn) + this.getTextAfterCaret();
this.caretIndex -= dn;
}
}

insertTextBeforeCaret(s: string): void {
this.adjustDeadkeys(s._kmwLength());
Comment on lines +442 to +449
Copy link
Copy Markdown
Contributor Author

@jahorton jahorton Sep 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Compare:

keyman/web/source/dom/targets/touchAlias.ts

Lines 53 to 67 in 1a7ad09

deleteCharsBeforeCaret(dn: number): void {
if(dn > 0) {
let curText = this.getTextBeforeCaret();
if(this.getDeadkeyCaret() < dn) {
dn = this.getDeadkeyCaret();
}
this.adjustDeadkeys(-dn);
this.root.setTextBeforeCaret(curText.kmwSubstring(0, this.root.getTextCaret() - dn));
}
}
insertTextBeforeCaret(s: string): void {
this.adjustDeadkeys(s._kmwLength());
this.root.setTextBeforeCaret(this.root.getTextBeforeCaret() + s);
}

Touch-alias elements are relatively similar to Mocks - both are programmatically managed and modeled by KMW. Note how the former has code updating deadkeys... while the latter doesn't. This was almost certainly a simple oversight that only just now got caught.

Copy link
Copy Markdown
Member

@mcdurdin mcdurdin Sep 28, 2022

Choose a reason for hiding this comment

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

Why s._kmwLength() as opposed to s.kmwLength()?

Copy link
Copy Markdown
Contributor Author

@jahorton jahorton Sep 29, 2022

Choose a reason for hiding this comment

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

Most of the calls in that file use _kmwLength, so it follows the same pattern.

this.text = this.getTextBeforeCaret() + s + this.getTextAfterCaret();
this.caretIndex += s.kmwLength();
}
Expand Down
6 changes: 5 additions & 1 deletion common/web/lm-worker/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,16 @@ fi
# of typescript, we need to avoid this!
# TODO: we should try and rework this to avoid the need to manually wrap

if builder_start_action clean || builder_has_action build; then
if builder_start_action clean; then
npm run clean
builder_finish_action success clean
fi

if builder_start_action build; then
if ! builder_has_action clean; then
npm run clean
fi

# Ensure keyman-version is properly build (requires build script)
"$KEYMAN_ROOT/common/web/keyman-version/build.sh" || fail "Could not build keyman-version"

Expand Down
2 changes: 1 addition & 1 deletion common/web/lm-worker/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"@keymanapp/resources-gosh": "*",
"@types/node": "^10.17.21",
"chai": "^4.3.4",
"karma": "^6.3.4",
"karma": "^6.4.1",
"karma-browserstack-launcher": "^1.6.0",
"karma-chai": "^0.1.0",
"karma-chrome-launcher": "^2.2.0",
Expand Down
64 changes: 32 additions & 32 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion web/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"@sentry/cli": "2.2.0",
"chai": "^4.3.4",
"google-closure-compiler-java": "^20200224.0.0",
"karma": "^6.3.4",
"karma": "^6.4.1",
"karma-browserstack-launcher": "^1.6.0",
"karma-chai": "^0.1.0",
"karma-chrome-launcher": "^2.2.0",
Expand Down
4 changes: 2 additions & 2 deletions web/source/dom/domEventHandlers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ namespace com.keyman.dom {
doChangeEvent(_target: HTMLElement) {
if(DOMEventHandlers.states.changed) {
let event = new Event('change', {"bubbles": true, "cancelable": false});

// Ensure that touch-aliased elements fire as if from the aliased element.
if(_target['base'] && _target['base']['kmw_ip']) {
_target = _target['base'];
Expand Down Expand Up @@ -772,7 +772,7 @@ namespace com.keyman.dom {

const x = touch.screenX;
const y = touch.screenY;

// Allow content of input elements to be dragged horizontally or vertically
if(typeof this.firstTouch == 'undefined' || this.firstTouch == null) {
this.firstTouch={x:x,y:y};
Expand Down
3 changes: 1 addition & 2 deletions web/testing/attachment-api/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ <h2>KeymanWeb Sample Page - Attachment API Testing</h2>
<div id='DynamicInputs'><h3>Inputs:</h3></div>
<div id='DynamicTextareas'><h3>Textareas:</h3></div>
<div id='DynamicIFrames'><h3>IFrames:</h3>
<p><em>Note:</em> The iframe section should not actually attach/enable for touch devices
and does not support <code>setKeyboardForControl</code>.</p>
<p><em>Note:</em> The iframe section does not support <code>setKeyboardForControl</code>.</p>
</div>
<div id='DynamicDesignFrames'><h3>Design-mode IFrames:</h3>
<p><em>Note:</em> The iframe section should not actually attach/enable for touch devices
Expand Down
2 changes: 1 addition & 1 deletion web/unit_tests/cases/element_interfaces.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1455,7 +1455,7 @@ describe('Element Input/Output Interfacing', function() {
});

it('correctly maintains deadkeys', function() {
InterfaceTests.Tests.deadkeyMaintenance(InterfaceTests.TouchAlias);
InterfaceTests.Tests.deadkeyMaintenance(InterfaceTests.Mock);
});
});
});
Expand Down
2 changes: 1 addition & 1 deletion web/unit_tests/test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ get_browser_set_for_OS ( ) {
if [ $os_id = "mac" ]; then
BROWSERS="--browsers Firefox,Chrome,Safari"
elif [ $os_id = "win" ]; then
BROWSERS="--browsers Firefox,Chrome,IE,Edge"
BROWSERS="--browsers Firefox,Chrome,Edge"
else
BROWSERS="--browsers Firefox,Chrome"
fi
Expand Down