The implementation of adjust_horizontal_to_limit() is written with UI in
mind. As such, when there's a selection and we "adjust horizontal", the
selection will be cleared and the cursor will and up at the start/end of
the previous selection. This is what happens when you have a selection
and you press an arrow key on your keyboard, but it isn't the behaviour
we want when programmatically changing the value.
Instead, we need to first clear the selection, and then move the cursor
to the end. (We also need to reset the selection direction when clearing
the selection.)
Per the spec, selectionStart and selectionEnd should return the same
values regardless of the selectionDirection. (That is, selectionStart is
always less than or equal to selectionEnd; the direction then implies
which of selectionStart or selectionEnd is the cursor position.)
There was no explicit WPT test for this, so I added one.
This bug was initially quite hard to wrap my head around, and I think
part of the problem is the code in TextInput. Therefore, in the process
of fixing it I have refactored the implementation of TextInput:
* Rename selection_begin to selection_origin. This value doesn't
necessarily correspond directly to the selectionStart DOM value - in
the case of a backward selection, it corresponds to selectionEnd.
I feel that "origin" doesn't imply a specific ordering as strongly as
"begin" (or "start" for that matter) does.
* In various other cases where "begin" is used as a synonym for "start",
just use "start" for consistency.
* Implement selection_start() and selection_end() methods (and their
_offset() variants) which directly correspond to their DOM
equivalents.
* Rename other related methods to make them less wordy and more
consistent / intention-revealing.
* Add assertions to assert_ok_selection() to ensure that our assumptions
about the ordering of selection_origin and edit_point are met. This
then revealed a bug in adjust_selection_for_horizontal_change() where
the value of selection_direction was not maintained correctly (causing
a unit test failure when the new assertion failed).
Use specific assertions
Similar to #19865
r? jdm
Note: Should I squash all the commits into one commit?
---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes do not require tests because it should not break anything
<!-- 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/19868)
<!-- Reviewable:end -->
This more concrete wrapper type can write a prefix the very first time something
is written to it. This allows removing plenty of useless monomorphisations caused
by the former W/SequenceWriter<W> pair of types.
Implement Element::has_css_layout_box()
r? emilio
Here's my initial attempt to fix#19430. It seems surprisingly simple so I am wondering whether I have missed something! (Or maybe it just actually is quite simple...)
Some things I am unsure about:
1. The spec seems vague about what a [CSS layout box](https://drafts.csswg.org/cssom-view/#css-layout-box) actually is. Indeed it says: "The terms CSS layout box and SVG layout box are not currently defined by CSS or SVG."
2. One thing the spec *does* say explicitly is "For the purpose of the requirements in this specification, elements that have a computed value of the display property that is table-column or table-column-group must be considered to have an associated CSS layout box (the column or column group, respectively)." I am unclear about the relevance of this, since [overflow does not apply](https://drafts.csswg.org/css-overflow-3/#overflow-properties) to an [internal table element](https://drafts.csswg.org/css-display-3/#layout-specific-display). Therefore I haven't done anything about this explicitly, but maybe I'm missing some nuance.
<!-- 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/19803)
<!-- Reviewable:end -->
Calling scroll() on an element which is not rendered (by a parent with
display: none) would previously cause a crash. In fact, we should
terminate the algorithm
[https://drafts.csswg.org/cssom-view/#dom-element-scroll] at step 10 in
this situation.
The fix hinges on implementing Element::has_css_layout_box() correctly,
rather than just returning true in all cases as we did previously.
Fixes#19430.
Implement the create an element for token algorithm
<!-- Please describe your changes on the following line: -->
---
<!-- 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#19392 and fix#19393 (github issue number if applicable).
<!-- 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/19397)
<!-- Reviewable:end -->
replace DocumentSource::NotFromParser with DocumentSource::FromParser
<!-- Please describe your changes on the following line: -->
per https://github.com/servo/servo/pull/4297/files#r158729975
---
<!-- 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
- [ ] 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/19647)
<!-- Reviewable:end -->
implement valid DatetimeLocal input
<!-- Please describe your changes on the following line: -->
implement valid Date time Local input
part of #19172
---
<!-- 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#19587 fix #19603(github issue number if applicable).
<!-- 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/19602)
<!-- Reviewable:end -->
