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

Issue with Double paste on Windows 10 #18130

Closed
Ygg01 opened this issue Aug 17, 2017 · 3 comments · Fixed by #20327
Closed

Issue with Double paste on Windows 10 #18130

Ygg01 opened this issue Aug 17, 2017 · 3 comments · Fixed by #20327
Labels
A-input P-windows Any version of Windows capable of running Servo

Comments

@Ygg01
Copy link

Ygg01 commented Aug 17, 2017

Steps to reproduce:

  • Copy link e.g. https://www.mozilla.org/
  • Paste in Servo search bar

Expected: https://www.mozilla.org/
Got: https://www.mozilla.org/https://www.mozilla.org/
Link to image

Environment: Windows 10, browser was from https://download.servo.org with today's date. Unsure if bug exists on other platforms.

NOTE: Also Ctrl-z leaves a token in URL field.

@atouchet
Copy link
Contributor

I have also run into this problem. I wrote a bit about it here: #16459 (comment). #17413 solved some of my issues with copying and pasting in Servo but it did not solve the double pasting URL problem. I am also using Windows 10.

@atouchet
Copy link
Contributor

#17146 may be related to this as well.

@jdm jdm added A-input P-windows Any version of Windows capable of running Servo labels Aug 17, 2017
@atouchet
Copy link
Contributor

This is still happening.

kwonoj added a commit to kwonoj/servo that referenced this issue Mar 18, 2018
bors-servo pushed a commit that referenced this issue Mar 22, 2018
fix(keyevent): do not emit default ignorable codepoint

<!-- Please describe your changes on the following line: -->
This PR intends to update `KeyEvent` emit behavior around #18130. Issue #17146 (comment) briefly explains what's happening in servo currently - there are KeyboardInput event emitted for separated key (modifier, and `V` in case of paste) and there are also `ReceivedCharacter` event corresponds to `\u{0016}`.

`0x0016` is unicode representation of `Synchronous Idle` (https://en.wikipedia.org/wiki/Synchronous_Idle), belong under category of `Default Ignorable` charater in unicode range doesn't have visual representation (http://www.unicode.org/L2/L2002/02368-default-ignorable.pdf). Currently servo forwards all of emitted event from `winit` including this, eventually leads into double execution of control event.

In this change try to omit default ignorable charater , if given char received is within range of ignorable do not dispatch `KeyEvent`. Once those are omitted, current event handling logic already takes care of key event with correct modifier state so duplicated event handling won't occur.

For implementation perspective, `std::char` in Rust doesn't seem to support `isIdentifierIgnorable` like other platform does (i.e: https://msdn.microsoft.com/en-us/library/aa285330(v=vs.60).aspx / https://docs.oracle.com/javase/7/docs/api/java/lang/Character.html) - so does quick, naïve range comparison check based on unicode range specified in spec, similar to halfbuzz and other does. (https://github.com/harfbuzz/harfbuzz/blob/master/src/hb-unicode-private.hh#L134)

Lastly, this is indeed behavior of `winit` to emit all characters by default. *Why not try to make upstream changes instead?* While I've been reading through issues in `winit`, issue like rust-windowing/winit#350 trying to emit ignorable character by its intention (delete key `ReceivedCharacter` is also under category of default ignorable) and let each consumer application handles it as needed. I assume it'll cause breaking changes in winit's design if it intends to omit those characters, instead tried to make application-level changes.

Couple of consideration for review

- Is it desired changes to not emit `KeyEvent` for default ignorable chars? Do we rather want mapping / or restoring back to original char as @paulrouget mentioned in #17146 (comment)?
- Any better, recommended approach to detect unicode char range?
- Maybe try to make upstream changes to `winit` still, like having configuratble way to opt-in(out) those char event?

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #18130 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____
- This PR has been locally tested on Windows, Linux machines.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20327)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-input P-windows Any version of Windows capable of running Servo
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants