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.

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

Jump to bottom

fix(web): fixes touch form-factor default kbd on cookieless keymanweb.com page load #9786

Merged
merged 1 commit into from
Oct 18, 2023
Merged

fix(web): fixes touch form-factor default kbd on cookieless keymanweb.com page load #9786

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 15 additions & 0 deletions web/src/app/browser/src/keymanEngine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,19 @@ export default class KeymanEngine extends KeymanEngineBase<BrowserConfiguration,

await super.init(totalOptions);

// Used by keymanweb.com; if no keyboard-cookie exists, we need this to trigger
Copy link
Member

Choose a reason for hiding this comment

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

Really only used by keymanweb.com? Not by almost every single site that uses keymanweb? After all, no sites will have cookies on initial use.

Copy link
Contributor Author

@jahorton jahorton Oct 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Most sites don't directly call keyman.register.

keymanweb.com's cdn/prog/keyboards.php resolves to this due to caching:

/* tier alpha, version 17.0, cached 2023-10-18T02:25:03+00:00 */
function addKeyboards() {
    keyman.register({
        "options": {
// ...

which is then called here:

https://github.com/keymanapp/keymanweb.com/blob/0dba6472d3300ef15a4820ee53953d0051333aa0/inc/head.php#L207-L215

  keyman.init({
    attachType:'auto',
    setActiveOnRegister: setActiveOnRegister
  }).then(function() {
    if(typeof afterInit == 'function') {
      afterInit();
    }
    if(typeof addKeyboards == 'function') {
      addKeyboards();

Thus, it completely bypasses KeymanWeb's addKeyboards API - there's no use of keyman.addKeyboards() or similar - instead using an unofficial, undocumented optimization. Therefore...

not by almost every single site that uses keymanweb

// default-stub selection on mobile devices so that a keyboard - and thus, the
// globe key - are accessible.
//
// The `super` call above initializes `keyboardRequisitioner`, as needed here.
this.keyboardRequisitioner.cloudQueryEngine.once('unboundregister', () => {
Copy link
Contributor Author

@jahorton jahorton Oct 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This event was added in #9304 to facilitate handling of keymanweb.com's direct call of keyman.register with cached keyboard lists.

if(!this.contextManager.activeKeyboard?.keyboard) {
// Autoselects this.keyboardRequisitioner.cache.defaultStub, which will be
// set to an actual keyboard on mobile devices.
this.setActiveKeyboard('', '');
}
});

this.contextManager.initialize(); // will seek to attach to the page, which requires document.body

// Capture the saved-keyboard string now, before we load any keyboards/stubs
Expand Down Expand Up @@ -213,6 +226,8 @@ export default class KeymanEngine extends KeymanEngineBase<BrowserConfiguration,
// If we tracked cloud requests and awaited a Promise.all on pending queries,
// we could handle that too.
this.contextManager.restoreSavedKeyboard(savedKeyboardStr);

await Promise.resolve();
}

get register(): (x: CloudQueryResult) => void {
Expand Down