-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[Feature] IME support #5777
Comments
Thanks for taking the time to write this feature up @JoelEinbinder. Testing the complexities around IME is really important to us at Facebook, both for our internal projects and also our open-source projects. Your initial proposal looks good, however, I'd maybe opt for this not to be on For keyboard presses, or
I believe we just want to control it. I think we can do some investigation work around this after we have a MVP and see from that how accurately we can model test cases around user workflows.
I think we should be careful here. As this behavior can change depending on keyboard layout if I'm not mistaken, which makes tests harder to properly emulate. However, I might be wrong, maybe this is something we should consider.
Dead keys aren't always fired, so I don't think we need to worry too much about them. For example, on a Mac, holding down |
The cursor should change on
Its definitely layout specific. We currently have the US keyboard layout mapped out. Adding dead keys and other layouts is not too hard, but it sounds like you don't need this.
It doesn't. It also doesn't fire composition start/end, but rather does a text replacement for the character before the cursor. The web page sees the initial keydown/ Can you give an example of where a component breaks with IME? You have to be doing something a little bit strange for composition events to be relevant, so I want to make sure I understand what we are testing for. |
If I hold down Another interesting thing is that when you use a software keyboard with FF, it uses composition for insertions. On Safari and Chrome it does not, it just does text replacement. These edge cases are where the bulk of bugs we see at FB stem from. Another issue we found was around Japanese/Korean IME input. For years the last character of a word entered via IME on Draft meant the last character went missing! Don't get me started on Android IME issues too. :P I'd be happy to setup a video call at some point if you wanted to talk about this and dive into more specifics. |
Well if you are building a rich text editor or a code editor, that's a problem I can wrap my head around. I think the lower level api makes a lot of sense, because anyone building an editor will have enough expertise to send the exact events they need rather than relying on Playwright to make an educated guess based on browser/platform/language. No promises on time frame, but I'll make a prototype in webkit and see what the api looks like. |
@JoelEinbinder That's awesome. Thank you again for this. Another interesting idea might be if there was a way to record the native user invoked events in the form of Playwright API commands. We can obviously do a hacky form of this today by adding capture listeners to a page and recording all the events, and convert them into Playwright tests, but we want to distinguish between events that the user makes vs that from a library or framework. |
Check out Playwright codegen. I'd make it work with IME after adding this api. |
@JoelEinbinder It would also be good if we had a way of selecting a range of text with Playwright. Something like: await page.composition.setSelection(
'div.editor',
anchorPath,
anchorOffset,
focusPath,
focusOffset,
);
Where the paths are the DOM path from the given element selector (i.e. [0, 1, 0]). I realize you can kind of do this with `page.evaluate` but it's good to get a guarantee that selection was applied before continuing (as it's async). Otherwise things listening to `selectionchange` may not correctly get triggered. This is useful when emulating how software keyboards change selection and insert text (without triggering key events). |
Here are some visuals for the folks following along. Long-press diacritic composition on Mac OSHold down the ‘e’ key then press TAB, Left/RightArrow, or 1–7 to select a diacritic. Screen.Recording.2021-03-10.at.12.13.48.PM.movLong-press diacritic composition on Twitter in ChromeSame as above. Broken, as you can see – you get an unintended ‘e’. Screen.Recording.2021-03-10.at.12.15.09.PM.movHiragana keyboard input on Mac OSUse the Japanese – Romaji keyboard then type ‘watashi’ followed by TAB several times. Screen.Recording.2021-03-10.at.11.41.09.AM.movHiragana keyboard input on FacebookSame as above, in the Facebook post composer. Broken – pressing TAB commits the composition instead of moving you to the next selection in the candidate window. Screen.Recording.2021-03-10.at.11.39.48.AM.mov |
@JoelEinbinder I've been use Playwright internally for our new text editor and the experience has been highly positive. I was wondering if you had any updates, or if there was any way we could help contribute towards this feature, even if it's to get it into an experimental form? |
I have a patch working for chromium which replicates the draft js bug.
Webkit is pretty much done, and next up is Firefox.
Everything seems to be going smoothly, but if you want to help or would be
awesome to have a minimal repro of the draft js bug. I think it’s a good
way to verify that composition events are behaving realistically, but I
don’t really want to bundle draft js into our test suite.
…On Tue, Mar 23, 2021 at 1:56 PM Dominic Gannaway ***@***.***> wrote:
@JoelEinbinder <https://github.com/JoelEinbinder> I've been use
Playwright internally for our new text editor and the experience has been
highly positive. I was wondering if you had any updates, or if there was
any way we could help contribute towards this feature?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5777 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABDI62PUX5T4BXT3PSYMGJTTFD57DANCNFSM4Y5IVUCA>
.
|
@JoelEinbinder This is an awesome update! Do you have a working branch, so I can see how you've done this? I'm really interested in getting a better understanding of what happens under the hood. In terms of replicating what Draft does in a unit test might be tricky, as the project is largely unmaintained. However, the cause is likely to do with the fact that Draft doesn't apply the same selection that occurs during |
@JoelEinbinder Quickly catching up. Do you have any updates? Is there anything you need from my side of things? Can I help anywhere too? |
Chromium patch in review: https://chromium-review.googlesource.com/c/chromium/src/+/2764592 |
Yay! v1.13.0 sounds promising! |
@mxschmitt @JoelEinbinder are there any updates regarding this feature? Is there anything blocked that we can help with from our side? |
Hi there 👋 I'm currently working on the next version of Quill, and IME support is a crucial component as many bugs have arisen on the Quill repo due to IME. Would like to see if there is anything I can do to help to move this forward. |
I was just looking into this ticket (I've been working on a related one, #24249). CDP currently has an Input.imeSetComposition which seems to work, at least partially: test('ime', async ({ page }) => {
await page.goto('https://w3c.github.io/uievents/tools/key-event-viewer.html');
const client = await page.context().newCDPSession(page);
await page.locator('#input').focus();
await client.send('Input.imeSetComposition', {
selectionStart: -1,
selectionEnd: -1,
text: '😂'
});
// to cancel
await client.send('Input.imeSetComposition', {
selectionStart: -1,
selectionEnd: -1,
text: ''
});
}) Cancel the composition is supported, but I'm trying to figure out how to commit it. In their docs, they state:
I'm assuming that |
I look a little bit into chromium code, and it seems that the trick to commit is to ensure that this call is made: And that's what So, I tried to use it and it apparently is triggering very realistic events: test('ime', async ({ page }) => {
await page.goto('https://w3c.github.io/uievents/tools/key-event-viewer.html');
const client = await page.context().newCDPSession(page);
await page.locator('#input').focus();
await client.send('Input.imeSetComposition', {
selectionStart: -1,
selectionEnd: -1,
text: '😂😂',
});
await client.send('Input.imeSetComposition', {
selectionStart: 1,
selectionEnd: 2,
text: '😭',
});
// to commit
await client.send('Input.insertText', {
text: '😂😭',
});
}); |
@ruifigueira Thanks for the info! The CDP approach works good and we've already used it to cover some regressions 👍. Hope it would work on other browsers, especially on Safari as most IME bugs that we encountered happened on Safari 😄 |
@luin, if you really, really need to have import { test, expect } from '@playwright/test';
test('test', async ({ page, playwright, browserName }) => {
test.skip(browserName !== 'webkit');
await page.goto('https://w3c.github.io/uievents/tools/key-event-viewer.html');
// hack to get WKSession
const session = (await (playwright as any)._toImpl(page))._delegate._session;
// triggers a compositionstart and compositionupdate. Available parameters:
// https://github.com/microsoft/playwright/blob/175ae09fefb971236e984a20667cd04a52aa29d0/packages/playwright-core/src/server/webkit/protocol.d.ts#L7151-L7157
await session.send('Page.setComposition', { text: 'example', selectionStart: 0, selectionLength: 0 });
await expect(page.locator('#output span').filter({ hasText: 'compositionstart' })).toHaveCount(1);
await expect(page.locator('#output span').filter({ hasText: 'compositionupdate' })).toHaveCount(1);
await expect(page.locator('#output span').filter({ hasText: 'compositionend' })).toHaveCount(0);
await expect(page.locator("#input")).toHaveValue('example');
await page.pause();
// commits (triggers compositionend)
await page.keyboard.insertText("example commited");
await expect(page.locator('#output span').filter({ hasText: 'compositionend' })).toHaveCount(1);
await page.pause();
}); You'll need to launch tests with |
@ruifigueira Thanks for the insight! This approach works well and I've already used it in Quill repo. It successfully covers all the IME issues we've encountered. It'd be awesome to see something similar get implemented officially |
I think that IME in chrome and webkit could be easily implemented. The major problem is with firefox, which requires changes in juggler. There's a PR #9921 with some work on that but it's incomplete |
Unfortunately, this solution is not working anymore in 1.40+ because |
You're right. I tried with
I think webkit patch was updated to remove that, but the patch in the project doesn't reflect those changes because it still contains that instruction: |
Is there any update on potential support for IME input in Playwright? |
Right now our IME support is limited. We insert text, but do not allow for testing complicated scenarios with composition events.
The Web api: https://developer.mozilla.org/en-US/docs/Web/API/CompositionEvent
The Mac api: https://developer.apple.com/documentation/appkit/nstextinputclient?language=objc
The Chromium mojo api: https://source.chromium.org/chromium/chromium/src/+/master:third_party/blink/public/mojom/input/input_handler.mojom
This request comes from @trueadm on twitter: https://twitter.com/trueadm/status/1369332999242792967
My initial api proposal:
The only time I've seen composition events used in the wild is in CodeMirror to draw a synthetic composition highlight. It would be helpful to have some better use cases here to make sure that the api handles them without growing too large.
keyboard.compositionRange()
?keyboard.typeWithComposition("è")
types AltGr+` followed by e?The text was updated successfully, but these errors were encountered: