Let input JS event be dispatched by keydown instead of keypress#37078
Merged
xiaochengh merged 1 commit intoservo:mainfrom May 28, 2025
Merged
Let input JS event be dispatched by keydown instead of keypress#37078xiaochengh merged 1 commit intoservo:mainfrom
input JS event be dispatched by keydown instead of keypress#37078xiaochengh merged 1 commit intoservo:mainfrom
Conversation
Contributor
|
There should be some test cases using testdriver.js. Have you checked locally? And I still don't feel so good to merge a patch without CI coverage... What's the priority of this issue? |
Member
Author
|
🔨 Triggering try run (#15203430038) for Linux (WPT) |
Member
Author
|
Actually I ran the action yesterday.. Everything is passing because this requires testdriver to check. |
Member
|
Does #37081 fix the test driver issue? |
Member
Author
We still won't be able to run testdriver after #37081. Because synchronization PR #36932 would make all related test "TIMEOUT" due to greatly increased chance of failing hit-test. |
|
Test results for linux-wpt from try job (#15203430038): Flaky unexpected result (18)
Stable unexpected results that are known to be intermittent (17)
|
|
✨ Try run (#15203430038) succeeded. |
Contributor
Correct: PR #36932 add a blocking wait in |
Member
Author
This PR will make everything easier for embedder & related event fix, according to @stevennovaryo I think we should still review and merge this. Since there are multiple |
xiaochengh
reviewed
May 28, 2025
Contributor
xiaochengh
left a comment
xiaochengh
left a comment
There was a problem hiding this comment.
LGTM but please remove unused code
Contributor
OK, that's a fair justification. |
Signed-off-by: Euclid Ye <[email protected]>
xiaochengh
approved these changes
May 28, 2025
github-merge-queue bot
pushed a commit
that referenced
this pull request
May 30, 2025
At `egl/app_state.rs`, send keydown and keyup with PROCESS KEY when inserting text. This fixes OHOS input event, but maybe also for Android in the future (if it implements `ime_insert_text`). We will get input event since keydown is dispatched (#37078). This implementation is similar to [Chromium's](https://source.chromium.org/chromium/chromium/src/+/main:content/public/android/java/src/org/chromium/content/browser/input/ImeAdapterImpl.java;drc=404e8d654e8b26336d2cb103b9c21faecbf7f73a;bpv=1;bpt=1;l=851?gsn=sendCompositionToNative&gs=KYTHE%3A%2F%2Fkythe%3A%2F%2Fchromium.googlesource.com%2Fcodesearch%2Fchromium%2Fsrc%2F%2Fmain%3Flang%3Djava%3Fpath%3Dorg.chromium.content.browser.input.ImeAdapterImpl%23d840961d441fd4bb569f9689c86da91fb714c0c153366e3198a85e9c7a098dce) Android key event. Testing: manually checked on OHOS device For: #36259, but only in OHOS Signed-off-by: PotatoCP <[email protected]> Co-authored-by: PotatoCP <[email protected]> Co-authored-by: stevennovaryo <[email protected]>
PotatoCP
pushed a commit
to PotatoCP/servo
that referenced
this pull request
Jun 4, 2025
…servo#37078) 1. Let `input` JS event be dispatched by `keydown` instead of `keypress`, according to spec 2. Fire `input` event for Backspace and Delete. But do so only when something is actually deleted Testing: Manually tested and compared with other browsers. Fixes: servo#37051 cc @xiaochengh Signed-off-by: Euclid Ye <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
inputJS event be dispatched bykeydowninstead ofkeypress, according to specinputevent for Backspace and Delete. But do so only when something is actually deletedTesting: Manually tested and compared with other browsers.
Fixes: #37051
cc @xiaochengh