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.

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

fix(window): set enter key as non-printable char #20727

Closed
wants to merge 1 commit into from

Conversation

@kwonoj
Copy link
Contributor

kwonoj commented May 1, 2018

Mostly for discussion around #20630, #20726.
If this PR's accepted & merged, it may serve as temporal workaround only.

This PR changes behavior of window to consider ENTER key as non-printable, let document does not appends redundant carriage return when press enter on textinput field. But this PR instead omits keypress event on Enter like

image

while other browsers does emit
image

makes me feel proper fix would be something like #20726, revise flow to send over keyevent to document itself.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #20630 (github issue number if applicable).
  • There are tests for these changes OR
  • These changes do not require tests because _____

This change is Reviewable

@highfive
Copy link

highfive commented May 1, 2018

Heads up! This PR modifies the following files:

@paulrouget
Copy link
Contributor

paulrouget commented May 3, 2018

Having proper IME support would help solve these kind of issues.

@bors-servo
Copy link
Contributor

bors-servo commented Sep 12, 2018

The latest upstream changes (presumably #21676) made this pull request unmergeable. Please resolve the merge conflicts.

@kwonoj
Copy link
Contributor Author

kwonoj commented Sep 12, 2018

Let me close this as upstream has changed.

@kwonoj kwonoj closed this Sep 12, 2018
@kwonoj kwonoj deleted the kwonoj:fix-append-cr branch Sep 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants
You can’t perform that action at this time.