-
Notifications
You must be signed in to change notification settings - Fork 6k
Fix web editable text composing range #33590
Changes from all commits
8a321b1
becafe7
e25165a
8eb9d79
301d4a6
aae7f61
cc7786a
d5376e6
8826b18
8e172d0
d698637
878ce0a
c884e8b
e5b796a
c8011d7
e104af6
a1e34ae
7334e0a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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, | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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'; | ||||||||||||||||
|
|
||||||||||||||||
|
|
@@ -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; | ||||||||||||||||
|
|
@@ -618,6 +618,8 @@ class TextEditingDeltaState { | |||||||||||||||
| 'deltaEnd': deltaEnd, | ||||||||||||||||
| 'selectionBase': baseOffset, | ||||||||||||||||
| 'selectionExtent': extentOffset, | ||||||||||||||||
| 'composingBase': composingOffset, | ||||||||||||||||
| 'composingExtent': composingExtent | ||||||||||||||||
| }, | ||||||||||||||||
| ], | ||||||||||||||||
| }; | ||||||||||||||||
|
|
@@ -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. | ||||||||||||||||
|
|
@@ -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'); | ||||||||||||||||
antholeole marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||
|
|
||||||||||||||||
| final int? composingBase = flutterEditingState.tryInt('composingBase'); | ||||||||||||||||
| final int? composingExtent = flutterEditingState.tryInt('composingExtent'); | ||||||||||||||||
|
|
||||||||||||||||
| return EditingState( | ||||||||||||||||
| text: text, | ||||||||||||||||
| baseOffset: selectionBase, | ||||||||||||||||
| extentOffset: selectionExtent, | ||||||||||||||||
| composingBaseOffset: composingBase, | ||||||||||||||||
| composingExtentOffset: composingExtent | ||||||||||||||||
| ); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
|
|
@@ -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. | ||||||||||||||||
|
|
@@ -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) { | ||||||||||||||||
|
|
@@ -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(); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
|
|
@@ -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); | ||||||||||||||||
|
|
@@ -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. | ||||||||||||||||
|
|
@@ -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. | ||||||||||||||||
|
|
@@ -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); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
|
|
@@ -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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: engine/lib/web_ui/lib/src/engine/text_editing/text_editing.dart Lines 506 to 512 in 593140b
I'd love to get @Renzo-Olivares's opinion on this one.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We were previously setting the
To fix this the Which would not cause a failure on The fix would look something like Renzo-Olivares@f459888 . I'm unsure if we want to rely on |
||||||||||||||||
|
|
||||||||||||||||
| void maybeSendAction(html.Event event) { | ||||||||||||||||
| if (event is html.KeyboardEvent && event.keyCode == _kReturnKeyCode) { | ||||||||||||||||
| onAction!(inputConfiguration.inputAction); | ||||||||||||||||
|
|
@@ -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((_) { | ||||||||||||||||
|
|
@@ -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) { | ||||||||||||||||
|
|
@@ -1650,7 +1692,7 @@ class FirefoxTextEditingStrategy extends GloballyPositionedTextEditingStrategy { | |||||||||||||||
|
|
||||||||||||||||
| activeDomElement.addEventListener('beforeinput', handleBeforeInput); | ||||||||||||||||
|
|
||||||||||||||||
| activeDomElement.addEventListener('compositionupdate', handleCompositionUpdate); | ||||||||||||||||
| addCompositionEventHandlers(activeDomElement); | ||||||||||||||||
|
|
||||||||||||||||
| // Detects changes in text selection. | ||||||||||||||||
| // | ||||||||||||||||
|
|
||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.