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 https://github.com/servo/servo/issues/17146#issue-233361568 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 https://github.com/tomaka/winit/issues/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 https://github.com/servo/servo/issues/17146#issue-233361568?
- 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 -->
Use typed coordinates more
Requires https://github.com/servo/servo/pull/19895
We use Size2D and Point2D across compositing, constellation and script, loosing the type of pixels we use (DevicePixel, DeviceIndepententPixel or CSSPixel) along the way, which might lead to bugs like `window.outerHeight` not taking into account the page zoom (using DeviceIndepententPixel instead of CSSPixel).
This should make the situation a bit better.
---
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).
<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because we can't zoom in a test
<!-- 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/20071)
<!-- Reviewable:end -->
We use Size2D and Point2D across compositing, constellation and script,
losing the type of pixels we use (DevicePixel, DeviceIndepententPixel
or CSSPixel) along the way, which might lead to bugs like
window.outerHeight not taking into account the page zoom (using
DeviceIndepententPixel instead of CSSPixel).
In Winit, the Resize event has multiple purposes (actual resize,
screen change, workspace change, …). So we might get a Resize
event even if the window size hasn't changed yet. For example,
on MacOS, when going fullscreen.
Servo assume a resize and request a sync paint, which will freeze the
app. See:
0fa3248723/components/compositing/compositor.rs (L1485)
This disables bindgen's `logging` feature, which builds `env_logger`
with default features, including regex support. Disabling it allows
Gecko to build `env_logger` without the `regex` crate, reducing code
size.
Part of https://bugzilla.mozilla.org/show_bug.cgi?id=1444097
Prompt URL on Cmd/Ctrl-L
<!-- Please describe your changes on the following line: -->
The `sanitize_url` code is very naive. I'm sure we can do better.
This ServoShell issue describes the problem: https://github.com/paulrouget/servoshell/issues/59
I can fix that now if someone can help me figure out how to tell if a string is a valid url which is just missing a scheme. Or we can do that in a follow up.
---
<!-- 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#20165
<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____
<!-- 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/20182)
<!-- Reviewable:end -->
style: Make 'font' shorthand reset 'font-variation-settings' property
As required by the spec: https://drafts.csswg.org/css-fonts-4/#font-prop
See https://bugzilla.mozilla.org/show_bug.cgi?id=1435983
Basically, make font-variation-settings work in the same way as font-feature-settings
already does.
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because font-variation-settings isn't supported in servo; it is implemented here for gecko/stylo, and will be tested by mozilla-central mochitests.
<!-- 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/20191)
<!-- Reviewable:end -->
style: Only expose longhands to rust via iterators.
The key here is that we only filter longhands if the shorthand is accessible to
content and vice-versa. This prevents the bug that prevented me to land this
patch before, which was us not expanding properly chrome-only shorthands.
<!-- 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/20138)
<!-- Reviewable:end -->
The key here is that we only filter longhands if the shorthand is accessible to
content and vice-versa. This prevents the bug that prevented me to land this
patch before, which was us not expanding properly chrome-only shorthands.
Again, this is incomplete, and I need to teach LonghandsToSerialize to get a
potentially incomplete list of properties, and all that.
More improvements to come. In particular, this still iterates through Shadow DOM
in each_xbl_cascade_data, but that should be changed later. That allows to
cleanup a bunch of stuff and finally fix Shadow DOM cascade order.
We still rely on the binding parent to be setup properly in the shadow tree, but
that requirement can go away later (we can walk the containing shadow chain
instead).
This mostly focuses on removing the XBL binding from the Shadow host.
It'd be nice to do EnumerateShadowRoots faster. I think that should also be a
followup, if needed.
Bug: 1425759
Reviewed-by: xidorn
MozReview-Commit-ID: Jf2iGvLC5de
