Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -1132,6 +1132,7 @@ FILE: ../../../flutter/lib/web_ui/lib/src/engine/text/unicode_range.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/text/word_break_properties.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/text/word_breaker.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/text_editing/autofill_hint.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/text_editing/input_type.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/text_editing/text_capitalization.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/text_editing/text_editing.dart
Expand Down
1 change: 1 addition & 0 deletions lib/web_ui/lib/src/engine.dart
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ export 'engine/text/unicode_range.dart';
export 'engine/text/word_break_properties.dart';
export 'engine/text/word_breaker.dart';
export 'engine/text_editing/autofill_hint.dart';
export 'engine/text_editing/composition_aware_mixin.dart';
export 'engine/text_editing/input_type.dart';
export 'engine/text_editing/text_capitalization.dart';
export 'engine/text_editing/text_editing.dart';
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:html' as html;

import 'text_editing.dart';

/// Provides default functionality for listening to HTML composition events.
///
/// A class with this mixin generally calls [determineCompositionState] in order to update
/// an [EditingState] with new composition values; namely, [EditingState.composingBaseOffset]
/// and [EditingState.composingExtentOffset].
///
/// A class with this mixin should call [addCompositionEventHandlers] on initalization, and
/// [removeCompositionEventHandlers] on deinitalization.
///
/// See also:
///
/// * [EditingState], the state of a text field that [CompositionAwareMixin] updates.
/// * [DefaultTextEditingStrategy], the primary implementer of [CompositionAwareMixin].
mixin CompositionAwareMixin {
/// The name of the HTML composition event type that triggers on starting a composition.
static const String _kCompositionStart = 'compositionstart';

/// The name of the browser composition event type that triggers on updating a composition.
static const String _kCompositionUpdate = 'compositionupdate';

/// The name of the browser composition event type that triggers on ending a composition.
static const String _kCompositionEnd = 'compositionend';

late final html.EventListener _compositionStartListener = _handleCompositionStart;
late final html.EventListener _compositionUpdateListener = _handleCompositionUpdate;
late final html.EventListener _compositionEndListener = _handleCompositionEnd;

/// The currently composing text in the `domElement`.
///
/// Will be null if composing just started, ended, or no composing is being done.
/// This member is kept up to date provided compositionEventHandlers are in place,
/// so it is safe to reference it to get the current composingText.
String? composingText;

void addCompositionEventHandlers(html.HtmlElement domElement) {
domElement.addEventListener(_kCompositionStart, _compositionStartListener);
domElement.addEventListener(_kCompositionUpdate, _compositionUpdateListener);
domElement.addEventListener(_kCompositionEnd, _compositionEndListener);
}

void removeCompositionEventHandlers(html.HtmlElement domElement) {
domElement.removeEventListener(_kCompositionStart, _compositionStartListener);
domElement.removeEventListener(_kCompositionUpdate, _compositionUpdateListener);
domElement.removeEventListener(_kCompositionEnd, _compositionEndListener);
}

void _handleCompositionStart(html.Event event) {
composingText = null;
}

void _handleCompositionUpdate(html.Event event) {
if (event is html.CompositionEvent) {
composingText = event.data;
}
}

void _handleCompositionEnd(html.Event event) {
composingText = null;
}

EditingState determineCompositionState(EditingState editingState) {
if (editingState.baseOffset == null || composingText == null || editingState.text == null) {
return editingState;
}

final int composingBase = editingState.baseOffset! - composingText!.length;

if (composingBase < 0) {
return editingState;
}

return editingState.copyWith(
composingBaseOffset: composingBase,
composingExtentOffset: composingBase + composingText!.length,
);
}
}
78 changes: 60 additions & 18 deletions lib/web_ui/lib/src/engine/text_editing/text_editing.dart
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import '../services.dart';
import '../text/paragraph.dart';
import '../util.dart';
import 'autofill_hint.dart';
import 'composition_aware_mixin.dart';
import 'input_type.dart';
import 'text_capitalization.dart';

Expand Down Expand Up @@ -508,7 +509,6 @@ class TextEditingDeltaState {
final bool isCurrentlyComposing = newTextEditingDeltaState.composingOffset != null && newTextEditingDeltaState.composingOffset != newTextEditingDeltaState.composingExtent;
if (newTextEditingDeltaState.deltaText.isNotEmpty && previousSelectionWasCollapsed && isCurrentlyComposing) {
newTextEditingDeltaState.deltaStart = newTextEditingDeltaState.composingOffset!;
newTextEditingDeltaState.deltaEnd = newTextEditingDeltaState.composingExtent!;
}

final bool isDeltaRangeEmpty = newTextEditingDeltaState.deltaStart == -1 && newTextEditingDeltaState.deltaStart == newTextEditingDeltaState.deltaEnd;
Expand Down Expand Up @@ -618,6 +618,8 @@ class TextEditingDeltaState {
'deltaEnd': deltaEnd,
'selectionBase': baseOffset,
'selectionExtent': extentOffset,
'composingBase': composingOffset,
'composingExtent': composingExtent
},
],
};
Expand Down Expand Up @@ -647,7 +649,13 @@ class TextEditingDeltaState {

/// The current text and selection state of a text field.
class EditingState {
EditingState({this.text, int? baseOffset, int? extentOffset}) :
EditingState({
this.text,
int? baseOffset,
int? extentOffset,
this.composingBaseOffset,
this.composingExtentOffset
}) :
// Don't allow negative numbers. Pick the smallest selection index for base.
baseOffset = math.max(0, math.min(baseOffset ?? 0, extentOffset ?? 0)),
// Don't allow negative numbers. Pick the greatest selection index for extent.
Expand All @@ -674,14 +682,20 @@ class EditingState {
/// valid selection range for input DOM elements.
factory EditingState.fromFrameworkMessage(
Map<String, dynamic> flutterEditingState) {
final String? text = flutterEditingState.tryString('text');

final int selectionBase = flutterEditingState.readInt('selectionBase');
final int selectionExtent = flutterEditingState.readInt('selectionExtent');
final String? text = flutterEditingState.tryString('text');

final int? composingBase = flutterEditingState.tryInt('composingBase');
final int? composingExtent = flutterEditingState.tryInt('composingExtent');

return EditingState(
text: text,
baseOffset: selectionBase,
extentOffset: selectionExtent,
composingBaseOffset: composingBase,
composingExtentOffset: composingExtent
);
}

Expand All @@ -708,13 +722,31 @@ class EditingState {
}
}

EditingState copyWith({
String? text,
int? baseOffset,
int? extentOffset,
int? composingBaseOffset,
int? composingExtentOffset,
}) {
return EditingState(
text: text ?? this.text,
baseOffset: baseOffset ?? this.baseOffset,
extentOffset: extentOffset ?? this.extentOffset,
composingBaseOffset: composingBaseOffset ?? this.composingBaseOffset,
composingExtentOffset: composingExtentOffset ?? this.composingExtentOffset,
);
}

/// The counterpart of [EditingState.fromFrameworkMessage]. It generates a Map that
/// can be sent to Flutter.
// TODO(mdebbar): Should we get `selectionAffinity` and other properties from flutter's editing state?
Map<String, dynamic> toFlutter() => <String, dynamic>{
'text': text,
'selectionBase': baseOffset,
'selectionExtent': extentOffset,
'composingBase': composingBaseOffset,
'composingExtent': composingExtentOffset,
};

/// The current text being edited.
Expand All @@ -726,11 +758,19 @@ class EditingState {
/// The offset at which the text selection terminates.
final int? extentOffset;

/// The offset at which [CompositionAwareMixin.composingText] begins, if any.
final int? composingBaseOffset;

/// The offset at which [CompositionAwareMixin.composingText] terminates, if any.
final int? composingExtentOffset;

/// Whether the current editing state is valid or not.
bool get isValid => baseOffset! >= 0 && extentOffset! >= 0;

@override
int get hashCode => Object.hash(text, baseOffset, extentOffset);
int get hashCode => Object.hash(
text, baseOffset, extentOffset, composingBaseOffset, composingExtentOffset
);

@override
bool operator ==(Object other) {
Expand All @@ -743,13 +783,15 @@ class EditingState {
return other is EditingState &&
other.text == text &&
other.baseOffset == baseOffset &&
other.extentOffset == extentOffset;
other.extentOffset == extentOffset &&
other.composingBaseOffset == composingBaseOffset &&
other.composingExtentOffset == composingExtentOffset;
}

@override
String toString() {
return assertionsEnabled
? 'EditingState("$text", base:$baseOffset, extent:$extentOffset)'
? 'EditingState("$text", base:$baseOffset, extent:$extentOffset, composingBase:$composingBaseOffset, composingExtent:$composingExtentOffset)'
: super.toString();
}

Expand Down Expand Up @@ -1038,7 +1080,7 @@ class SafariDesktopTextEditingStrategy extends DefaultTextEditingStrategy {
///
/// Unless a formfactor/browser requires specific implementation for a specific
/// strategy the methods in this class should be used.
abstract class DefaultTextEditingStrategy implements TextEditingStrategy {
abstract class DefaultTextEditingStrategy with CompositionAwareMixin implements TextEditingStrategy {
final HybridTextEditing owner;

DefaultTextEditingStrategy(this.owner);
Expand Down Expand Up @@ -1169,7 +1211,7 @@ abstract class DefaultTextEditingStrategy implements TextEditingStrategy {

activeDomElement.addEventListener('beforeinput', handleBeforeInput);

activeDomElement.addEventListener('compositionupdate', handleCompositionUpdate);
addCompositionEventHandlers(activeDomElement);

// Refocus on the activeDomElement after blur, so that user can keep editing the
// text field.
Expand Down Expand Up @@ -1210,6 +1252,8 @@ abstract class DefaultTextEditingStrategy implements TextEditingStrategy {
subscriptions[i].cancel();
}
subscriptions.clear();
removeCompositionEventHandlers(activeDomElement);

// If focused element is a part of a form, it needs to stay on the DOM
// until the autofill context of the form is finalized.
// More details on `TextInput.finishAutofillContext` call.
Expand Down Expand Up @@ -1246,9 +1290,13 @@ abstract class DefaultTextEditingStrategy implements TextEditingStrategy {
void handleChange(html.Event event) {
assert(isEnabled);

final EditingState newEditingState = EditingState.fromDomElement(activeDomElement);
EditingState newEditingState = EditingState.fromDomElement(activeDomElement);
newEditingState = determineCompositionState(newEditingState);

TextEditingDeltaState? newTextEditingDeltaState;
if (inputConfiguration.enableDeltaModel) {
editingDeltaState.composingOffset = newEditingState.composingBaseOffset;
editingDeltaState.composingExtent = newEditingState.composingExtentOffset;
newTextEditingDeltaState = TextEditingDeltaState.inferDeltaState(newEditingState, lastEditingState, editingDeltaState);
}

Expand Down Expand Up @@ -1295,12 +1343,6 @@ abstract class DefaultTextEditingStrategy implements TextEditingStrategy {
}
}

void handleCompositionUpdate(html.Event event) {
final EditingState newEditingState = EditingState.fromDomElement(activeDomElement);
editingDeltaState.composingOffset = newEditingState.baseOffset!;
editingDeltaState.composingExtent = newEditingState.extentOffset!;
}
Comment on lines -1298 to -1302
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I honestly don't know how delta state works with composing range. Your change here seems to break delta state because it won't be able to take composing range into account anymore:

// If we are composing then set the delta range to the composing region we
// captured in compositionupdate.
final bool isCurrentlyComposing = newTextEditingDeltaState.composingOffset != null && newTextEditingDeltaState.composingOffset != newTextEditingDeltaState.composingExtent;
if (newTextEditingDeltaState.deltaText.isNotEmpty && previousSelectionWasCollapsed && isCurrentlyComposing) {
newTextEditingDeltaState.deltaStart = newTextEditingDeltaState.composingOffset!;
newTextEditingDeltaState.deltaEnd = newTextEditingDeltaState.composingExtent!;
}

I'd love to get @Renzo-Olivares's opinion on this one.

Copy link
Contributor

@Renzo-Olivares Renzo-Olivares May 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to be causing some issues, but i'm still looking into it. Ideally we could just pull the composingOffset and composingExtent from the EditingState after determineCompositionState() is called, but I'm getting some weird behavior doing that. I'll keep investigating, and post an update asap. I'll also update the documentation since this should be more clearly explained.

Copy link
Contributor

@Renzo-Olivares Renzo-Olivares May 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were previously setting the deltaStart and deltaEnd to the composing region captured in compositionupdate. I previously derived the composing region by grabbing the editing state from the DOM at the time when the compositionupdate event was fired and using the selection as the composing region. At the time of firing the compositionupdate event, the editing states selection contains the range that was replaced during composition. Previously this worked fine because we were replacing the replaced range with the composed text. However, this was a bit inaccurate since it is not the composing region after the edit was made. When making the composing region accurate through this change, a failure occurs in the _replace method because the range being replaced did not exist yet. For example when composing:

Type: ’n’
oldText = ‘’
deltaText = ’n’
deltaStart = ‘0’ (from composingOffset)
deltaEnd = ‘1’ (from composingExtent)

_replace tries to replace (0,1) with ’n’ in oldText which is an empty string. This fails and a delta is not produced. The reason for this being, by deriving the composing region from compositionupdate using CompositionAwareMixin we are actually deriving the composing region after that composing step has finished, i.e. after ’n’ has already been added to the EditingState the composing region is (0,1).

To fix this the deltaStart and deltaEnd should be the range replaced by the composing region, and not the composing region after the compositionupdate event has updated the editing state. We can do this by setting the deltaStart to the composingOffset captured by the CompositionAwareMixin and leaving the deltaEnd as the one captured by beforeInput. This will be the position of the caret before the compositionupdate event is fired. In the case of our example above it would be (0). So coming back to our previous example we would have:

Type: ’n’
oldText = ‘’
deltaText = ’n’
deltaStart = ‘0’ (from composingOffset)
deltaEnd = ‘0’ (from beforeInput extentOffset)

Which would not cause a failure on _replace and generate the correct delta.

The fix would look something like Renzo-Olivares@f459888 .

I'm unsure if we want to rely on beforeInput or if we want to rely on 'CompositionAwareMixin’ to save the selection range from the active DOM editing state in compositionupdate and set the composing region and deltaStart and deltaEnd through something like determineCompositionStateForDelta().


void maybeSendAction(html.Event event) {
if (event is html.KeyboardEvent && event.keyCode == _kReturnKeyCode) {
onAction!(inputConfiguration.inputAction);
Expand Down Expand Up @@ -1450,7 +1492,7 @@ class IOSTextEditingStrategy extends GloballyPositionedTextEditingStrategy {

activeDomElement.addEventListener('beforeinput', handleBeforeInput);

activeDomElement.addEventListener('compositionupdate', handleCompositionUpdate);
addCompositionEventHandlers(activeDomElement);

// Position the DOM element after it is focused.
subscriptions.add(activeDomElement.onFocus.listen((_) {
Expand Down Expand Up @@ -1594,7 +1636,7 @@ class AndroidTextEditingStrategy extends GloballyPositionedTextEditingStrategy {

activeDomElement.addEventListener('beforeinput', handleBeforeInput);

activeDomElement.addEventListener('compositionupdate', handleCompositionUpdate);
addCompositionEventHandlers(activeDomElement);

subscriptions.add(activeDomElement.onBlur.listen((_) {
if (windowHasFocus) {
Expand Down Expand Up @@ -1650,7 +1692,7 @@ class FirefoxTextEditingStrategy extends GloballyPositionedTextEditingStrategy {

activeDomElement.addEventListener('beforeinput', handleBeforeInput);

activeDomElement.addEventListener('compositionupdate', handleCompositionUpdate);
addCompositionEventHandlers(activeDomElement);

// Detects changes in text selection.
//
Expand Down
Loading