mirror of
https://github.com/servo/servo.git
synced 2025-08-05 21:50:18 +01:00
Auto merge of #23272 - tdelacour:ISSUE-20455, r=SimonSapin
ISSUE-20455: introduce stronger types for textinput indexing <!-- Please describe your changes on the following line: --> Added two new types: - ByteOffset - UTF16CodeUnitOffset I've replaced any instance of `usize` that would be better represented by one or the other of these. I also updated any downstream code, including the unit tests for `textinput.rs`. Along the way, I tried to add or edit comments to better reflect my understanding of this file - happy to revisit if I have misrepresented anything. I did not end up finding any places where types were very obviously being mixed, as the issue description suggested I should do... LMK if I should re-audit the file for that (might need a bit of guidance)! --- <!-- 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 #20455 (GitHub issue number if applicable) <!-- Either: --> - [X] 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/23272) <!-- Reviewable:end -->
This commit is contained in:
commit
f24f517965
5 changed files with 558 additions and 324 deletions
|
@ -45,7 +45,7 @@ use crate::textinput::KeyReaction::{
|
|||
DispatchInput, Nothing, RedrawSelection, TriggerDefaultAction,
|
||||
};
|
||||
use crate::textinput::Lines::Single;
|
||||
use crate::textinput::{Direction, SelectionDirection, TextInput};
|
||||
use crate::textinput::{Direction, SelectionDirection, TextInput, UTF16CodeUnits, UTF8Bytes};
|
||||
use caseless::compatibility_caseless_match_str;
|
||||
use dom_struct::dom_struct;
|
||||
use embedder_traits::FilterPattern;
|
||||
|
@ -434,7 +434,7 @@ impl LayoutHTMLInputElementHelpers for LayoutDom<HTMLInputElement> {
|
|||
match (*self.unsafe_get()).input_type() {
|
||||
InputType::Password => {
|
||||
let text = get_raw_textinput_value(self);
|
||||
let sel = textinput.sorted_selection_offsets_range();
|
||||
let sel = UTF8Bytes::unwrap_range(textinput.sorted_selection_offsets_range());
|
||||
|
||||
// Translate indices from the raw value to indices in the replacement value.
|
||||
let char_start = text[..sel.start].chars().count();
|
||||
|
@ -443,9 +443,9 @@ impl LayoutHTMLInputElementHelpers for LayoutDom<HTMLInputElement> {
|
|||
let bytes_per_char = PASSWORD_REPLACEMENT_CHAR.len_utf8();
|
||||
Some(char_start * bytes_per_char..char_end * bytes_per_char)
|
||||
},
|
||||
input_type if input_type.is_textual() => {
|
||||
Some(textinput.sorted_selection_offsets_range())
|
||||
},
|
||||
input_type if input_type.is_textual() => Some(UTF8Bytes::unwrap_range(
|
||||
textinput.sorted_selection_offsets_range(),
|
||||
)),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
@ -1357,7 +1357,7 @@ impl VirtualMethods for HTMLInputElement {
|
|||
if value < 0 {
|
||||
textinput.set_max_length(None);
|
||||
} else {
|
||||
textinput.set_max_length(Some(value as usize))
|
||||
textinput.set_max_length(Some(UTF16CodeUnits(value as usize)))
|
||||
}
|
||||
},
|
||||
_ => panic!("Expected an AttrValue::Int"),
|
||||
|
@ -1369,7 +1369,7 @@ impl VirtualMethods for HTMLInputElement {
|
|||
if value < 0 {
|
||||
textinput.set_min_length(None);
|
||||
} else {
|
||||
textinput.set_min_length(Some(value as usize))
|
||||
textinput.set_min_length(Some(UTF16CodeUnits(value as usize)))
|
||||
}
|
||||
},
|
||||
_ => panic!("Expected an AttrValue::Int"),
|
||||
|
|
|
@ -31,7 +31,9 @@ use crate::dom::nodelist::NodeList;
|
|||
use crate::dom::textcontrol::{TextControlElement, TextControlSelection};
|
||||
use crate::dom::validation::Validatable;
|
||||
use crate::dom::virtualmethods::VirtualMethods;
|
||||
use crate::textinput::{Direction, KeyReaction, Lines, SelectionDirection, TextInput};
|
||||
use crate::textinput::{
|
||||
Direction, KeyReaction, Lines, SelectionDirection, TextInput, UTF16CodeUnits, UTF8Bytes,
|
||||
};
|
||||
use dom_struct::dom_struct;
|
||||
use html5ever::{LocalName, Prefix};
|
||||
use script_traits::ScriptToConstellationChan;
|
||||
|
@ -89,7 +91,9 @@ impl LayoutHTMLTextAreaElementHelpers for LayoutDom<HTMLTextAreaElement> {
|
|||
return None;
|
||||
}
|
||||
let textinput = (*self.unsafe_get()).textinput.borrow_for_layout();
|
||||
Some(textinput.sorted_selection_offsets_range())
|
||||
Some(UTF8Bytes::unwrap_range(
|
||||
textinput.sorted_selection_offsets_range(),
|
||||
))
|
||||
}
|
||||
|
||||
#[allow(unsafe_code)]
|
||||
|
@ -307,7 +311,8 @@ impl HTMLTextAreaElementMethods for HTMLTextAreaElement {
|
|||
|
||||
// https://html.spec.whatwg.org/multipage/#dom-textarea-textlength
|
||||
fn TextLength(&self) -> u32 {
|
||||
self.textinput.borrow().utf16_len() as u32
|
||||
let UTF16CodeUnits(num_units) = self.textinput.borrow().utf16_len();
|
||||
num_units as u32
|
||||
}
|
||||
|
||||
// https://html.spec.whatwg.org/multipage/#dom-lfe-labels
|
||||
|
@ -423,7 +428,7 @@ impl VirtualMethods for HTMLTextAreaElement {
|
|||
if value < 0 {
|
||||
textinput.set_max_length(None);
|
||||
} else {
|
||||
textinput.set_max_length(Some(value as usize))
|
||||
textinput.set_max_length(Some(UTF16CodeUnits(value as usize)))
|
||||
}
|
||||
},
|
||||
_ => panic!("Expected an AttrValue::Int"),
|
||||
|
@ -435,7 +440,7 @@ impl VirtualMethods for HTMLTextAreaElement {
|
|||
if value < 0 {
|
||||
textinput.set_min_length(None);
|
||||
} else {
|
||||
textinput.set_min_length(Some(value as usize))
|
||||
textinput.set_min_length(Some(UTF16CodeUnits(value as usize)))
|
||||
}
|
||||
},
|
||||
_ => panic!("Expected an AttrValue::Int"),
|
||||
|
|
|
@ -15,7 +15,7 @@ use crate::dom::bindings::str::DOMString;
|
|||
use crate::dom::event::{EventBubbles, EventCancelable};
|
||||
use crate::dom::eventtarget::EventTarget;
|
||||
use crate::dom::node::{window_from_node, Node, NodeDamage};
|
||||
use crate::textinput::{SelectionDirection, SelectionState, TextInput};
|
||||
use crate::textinput::{SelectionDirection, SelectionState, TextInput, UTF8Bytes};
|
||||
use script_traits::ScriptToConstellationChan;
|
||||
|
||||
pub trait TextControlElement: DerivedFrom<EventTarget> + DerivedFrom<Node> {
|
||||
|
@ -177,7 +177,8 @@ impl<'a, E: TextControlElement> TextControlSelection<'a, E> {
|
|||
// change the selection state in order to replace the text in the range.
|
||||
let original_selection_state = self.textinput.borrow().selection_state();
|
||||
|
||||
let content_length = self.textinput.borrow().len() as u32;
|
||||
let UTF8Bytes(content_length) = self.textinput.borrow().len_utf8();
|
||||
let content_length = content_length as u32;
|
||||
|
||||
// Step 5
|
||||
if start > content_length {
|
||||
|
@ -262,11 +263,13 @@ impl<'a, E: TextControlElement> TextControlSelection<'a, E> {
|
|||
}
|
||||
|
||||
fn start(&self) -> u32 {
|
||||
self.textinput.borrow().selection_start_offset() as u32
|
||||
let UTF8Bytes(offset) = self.textinput.borrow().selection_start_offset();
|
||||
offset as u32
|
||||
}
|
||||
|
||||
fn end(&self) -> u32 {
|
||||
self.textinput.borrow().selection_end_offset() as u32
|
||||
let UTF8Bytes(offset) = self.textinput.borrow().selection_end_offset();
|
||||
offset as u32
|
||||
}
|
||||
|
||||
fn direction(&self) -> SelectionDirection {
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue