| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| ... | |
| |
|
|
|
|
|
|
|
|
|
|
| |
Handle "Multiple" attribute for email value sanitization
Fix code formatting
Tests for email value sanitization should pass
Make failing wpt tests pass
Change new_value => sanitized
|
| | |
|
| |
|
|
| |
Fix some is_in_doc -> is_connected mistakes
|
| | |
|
| |
|
|
| |
We can replace all uses of RootedReference for Option<T> by Option::deref calls.
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
| |
The following IDLs have the src/href attributes typed as a DOMString
while in the spec the attribute has been updated to be a USVString:
- HTMLIFrameElement
- HTMLImageElement
- HTMLInputElement
- HTMLLinkElement
- HTMLMediaElement
- HTMLScriptElement
|
| |\
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Forward WebDriver CompositionEvent
Dispatch composition events in JS.
Insert characters from composition events to text input.
CompositionEvents currently can only be
created by WebDriver and not by embedders.
<!-- 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
- [ ] 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/22224)
<!-- Reviewable:end -->
|
| | |
| |
| |
| |
| |
| |
| |
| | |
Dispatch composition events in JS.
Insert characters from composition events to text input.
CompositionEvents currently can only be
created by WebDriver and not by embedders.
|
| |/ |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
I moved away from the `Window` struct all the logic to handle task
sources, into a new struct called `TaskManager`. In a happy world, I'd
be able to just have there two functions, of the types:
```rust
fn task_source<T: TaskSource>(&self, name: TaskSourceName) -> Box<T>
fn task_source_with_canceller<T: TaskSource>(&self, name: TaskSourceName)
-> (Box<T>, TaskSourceCanceller)
```
And not so much duplicated code. However, because TaskSource can't be a
trait object (because it has generic type parameters), that's not
possible. Instead, I decided to reduce duplicated logic through macros.
For reasons[1], I have to pass both the name of the function with
canceller and the name of the function without, as I'm not able to
concatenate them in the macro itself. I could probably use
`concat_idents` to create both types already defined and reduce the
amount of arguments by one, but that macro is nightly only. At the same
time, not being able to declare macros inside `impl` forces me to pass
`self` as an argument.
All this makes this solution more verbose than it would be ideally. It
does reduce duplication, but it doesn't reduce the size of the file.
[1](https://github.com/rust-lang/rust/issues/29599)
|
| |
|
|
| |
A `crate_name::foo` path always works in 2018
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| |
|
|
| |
This reverts commit b2c1f89b932a72f9e0110c17adde33647e84c902.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The TextInput::assert_ok_selection() method is meant to ensure that we
are not getting into a state where a selection refers to a location in
the control's contents which doesn't exist.
However, before this change we could have a situation where the
internals of the TextInput are changed by another part of the code,
without using its public API. This could lead to us having an invalid
selection.
I did manage to trigger such a situation (see the test added in this
commit) although it is quite contrived. There may be others that I
didn't think of, and it's also possible that future changes could
introduce new cases. (Including ones which trigger panics, if indexing
is used on the assumption that the selection indices are always valid.)
The current HTML specification doesn't explicitly say that
selectionStart/End must remain within the length of the content, but
that does seems to be the consensus reached in a discussion of this:
https://github.com/whatwg/html/issues/2424
The test case I've added here is currently undefined in the spec which
is why I've added it in tests/wpt/mozilla.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The intention here is to make the flow more explicit. I.e. rather than
calling `self.dom_select()` and relying on the programmer to
know/realise that this method is provided by a trait, we call
`self.selection().dom_select()` and the programmer can inspect the
definition of `self.selection()` to follow the code.
This came out of a discussion with KiChjang here:
https://github.com/servo/servo/pull/19544#discussion_r156167044
Note that I tried to make "selection" be a member field of
HTML{Input,TextArea}Element but it opened up a whole can of worms with
lifetimes, so it seemed simpler to not do that since it is not
essential for this code to work.
|
| |
|
|
|
|
|
|
|
|
| |
Spec: https://html.spec.whatwg.org/multipage/input.html#input-type-change
In short, this resets the selection to the start of the field when the
type has changed from one which doesn't support the selection API to one
that does.
I couldn't see an existing WPT test covering this.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Spec: https://html.spec.whatwg.org/multipage/#dom-textarea/input-setrangetext
In order to do this, we need to define the SelectionMode enum in WebIDL:
https://html.spec.whatwg.org/multipage/#selectionmode
Since the enum is used by HTMLTextAreaElement and HTMLInputElement, it
doesn't seem to make sense to define it in the WebIDL file for one or
other of those.
However, we also can't create a stand-alone SelectionMode.webidl file,
because the current binding-generation code won't generate a "pub mod
SelectionMode;" line in mod.rs unless SelectionMode.webidl contains
either an interface or a namespace. (This logic happens in
components/script/dom/bindings/codegen/Configuration.py:35, in the
Configuration.__init__ method.)
I thought about changing the binding-generation code, but that seems
difficult. So I settled for placing the enum inside
HTMLFormElement.webidl, as that seems like a "neutral" location. We
could equally settle for putting it under HTMLTextAreaElement or
HTMLInputElement, it probably doesn't really matter.
The setRangeText algorithm set the "dirty value flag" on the
input/textarea. I made some clean-ups related to this:
1. HTMLTextAreaElement called its dirty value flag "value_changed"; I
changed this to "value_dirty" to be consistent with the spec.
2. HTMLInputElement had a "value_changed" field and also a "value_dirty"
field, which were each used in slightly different places (and
sometimes in both places). I consolidated these into a single
"value_dirty" field, which was necessary in order to make some of the
tests pass.
TextControl::set_dom_range_text replaces part of the existing textinput
content with the replacement string (steps 9-10 of the algorithm). My
implementation changes the textinput's selection and then replaces the
selection. A downside of this approach is that we lose the original
selection state from before the call to setRangeText. Therefore, we have
to save the state into the original_selection_state variable so that we
can later pass it into TextControl::set_selection_range. This allows
TextControl::set_selection_range to correctly decide whether or not to
fire the select event.
An alternative approach would be to implement a method on TextInput
which allows a subtring of the content to be mutated, without touching
the current selection state. However, any such method would potentially
put the TextInput into an inconsistent state where the edit_point and/or
selection_origin is a TextPoint which doesn't exist in the content. It
would be up to the caller to subsequently make sure that the TextInput
gets put back into a valid state (which would actually happen, when
TextControl::set_selection_range is called).
I think TextInput's public API should not make it possible to put it
into an invalid state, as that would be a potential source of bugs.
That's why I didn't take this approach. (TextInput's public API does
currently make it possible to create an invalid state, but I'd like to
submit a follow-up patch to lock this down.)
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
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).
|
| |
|
|
| |
Issue #19171
|
| | |
|
| | |
|
| | |
|
| | |
|
| |\
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Handle cases where selection API doesn't apply
The selection API only applies to certain <input> types:
https://html.spec.whatwg.org/multipage/#do-not-apply
This commit ensures that we handle that correctly.
Some notes:
1. TextControl::set_dom_selection_direction now calls
set_selection_range(), which means that setting selectionDirection will
now fire a selection event, as it should per the spec.
2. There is a test for the firing of the select event in
tests/wpt/web-platform-tests/html/semantics/forms/textfieldselection/select-event.html,
however the test did not run due to this syntax error:
(pid:26017) "ERROR:script::dom::bindings::error: Error at http://web-platform.test:8000/html/semantics/forms/textfieldselection/select-event.html:50:11 missing = in const declaration"
This happens due to the us of the "for (const foo of ...)" construct.
Per https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...of
this should actually work, so it's somewhat unsatisfying to have to
change the test.
3. I removed tests/wpt/web-platform-tests/html/semantics/forms/textfieldselection/selection-not-application-textarea.html
because it doesn't seem to add any extra value - the selection API
always applies to textarea elements, and the API is tested elsewhere.
4. If an `<input>`'s type is unset, it defaults to a text, and the
selection API applies. Also, if an `<input>`'s type is set to an
invalid value, it defaults to a text too. This second case doesn't
currently work, and I'll need to do more restructuring of the code in
a future commit. See discussion with nox in IRC:
https://mozilla.logbot.info/servo/20171201#c13946454-c13946594
<!-- 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/19461)
<!-- Reviewable:end -->
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
The selection API only applies to certain <input> types:
https://html.spec.whatwg.org/multipage/#do-not-apply
This commit ensures that we handle that correctly.
Some notes:
1. TextControl::set_dom_selection_direction now calls
set_selection_range(), which means that setting selectionDirection will
now fire a selection event, as it should per the spec.
2. There is a test for the firing of the select event in
tests/wpt/web-platform-tests/html/semantics/forms/textfieldselection/select-event.html,
however the test did not run due to this syntax error:
(pid:26017) "ERROR:script::dom::bindings::error: Error at http://web-platform.test:8000/html/semantics/forms/textfieldselection/select-event.html:50:11 missing = in const declaration"
This happens due to the us of the "for (const foo of ...)" construct.
Per https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...of
this should actually work, so it's somewhat unsatisfying to have to
change the test.
4. If an <input>'s type is unset, it defaults to a text, and the
selection API applies. Also, if an <input>'s type is set to an
invalid value, it defaults to a text too. I've expanded the tests
to account for this second case.
|
| |/
|
|
|
|
| |
It has been removed from the spec: https://github.com/whatwg/html/issues/336
See also https://github.com/servo/servo/pull/19471#pullrequestreview-80711878
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This came out of a conversation with nox in IRC:
https://mozilla.logbot.info/servo/20171201#c13946454-c13946594
The code I was working on which motivated this change is here:
https://github.com/servo/servo/pull/19461
Previously, InputType::Text was used to represent several different
values of the type attribute on an input element.
If an input element doesn't have a type attribute, or its type attribute
doesn't contain a recognised value, then the input's type defaults to
"text".
Before this change, there were a number of checks in the code which
directly looked at the type attribute. If those checks matched against
the value "text", then they were potentially buggy, since an input with
type=invalid should also behave like an input with type=text.
Rather than have every conditional which cares about the input type also
have to deal with invalid input types, we can convert the type attribute
to an InputType enum once, and then match against the enum.
A secondary benefit is that the compiler can tell us whether we've
missed branches in a match expression. While working on this I
discovered that the HTMLInputElement::value_mode() method misses a case
for inputs with type=hidden (this resulted in a failing WPT test
passing).
I've also implemented the Default trait for InputType, so we now only
have one place in the code which knows that InputType::Text is the
default, where previously there were several.
|
| | |
|
| |
|
|
|
|
| |
- Implemented is_valid_time_string for DOMString.
- Use is_valid_time_string for sanitize_value with time input.
- Improved input type change test
|
| |
|
|
|
|
|
|
|
|
|
|
| |
I had to change the test a little bit to avoid some failures due to
color and text both having a sanitizedValue which was making the test
use the first assertion instead of the second one in some cases.
The sanitize_value implementation is pretty simple, we iterate over the
content and checks that the content is 7 characters long, that the first
character is a `#` and then that all the following characters are
hexadecimal. If all those requirements are met, we lowercase the
content, otherwise we put `#000000` in it.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
The API for text control selection is the same for both <input> and
<textarea>:
https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#textFieldSelection
Before this change, they had similar but not identical implementations
with duplicate code. Now there is a common TextControl trait which
contains the implementation used by both. As a result, some previously
failing tests now pass.
|
| |
|
|
| |
Issue #19171
|
| | |
|
| | |
|
| |
|
|
|
|
| |
failures, e.g. in layout/reftests/bugs/392435-1.html. r=backout on a CLOSED TREE
Backs out https://github.com/servo/servo/pull/18809
|
| |
|
|
|
| |
It still needs dependencies update to remove all the other bitflags
versions.
|
| |\
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Use WebRender to compute text index on click events
This is the second half of switching over to WebRender for hit testing.
Now that WebRender gives us the location of the hit tested point in the
display item, we can use that to calculate text index.
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).
<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because they shouldn't change behavior.
<!-- 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/18933)
<!-- Reviewable:end -->
|
