Keypresses

[zcorpan]

API to simulate keypresses. Milestone 3 in #15

---

Preview | Diff

[zcorpan]

From today's AT Automation Group Meeting (minutes)

We should have these commands:

• pause for a certain duration (like in WebDriver)
• pressKeys to press one or more keys in combination. Include a special modifier for the screen-reader specific modifier keys.

If we need keydown/keyup in the future, it can be added at that time.

sendKeys (which is "type text") doesn't need to be supported since it's a convenience command that is more relevant in WebDriver (for typing text into e.g. an <input> element).

The level of keyboard simulation should be such that the AT will respond to them, but there may be security risks with too low-level simulation.

For keys that the AT doesn't itself respond to, those keys should be forwarded to the OS/focused application (as normal if the user were to press the same keys).

[jkva]

Consider this review to also encompass the previous reviews requested for PRs, since these all incorporate each other.

The extract-schemas.js script is currently invalid because of added CDDL; see comments.

[jkva]

In order to understand the extraction script better, I've separated some code out into functions. Feel free to incorporate what seems useful to you.

import { Lexer, Parser } from 'cddl';
import * as fs from 'node:fs/promises';

const parseCddl = input => new Parser(new Lexer(input)).parse();

const cddlToJSONSchema = input => {
  const cddlAst = parseCddl(input);
  const jsonSchemaAst = {
    "$schema": "https://json-schema.org/draft/2020-12/schema",
    "$id": "TODO",
    "$defs": {},
  };
  for (const obj of cddlAst) {
    console.log(JSON.stringify(obj, null, 2)); // TODO
    // For SessionNewCommand, need to not set additionalProperties: false
    // since Command allows additional properties, all schemas in Command's allOf
    // need to also allow additional properties (otherwise 'id' will be invalid).
  }
}

const removeIndentation = (indentation, cddl) => {
  if (indentation.length === 0) {
    return cddl;
  }

  let indentationRegexp = new RegExp(`^${indentation}`);
  return cddl
      .split('\n')
      .map(line => line.replace(indentationRegexp, ''))
      .join('\n');
}

const formatCddl = cddl => cddl.join('\n\n').trim() + '\n';

const extractCddlFromSpec = async () => {
  const source = await fs.readFile('index.bs', { encoding: 'utf8' });
  const matches = [...source.matchAll(/^([ \t]*)<pre class=['"]cddl((?: [a-zA-Z0-9_-]+)+)['"]>([\s\S]*?)<\/pre>/gm)];

  const [local, remote] = matches.reduce(([local, remote], match) => {
    const [_, indentation, cssClass, content] = match;

    let isLocal = cssClass.indexOf(' local-cddl') > -1;
    let isRemote = cssClass.indexOf(' remote-cddl') > -1;

    if (!isLocal && !isRemote) {
      return [local, remote];
    }

    let cddl = removeIndentation(indentation, content.trim());

    if (isLocal) {
      local.push(cddl);
    } else {
      remote.push(cddl);
    }

    return [local, remote];
  }, [[], []])

  return [
      formatCddl(local),
      formatCddl(remote)
  ]
}

try {
  await fs.mkdir('schemas');
} catch(ex) {
  if (ex.code !== 'EEXIST') {
    throw ex;
  }
}

const [localCddl, remoteCddl] = await extractCddlFromSpec();

await fs.writeFile('schemas/at-driver-local.cddl', localCddl);
await fs.writeFile('schemas/at-driver-remote.cddl', remoteCddl);

// Convert CDDL to JSON Schema
cddlToJSONSchema(localCddl);
cddlToJSONSchema(remoteCddl);

[jkva]

@zcorpan I noticed you also ran into a problem with the module entry script being incorrect; I've submitted a PR which addresses this. Yet if this is not incorporated by the maintainer, we should consider forking the module.

[jugglinmike]

@zcorpan I noticed you also ran into a problem with the module entry script being incorrect; I've submitted a PR which addresses this. Yet if this is not incorporated by the maintainer, we should consider forking the module.

Much appreciated. I'm including a corrected version of the link so that GitHub recognizes the relationship between this pull request and that one:

christian-bromann/cddl#5

And in the mean time, the following change will allow the script in this patch to run (though I don't recommend relying on it indefinitely):

diff --git a/scripts/extract-schemas.js b/scripts/extract-schemas.js
index 0bd627d..582e0fc 100644
--- a/scripts/extract-schemas.js
+++ b/scripts/extract-schemas.js
@@ -1,4 +1,4 @@
-import { Lexer, Parser } from 'cddl';
+import { Lexer, Parser } from 'cddl/build/index.js';
 import * as fs from 'node:fs/promises';

[zcorpan]

Thank you for your review @jkva ! I've incorporated your script refactoring in #19 and I think all comments are now addressed.

I would like to merge each open PR in order so that the history is clear both in the commit log and in GitHub's UI. We've set up this repo to require an approved review in GitHub before merging PRs. Since you have reviewed this, it means #19 and #25 are also reviewed. Can you mark this PR and the other 2 as reviewed, please?

The Settings PR is not included here: #22. If you have reviewed that one, can you also approve it?

Thanks!

[s3ththompson]

@jkva could you take another look and mark as reviewed if this looks good to merge? thanks!

[jkva]

Not per se related to this PR, but I notice that lines 221 and 222 of index.bs refer to the key platform while it seems that ought to be platformName.

There are several lines where there is referral to a definition having been defined in CDDL. This seems redundant and perhaps ought to be written as a definition existing in as CDDL.

[zcorpan]

Not per se related to this PR, but I notice that lines 221 and 222 of index.bs refer to the key platform while it seems that ought to be platformName.

Good catch! Fix: #29

There are several lines where there is referral to a definition having been defined in CDDL. This seems redundant and perhaps ought to be written as a definition existing in as CDDL.

Hmm, yeah. Filed a new issue: #30

[zcorpan]

I've now updated the PR to specify interaction.pressKeys as discussed in #26 (comment)

Include a special modifier for the screen-reader specific modifier keys.

This is not done yet. How do we want to support this? Should there be a special string for each vendor in place of the raw key string, e.g. "nvda", "macOS VoiceOver", etc?

{
  "method": "interaction.pressKeys",
  "params": {
    "keys": ["nvda", "a"]
  }
}

Or a boolean property in InteractionPressKeysParameters to indicate that the screen reader specific modifier keys should be pressed?

{
  "method": "interaction.pressKeys",
  "params": {
    "keys": ["a"],
    "vendorModifier": true
  }
}

[zcorpan]

I filed a new issue #34 to track the above question.

As discussed in our call today, we could merge this PR without special modifier support and address that later.

[jugglinmike]

The final remote end step describes a pretty involved algorithm. I think it'd be more clear as a series of steps, particularly if the "implementation defined" part was formally structured as its own algorithm (e.g. "simulate keyboard interaction"). Given that this has already been accepted in its current form, though, I'll propose the refactoring in a follow-up patch.

[jugglinmike]

Here's that patch: #41