bug(web): `Uncaught TypeError: this.getTextBeforeCaret() is undefined`

mcdurdin commented 7 months ago

Reported by @jmdurdin. KMW 16.0.145.

Uncaught TypeError: this.getTextBeforeCaret() is undefined

    getDeadkeyCaret contentEditable.ts:105
    buildTransformFrom outputTarget.ts:122
    buildTranscriptionFrom outputTarget.ts:200
    process kbdInterface.ts:1081
    processNewContextEvent kbdInterface.ts:1014
    processNewContextEvent keyboardProcessor.ts:239
    processNewContextEvent inputProcessor.ts:73
    resetContext inputProcessor.ts:358
    resetContext kmwbase.ts:509
    _ControlBlur domEventHandlers.ts:200
    attachDOMEvent kmwutils.ts:199
    enableInputElement domManager.ts:342
    attachToControl domManager.ts:442
    _SetupDocument domManager.ts:713
    init domManager.ts:1547
    init kmwbase.ts:490
    init t.html:7
    <anonymous> t.html:41
    EventListener.handleEvent* t.html:41

contentEditable.ts:105:13

Sample page which produced the error:

<html>
<head>
  <script src='https://s.keyman.com/kmw/engine/16.0.145/keymanweb.js'></script>
  <script src='https://s.keyman.com/kmw/engine/16.0.145/kmwuibutton.js'></script>
  <script>
    init = function () {
      keyman.init({
        attachType: 'auto'
      }).then(function () {
        keyman.addKeyboards('@th', '@lo');
        var pt = document.getElementById('thai'),
          pl = document.getElementById('lao');
        ps = document.getElementById('cd');
        if (pt) keyman.setKeyboardForControl(pt, 'Keyboard_basic_kbdth0');
        if (pl) keyman.setKeyboardForControl(pl, 'Keyboard_basic_kbdlao');
        if (ps) keyman.setKeyboardForControl(ps, 'Keyboard_basic_kbdth0');
      });
    }

    window.addEventListener('load', () => { init(); });
  </script>
  <style>
    .main {
      padding: 4em;
    }
    input {
      margin: 2em;
      width: 10em
    }
    div#cd {
      display: block;
      position: relative;
      left: 4em;
      top: 0;
      height: 4em;
      width: 40em;
      border: 1px solid blue;
      background-color: pink;
    }
  </style>
</head>
<body>
  <div class='main'>
    <div contenteditable id='cd'></div>
    <label for 'thai'>Thai</label>
    <input id='thai' />
    <label for 'lao'>Lao</label>
    <input id='lao' />
  </div>
</body>
</html>

jahorton commented 7 months ago

Editing the sample page to...

<html>
<head>
  <!-- Addition:  fixes touch-OSK layout issues -->
  <meta http-equiv="content-type" content="text/html; charset=utf-8" />

  <!-- Set the viewport width to match iOS device widths -->
  <!-- <meta name="viewport" content="width=device-width,initial-scale=1.0,maximum-scale=1.0,minimum-scale=1.0,user-scalable=no" /> -->
  <meta name="viewport" content="width=device-width,user-scalable=no" />
  <meta name="apple-mobile-web-app-capable" content="yes" />
  <!-- end addition re: touch-OSK layout issues -->

  <script src='./build/publish/debug/keymanweb.js'></script>
  <script src='./build/publish/debug/kmwuibutton.js'></script>
  <script>
    init = function () {
      keyman.init({
        attachType: 'auto'
      }).then(function () {
        keyman.addKeyboards('@th', '@lo').then(() => {
          // note:  the `.then` block we're in didn't exist originally.  
          // I needed it so that the keyboard-query will have resolved and returned before proceeding.
          var pt = document.getElementById('thai'), pl = document.getElementById('lao');
          ps = document.getElementById('cd');
          if (pt) keyman.setKeyboardForControl(pt, 'Keyboard_basic_kbdth0');
          if (pl) keyman.setKeyboardForControl(pl, 'Keyboard_basic_kbdlao');
          if (ps) keyman.setKeyboardForControl(ps, 'Keyboard_basic_kbdth0');
        });
      });
    }

    window.addEventListener('load', () => { init(); });
  </script>
  <style>
    .main {
      padding: 4em;
    }
    input {
      margin: 2em;
      width: 10em
    }
    div#cd {
      display: block;
      position: relative;
      left: 4em;
      top: 0;
      height: 4em;
      width: 40em;
      border: 1px solid blue;
      background-color: pink;
    }
  </style>
</head>
<body>
  <div class='main'>
    <div contenteditable id='cd'></div>
    <label for 'thai'>Thai</label>
    <input id='thai' />
    <label for 'lao'>Lao</label>
    <input id='lao' />
  </div>
</body>
</html>

...and placing it within the ${KEYMAN_ROOT}/web folder shows that this error does not exist for 17.0. I never saw anything even closely related to the error reported in the main issue body.

jahorton commented 7 months ago

For 16.0 use:

<html>
<head>
  <!-- Addition:  fixes touch-OSK layout issues -->
  <meta http-equiv="content-type" content="text/html; charset=utf-8" />

  <!-- Set the viewport width to match iOS device widths -->
  <!-- <meta name="viewport" content="width=device-width,initial-scale=1.0,maximum-scale=1.0,minimum-scale=1.0,user-scalable=no" /> -->
  <meta name="viewport" content="width=device-width,user-scalable=no" />
  <meta name="apple-mobile-web-app-capable" content="yes" />
  <!-- end addition re: touch-OSK layout issues -->

  <script src='./release/web/keymanweb.js'></script>
  <script src='./release/web/kmwuibutton.js'></script>
  <script>
    init = function () {
      keyman.init({
        attachType: 'auto'
      }).then(function () {
        keyman.addKeyboards('@th', '@lo').then(() => {
          // note:  the `.then` block we're in didn't exist originally.
          // I needed it so that the keyboard-query will have resolved and returned before proceeding.
          var pt = document.getElementById('thai'), pl = document.getElementById('lao');
          ps = document.getElementById('cd');
          if (pt) keyman.setKeyboardForControl(pt, 'Keyboard_basic_kbdth0');
          if (pl) keyman.setKeyboardForControl(pl, 'Keyboard_basic_kbdlao');
          if (ps) keyman.setKeyboardForControl(ps, 'Keyboard_basic_kbdth0');
        });
      });
    }

    window.addEventListener('load', () => { init(); });
  </script>
  <style>
    .main {
      padding: 4em;
    }
    input {
      margin: 2em;
      width: 10em
    }
    div#cd {
      display: block;
      position: relative;
      left: 4em;
      top: 0;
      height: 4em;
      width: 40em;
      border: 1px solid blue;
      background-color: pink;
    }
  </style>
</head>
<body>
  <div class='main'>
    <div contenteditable id='cd'></div>
    <label for 'thai'>Thai</label>
    <input id='thai' />
    <label for 'lao'>Lao</label>
    <input id='lao' />
  </div>
</body>
</html>

(The file-system layout was restructured during 17.0 alpha.)

The added .then() tidbit isn't needed on 16.0, it seems - there wasn't robust checking that the keyboard being set... was actually a valid, available keyboard. 17.0 double-checks that there's an actual stub first... but to do so, it needs to actually have the stub.

Going closer to the original...

<html>
<head>
  <meta http-equiv="content-type" content="text/html; charset=utf-8" />

  <!-- Set the viewport width to match iOS device widths -->
  <!-- <meta name="viewport" content="width=device-width,initial-scale=1.0,maximum-scale=1.0,minimum-scale=1.0,user-scalable=no" /> -->
  <meta name="viewport" content="width=device-width,user-scalable=no" />
  <meta name="apple-mobile-web-app-capable" content="yes" />
  <script src='./build/publish/debug/keymanweb.js'></script>
  <script src='./build/publish/debug/kmwuibutton.js'></script>
  <script>
    init = function () {
      keyman.init({
        attachType: 'auto'
      }).then(function () {
        keyman.addKeyboards('@th', '@lo')//.then(() => {
          var pt = document.getElementById('thai'), pl = document.getElementById('lao');
          ps = document.getElementById('cd');
          if (pt) keyman.setKeyboardForControl(pt, 'Keyboard_basic_kbdth0');
          if (pl) keyman.setKeyboardForControl(pl, 'Keyboard_basic_kbdlao');
          if (ps) keyman.setKeyboardForControl(ps, 'Keyboard_basic_kbdth0');
        //});
      });
    }

    window.addEventListener('load', () => { init(); });
  </script>
  <style>
    .main {
      padding: 4em;
    }
    input {
      margin: 2em;
      width: 10em
    }
    div#cd {
      display: block;
      position: relative;
      left: 4em;
      top: 0;
      height: 4em;
      width: 40em;
      border: 1px solid blue;
      background-color: pink;
    }
  </style>
</head>
<body>
  <div class='main'>
    <div contenteditable id='cd'></div>
    <label for 'thai'>Thai</label>
    <input id='thai' />
    <label for 'lao'>Lao</label>
    <input id='lao' />
  </div>
</body>
</html>

I'm not getting a reproduction of the error. Even swapping back to the s.keyman.com link isn't giving me the error reported.

jahorton commented 7 months ago

One thing I'm noticing that's likely at least somewhat at play - that call stack doesn't make sense given the test page for one reason: it appears that one of the page's input elements is already focused when the page is loaded, and that focus is being lost, somehow, during the KMW init process. I don't see anything in the page setup that would trigger this, though.

Noting the linked #10621, perhaps there's a browser extension interaction at play here?

mcdurdin commented 7 months ago

it appears that one of the page's input elements is already focused when the page is loaded

I think this can happen if you reload the page while the element is focused, on some browsers at least

jahorton commented 7 months ago

it appears that one of the page's input elements is already focused when the page is loaded

I think this can happen if you reload the page while the element is focused, on some browsers at least

Using that, I occasionally get this in Firefox:

Mind you, this is with me alternating between typing F5 + clicking on a page element once the page loads. It's definitely not the same error, though.

jmdurdin commented 7 months ago

It is possible that an input element is focused when page loaded, but the error occurs quite consistently after page loading when moving the focus between the content-editable DIV (not an input element) and somewhere other than an input element.

I have also seen the error shown in the image posted but put it down to timing out due to internet delay. Sometimes see a little 'Loading keyboard...' window shown, too.

jahorton commented 7 months ago

This is likely related to #10693.

jahorton commented 7 months ago

This is now confirmed as having the same cause as #10693; the stack trace matches and the identified cause there would absolutely trigger the error for this PR as well. Fortunately, it's already fixed in 17.0.

keymanapp / keyman

bug(web): `Uncaught TypeError: this.getTextBeforeCaret() is undefined` #10620