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

Glyph count used a grapheme cluster count in text input hit testing #23399

Open
SimonSapin opened this issue May 15, 2019 · 0 comments

Comments

Projects
None yet
2 participants
@SimonSapin
Copy link
Member

commented May 15, 2019

From #23272 (comment)

The script::textinput::TextInput::set_edit_point_index method is called for click events on <input> elements to set the cursor’s position. It takes a usize parameter that is interpreted as a number of grapheme clusters:

pub fn set_edit_point_index(&mut self, index: usize) {
let byte_size = self.lines[self.edit_point.line]
.graphemes(true)
.take(index)
.fold(0, |acc, x| acc + x.len());
self.edit_point.index = byte_size;
}

This number ultimately comes from here:

/// Returns the index in the range of the first glyph advancing over given advance
pub fn range_index_of_advance(&self, range: &Range<ByteIndex>, advance: Au) -> usize {
// TODO(Issue #199): alter advance direction for RTL
// TODO(Issue #98): using inter-char and inter-word spacing settings when measuring text
let mut remaining = advance;
self.natural_word_slices_in_range(range)
.map(|slice| {
let (slice_index, slice_advance) = slice.glyphs.range_index_of_advance(
&slice.range,
remaining,
self.extra_word_spacing,
);
remaining -= slice_advance;
slice_index
})
.sum()
}

// Scan the glyphs for a given range until we reach a given advance. Returns the index
// and advance of the glyph in the range at the given advance, if reached. Otherwise, returns the
// the number of glyphs and the advance for the given range.
#[inline]
pub fn range_index_of_advance(
&self,
range: &Range<ByteIndex>,
advance: Au,
extra_word_spacing: Au,
) -> (usize, Au) {
let mut index = 0;
let mut current_advance = Au(0);
for glyph in self.iter_glyphs_for_byte_range(range) {
if glyph.char_is_space() {
current_advance += glyph.advance() + extra_word_spacing
} else {
current_advance += glyph.advance()
}
if current_advance > advance {
break;
}
index += 1;
}
(index, current_advance)
}

… where is is computed as a number of glyphs.

Grapheme clusters only map to glyphs one to one in simple cases. This could cause the cursor to end up at an incorrect position, far from where click cursor was.

Instead, the gfx code should map positions in the glyph sequence back to UTF-8 positions in the original Unicode text, and set_edit_point_index interpret its parameter as a UTF-8 byte count instead of a grapheme cluster count.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.