From 8a321b15eab3024e3e017cdd7b29fd3532c9af32 Mon Sep 17 00:00:00 2001 From: Anthony Oleinik Date: Sun, 22 May 2022 20:12:23 -0700 Subject: [PATCH 01/16] implementation for composition events in TextEditingStrategy --- .../src/engine/text_editing/text_editing.dart | 104 ++++++++++++++++-- 1 file changed, 94 insertions(+), 10 deletions(-) diff --git a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart index 0b69a49b0444a..96e08050527c8 100644 --- a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart +++ b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart @@ -647,7 +647,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 +680,20 @@ class EditingState { /// valid selection range for input DOM elements. factory EditingState.fromFrameworkMessage( Map 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 ); } @@ -708,6 +720,22 @@ 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? @@ -715,6 +743,8 @@ class EditingState { 'text': text, 'selectionBase': baseOffset, 'selectionExtent': extentOffset, + 'composingBase': composingBaseOffset ?? -1, + 'composingExtent': composingExtentOffset ?? -1, }; /// The current text being edited. @@ -726,11 +756,18 @@ class EditingState { /// The offset at which the text selection terminates. final int? extentOffset; + /// The offset at which text is still being composed. + final int? composingBaseOffset; + + /// The offset at which composing text terminates. + final int? composingExtentOffset; + /// Whether the current editing state is valid or not. bool get isValid => baseOffset! >= 0 && extentOffset! >= 0; @override - int get hashCode => ui.hashValues(text, baseOffset, extentOffset); + int get hashCode => ui.hashValues( + text, baseOffset, extentOffset, composingBaseOffset, composingExtentOffset); @override bool operator ==(Object other) { @@ -743,7 +780,9 @@ 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 @@ -1048,6 +1087,10 @@ abstract class DefaultTextEditingStrategy implements TextEditingStrategy { /// The DOM element used for editing, if any. html.HtmlElement? domElement; + html.EventListener? compositionStart; + html.EventListener? compositionUpdate; + html.EventListener? compositionEnd; + /// Same as [domElement] but null-checked. /// /// This must only be called in places that know for sure that a DOM element @@ -1087,6 +1130,8 @@ abstract class DefaultTextEditingStrategy implements TextEditingStrategy { bool get appendedToForm => _appendedToForm; bool _appendedToForm = false; + String? composingText; + html.FormElement? get focusedFormElement => inputConfiguration.autofillGroup?.formElement; @@ -1153,6 +1198,12 @@ abstract class DefaultTextEditingStrategy implements TextEditingStrategy { placeElement(); } + void addCompositionEventHandlers() { + activeDomElement.addEventListener('compositionstart', handleCompositionStart); + activeDomElement.addEventListener('compositionupdate', handleCompositionUpdate); + activeDomElement.addEventListener('compositionend', handleCompositionEnd); + } + @override void addEventHandlers() { if (inputConfiguration.autofillGroup != null) { @@ -1169,7 +1220,7 @@ abstract class DefaultTextEditingStrategy implements TextEditingStrategy { activeDomElement.addEventListener('beforeinput', handleBeforeInput); - activeDomElement.addEventListener('compositionupdate', handleCompositionUpdate); + addCompositionEventHandlers(); // Refocus on the activeDomElement after blur, so that user can keep editing the // text field. @@ -1196,6 +1247,12 @@ abstract class DefaultTextEditingStrategy implements TextEditingStrategy { } } + void removeCompositionEventHandlers() { + activeDomElement.removeEventListener('compositionstart', handleCompositionStart); + activeDomElement.removeEventListener('compositionupdate', handleCompositionUpdate); + activeDomElement.removeEventListener('compositionend', handleCompositionEnd); + } + @override void disable() { assert(isEnabled); @@ -1210,6 +1267,8 @@ abstract class DefaultTextEditingStrategy implements TextEditingStrategy { subscriptions[i].cancel(); } subscriptions.clear(); + removeCompositionEventHandlers(); + // 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,7 +1305,23 @@ abstract class DefaultTextEditingStrategy implements TextEditingStrategy { void handleChange(html.Event event) { assert(isEnabled); - final EditingState newEditingState = EditingState.fromDomElement(activeDomElement); + EditingState newEditingState = EditingState.fromDomElement(activeDomElement); + + // REVIEW text editing delta needs to have composing updated + if (composingText != null && newEditingState.text != null) { + //REVIEW if composition extent is null, we will certainly fail + //the following check: perhaps we short circuit instead of null check? + final int compositionExtent = newEditingState.baseOffset ?? -1; + final int composingBase = compositionExtent - composingText!.length; + + if (composingBase >= 0) { + newEditingState = newEditingState.copyWith( + composingBaseOffset: composingBase, + composingExtentOffset: composingBase + composingText!.length, + ); + } + } + TextEditingDeltaState? newTextEditingDeltaState; if (inputConfiguration.enableDeltaModel) { newTextEditingDeltaState = TextEditingDeltaState.inferDeltaState(newEditingState, lastEditingState, editingDeltaState); @@ -1295,12 +1370,21 @@ abstract class DefaultTextEditingStrategy implements TextEditingStrategy { } } - void handleCompositionUpdate(html.Event event) { - final EditingState newEditingState = EditingState.fromDomElement(activeDomElement); - editingDeltaState.composingOffset = newEditingState.baseOffset!; - editingDeltaState.composingExtent = newEditingState.extentOffset!; + 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) { + handleChange(event); + composingText = null; + } + void maybeSendAction(html.Event event) { if (event is html.KeyboardEvent) { if (inputConfiguration.inputType.submitActionOnEnter && From becafe728346ee807092156f730143fa9486fb6b Mon Sep 17 00:00:00 2001 From: Anthony Oleinik Date: Mon, 23 May 2022 08:53:37 -0700 Subject: [PATCH 02/16] extracted functionality for composing range into mixin --- lib/web_ui/lib/src/engine.dart | 1 + .../text_editing/composition_aware_mixin.dart | 59 +++++++++++++++++ .../src/engine/text_editing/text_editing.dart | 66 +++---------------- 3 files changed, 70 insertions(+), 56 deletions(-) create mode 100644 lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart diff --git a/lib/web_ui/lib/src/engine.dart b/lib/web_ui/lib/src/engine.dart index b6c325997af08..2983f3f132549 100644 --- a/lib/web_ui/lib/src/engine.dart +++ b/lib/web_ui/lib/src/engine.dart @@ -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'; diff --git a/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart b/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart new file mode 100644 index 0000000000000..a6b63609aa624 --- /dev/null +++ b/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart @@ -0,0 +1,59 @@ +import 'dart:html' as html; + +import 'text_editing.dart'; + +mixin CompositionAwareMixin { + String? composingText; + + void addCompositionEventHandlers(html.HtmlElement domElement) { + domElement.addEventListener('compositionstart', _handleCompositionStart); + domElement.addEventListener('compositionupdate', _handleCompositionUpdate); + domElement.addEventListener('compositionend', _handleCompositionEnd); + } + + void removeCompositionEventHandlers(html.HtmlElement domElement) { + domElement.removeEventListener('compositionstart', _handleCompositionStart); + domElement.removeEventListener('compositionupdate', _handleCompositionUpdate); + domElement.removeEventListener('compositionend', _handleCompositionEnd); + } + + 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) { + return editingState; + } + + if (composingText == null) { + return editingState; + } + + if (editingState.text == null) { + return editingState; + } + + final int compositionExtent = editingState.baseOffset!; + final int composingBase = compositionExtent - composingText!.length; + + if (composingBase < 0) { + return editingState; + } + + return editingState.copyWith( + composingBaseOffset: composingBase, + composingExtentOffset: composingBase + composingText!.length, + ); + } +} diff --git a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart index 96e08050527c8..95e8974415756 100644 --- a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart +++ b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart @@ -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'; @@ -648,8 +649,8 @@ class TextEditingDeltaState { /// The current text and selection state of a text field. class EditingState { EditingState({ - this.text, - int? baseOffset, + this.text, + int? baseOffset, int? extentOffset, this.composingBaseOffset, this.composingExtentOffset @@ -1077,7 +1078,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); @@ -1087,10 +1088,6 @@ abstract class DefaultTextEditingStrategy implements TextEditingStrategy { /// The DOM element used for editing, if any. html.HtmlElement? domElement; - html.EventListener? compositionStart; - html.EventListener? compositionUpdate; - html.EventListener? compositionEnd; - /// Same as [domElement] but null-checked. /// /// This must only be called in places that know for sure that a DOM element @@ -1130,8 +1127,6 @@ abstract class DefaultTextEditingStrategy implements TextEditingStrategy { bool get appendedToForm => _appendedToForm; bool _appendedToForm = false; - String? composingText; - html.FormElement? get focusedFormElement => inputConfiguration.autofillGroup?.formElement; @@ -1198,12 +1193,6 @@ abstract class DefaultTextEditingStrategy implements TextEditingStrategy { placeElement(); } - void addCompositionEventHandlers() { - activeDomElement.addEventListener('compositionstart', handleCompositionStart); - activeDomElement.addEventListener('compositionupdate', handleCompositionUpdate); - activeDomElement.addEventListener('compositionend', handleCompositionEnd); - } - @override void addEventHandlers() { if (inputConfiguration.autofillGroup != null) { @@ -1220,7 +1209,7 @@ abstract class DefaultTextEditingStrategy implements TextEditingStrategy { activeDomElement.addEventListener('beforeinput', handleBeforeInput); - addCompositionEventHandlers(); + addCompositionEventHandlers(activeDomElement); // Refocus on the activeDomElement after blur, so that user can keep editing the // text field. @@ -1247,12 +1236,6 @@ abstract class DefaultTextEditingStrategy implements TextEditingStrategy { } } - void removeCompositionEventHandlers() { - activeDomElement.removeEventListener('compositionstart', handleCompositionStart); - activeDomElement.removeEventListener('compositionupdate', handleCompositionUpdate); - activeDomElement.removeEventListener('compositionend', handleCompositionEnd); - } - @override void disable() { assert(isEnabled); @@ -1267,7 +1250,7 @@ abstract class DefaultTextEditingStrategy implements TextEditingStrategy { subscriptions[i].cancel(); } subscriptions.clear(); - removeCompositionEventHandlers(); + 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. @@ -1306,21 +1289,7 @@ abstract class DefaultTextEditingStrategy implements TextEditingStrategy { assert(isEnabled); EditingState newEditingState = EditingState.fromDomElement(activeDomElement); - - // REVIEW text editing delta needs to have composing updated - if (composingText != null && newEditingState.text != null) { - //REVIEW if composition extent is null, we will certainly fail - //the following check: perhaps we short circuit instead of null check? - final int compositionExtent = newEditingState.baseOffset ?? -1; - final int composingBase = compositionExtent - composingText!.length; - - if (composingBase >= 0) { - newEditingState = newEditingState.copyWith( - composingBaseOffset: composingBase, - composingExtentOffset: composingBase + composingText!.length, - ); - } - } + newEditingState = determineCompositionState(newEditingState); TextEditingDeltaState? newTextEditingDeltaState; if (inputConfiguration.enableDeltaModel) { @@ -1370,21 +1339,6 @@ abstract class DefaultTextEditingStrategy implements TextEditingStrategy { } } - 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) { - handleChange(event); - composingText = null; - } - void maybeSendAction(html.Event event) { if (event is html.KeyboardEvent) { if (inputConfiguration.inputType.submitActionOnEnter && @@ -1538,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((_) { @@ -1682,7 +1636,7 @@ class AndroidTextEditingStrategy extends GloballyPositionedTextEditingStrategy { activeDomElement.addEventListener('beforeinput', handleBeforeInput); - activeDomElement.addEventListener('compositionupdate', handleCompositionUpdate); + addCompositionEventHandlers(activeDomElement); subscriptions.add(activeDomElement.onBlur.listen((_) { if (windowHasFocus) { @@ -1738,7 +1692,7 @@ class FirefoxTextEditingStrategy extends GloballyPositionedTextEditingStrategy { activeDomElement.addEventListener('beforeinput', handleBeforeInput); - activeDomElement.addEventListener('compositionupdate', handleCompositionUpdate); + addCompositionEventHandlers(activeDomElement); // Detects changes in text selection. // From e25165a930498afd117ca8d84763b80cbd0a5fd0 Mon Sep 17 00:00:00 2001 From: Anthony Oleinik Date: Tue, 24 May 2022 00:07:31 +0000 Subject: [PATCH 03/16] completed unit tests for composing range --- .../text_editing/composition_aware_mixin.dart | 36 +++-- .../src/engine/text_editing/text_editing.dart | 10 +- .../test/composition_aware_mixin_test.dart | 143 ++++++++++++++++++ lib/web_ui/test/text_editing_test.dart | 65 ++++++-- 4 files changed, 225 insertions(+), 29 deletions(-) create mode 100644 lib/web_ui/test/composition_aware_mixin_test.dart diff --git a/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart b/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart index a6b63609aa624..168635c92c4db 100644 --- a/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart +++ b/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart @@ -2,19 +2,32 @@ import 'dart:html' as html; import 'text_editing.dart'; +typedef OnCompositionEndCallback = void Function(html.Event); mixin CompositionAwareMixin { + static const String kCompositionStart = 'compositionstart'; + static const String kCompositionUpdate = 'compositionupdate'; + static const String kCompositionEnd = 'compositionend'; + String? composingText; - void addCompositionEventHandlers(html.HtmlElement domElement) { - domElement.addEventListener('compositionstart', _handleCompositionStart); - domElement.addEventListener('compositionupdate', _handleCompositionUpdate); - domElement.addEventListener('compositionend', _handleCompositionEnd); + OnCompositionEndCallback? _onCompositionEndCallback; + + void addCompositionEventHandlers(html.HtmlElement domElement, + {required OnCompositionEndCallback onCompositionEndCallback}) { + domElement.addEventListener(kCompositionStart, _handleCompositionStart); + domElement.addEventListener(kCompositionUpdate, _handleCompositionUpdate); + domElement.addEventListener(kCompositionEnd, _handleCompositionEnd); + + _onCompositionEndCallback = onCompositionEndCallback; } void removeCompositionEventHandlers(html.HtmlElement domElement) { - domElement.removeEventListener('compositionstart', _handleCompositionStart); - domElement.removeEventListener('compositionupdate', _handleCompositionUpdate); - domElement.removeEventListener('compositionend', _handleCompositionEnd); + domElement.removeEventListener(kCompositionStart, _handleCompositionStart); + domElement.removeEventListener( + kCompositionUpdate, _handleCompositionUpdate); + domElement.removeEventListener(kCompositionEnd, _handleCompositionEnd); + + _onCompositionEndCallback = null; } void _handleCompositionStart(html.Event event) { @@ -28,11 +41,15 @@ mixin CompositionAwareMixin { } void _handleCompositionEnd(html.Event event) { + assert(_onCompositionEndCallback != null, + 'cannot call composition update without having registered composition handlers'); + + _onCompositionEndCallback!(event); composingText = null; } EditingState determineCompositionState(EditingState editingState) { - if (editingState.baseOffset == null) { + if (editingState.baseOffset == null || editingState.baseOffset == -1) { return editingState; } @@ -44,8 +61,7 @@ mixin CompositionAwareMixin { return editingState; } - final int compositionExtent = editingState.baseOffset!; - final int composingBase = compositionExtent - composingText!.length; + final int composingBase = editingState.baseOffset! - composingText!.length; if (composingBase < 0) { return editingState; diff --git a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart index 95e8974415756..1cf15e16bb10b 100644 --- a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart +++ b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart @@ -789,7 +789,7 @@ class EditingState { @override String toString() { return assertionsEnabled - ? 'EditingState("$text", base:$baseOffset, extent:$extentOffset)' + ? 'EditingState("$text", base:$baseOffset, extent:$extentOffset, composingBase:$composingBaseOffset, composingExtent:$composingExtentOffset)' : super.toString(); } @@ -1209,7 +1209,7 @@ abstract class DefaultTextEditingStrategy with CompositionAwareMixin implements activeDomElement.addEventListener('beforeinput', handleBeforeInput); - addCompositionEventHandlers(activeDomElement); + addCompositionEventHandlers(activeDomElement, onCompositionEndCallback: handleChange); // Refocus on the activeDomElement after blur, so that user can keep editing the // text field. @@ -1492,7 +1492,7 @@ class IOSTextEditingStrategy extends GloballyPositionedTextEditingStrategy { activeDomElement.addEventListener('beforeinput', handleBeforeInput); - addCompositionEventHandlers(activeDomElement); + addCompositionEventHandlers(activeDomElement, onCompositionEndCallback: handleChange); // Position the DOM element after it is focused. subscriptions.add(activeDomElement.onFocus.listen((_) { @@ -1636,7 +1636,7 @@ class AndroidTextEditingStrategy extends GloballyPositionedTextEditingStrategy { activeDomElement.addEventListener('beforeinput', handleBeforeInput); - addCompositionEventHandlers(activeDomElement); + addCompositionEventHandlers(activeDomElement, onCompositionEndCallback: handleChange); subscriptions.add(activeDomElement.onBlur.listen((_) { if (windowHasFocus) { @@ -1692,7 +1692,7 @@ class FirefoxTextEditingStrategy extends GloballyPositionedTextEditingStrategy { activeDomElement.addEventListener('beforeinput', handleBeforeInput); - addCompositionEventHandlers(activeDomElement); + addCompositionEventHandlers(activeDomElement, onCompositionEndCallback: handleChange); // Detects changes in text selection. // diff --git a/lib/web_ui/test/composition_aware_mixin_test.dart b/lib/web_ui/test/composition_aware_mixin_test.dart new file mode 100644 index 0000000000000..97a451bf5ee54 --- /dev/null +++ b/lib/web_ui/test/composition_aware_mixin_test.dart @@ -0,0 +1,143 @@ +// 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:async'; +import 'dart:html' as html; + +import 'package:test/bootstrap/browser.dart'; +import 'package:test/test.dart'; + +import 'package:ui/src/engine/initialization.dart'; +import 'package:ui/src/engine/text_editing/composition_aware_mixin.dart'; +import 'package:ui/src/engine/text_editing/input_type.dart'; +import 'package:ui/src/engine/text_editing/text_editing.dart'; + +import 'text_editing_test.dart'; + +void main() { + internalBootstrapBrowserTest(() => testMain); +} + +class MockWithCompositionAwareMixin with CompositionAwareMixin {} + +html.HtmlElement get inputElement { + return defaultTextEditingRoot.querySelectorAll('input').first + as html.HtmlElement; +} + +Future testMain() async { + await initializeEngine(); + + final GloballyPositionedTextEditingStrategy editingStrategy = + GloballyPositionedTextEditingStrategy(HybridTextEditing() + ..configuration = InputConfiguration( + inputType: EngineInputType.text, + )); + editingStrategy.owner.debugTextEditingStrategyOverride = editingStrategy; + const String fakeComposingText = 'ImComposingText'; + + setUp(() { + editingStrategy.enable( + singlelineConfig, + onChange: trackEditingState, + onAction: trackInputAction, + ); + }); + + tearDown(() { + editingStrategy.disable(); + }); + + group('$CompositionAwareMixin', () { + group('composition end', () { + test('should reset composing text on handle composition end', () { + final MockWithCompositionAwareMixin mockWithCompositionAwareMixin = + MockWithCompositionAwareMixin(); + mockWithCompositionAwareMixin.composingText = fakeComposingText; + mockWithCompositionAwareMixin.addCompositionEventHandlers(inputElement, + onCompositionEndCallback: (_) {}); + + inputElement + .dispatchEvent(html.Event(CompositionAwareMixin.kCompositionEnd)); + + expect(mockWithCompositionAwareMixin.composingText, null); + }); + + test('should call onCompositionEndCallback', () { + const String fakeEventText = 'IAmComposingThis'; + final MockWithCompositionAwareMixin mockWithCompositionAwareMixin = + MockWithCompositionAwareMixin(); + mockWithCompositionAwareMixin.composingText = fakeComposingText; + final Completer didCallOnCompositionEndCallback = + Completer(); + mockWithCompositionAwareMixin.addCompositionEventHandlers(inputElement, + onCompositionEndCallback: didCallOnCompositionEndCallback.complete); + + inputElement.dispatchEvent(html.CompositionEvent( + CompositionAwareMixin.kCompositionEnd, + data: fakeEventText)); + + expect(didCallOnCompositionEndCallback.isCompleted, isTrue); + }); + }); + + group('composition start', () { + test('should reset composing text on handle composition start', () { + final MockWithCompositionAwareMixin mockWithCompositionAwareMixin = + MockWithCompositionAwareMixin(); + mockWithCompositionAwareMixin.composingText = fakeComposingText; + mockWithCompositionAwareMixin.addCompositionEventHandlers(inputElement, + onCompositionEndCallback: (_) {}); + + inputElement + .dispatchEvent(html.Event(CompositionAwareMixin.kCompositionStart)); + + expect(mockWithCompositionAwareMixin.composingText, null); + }); + }); + + group('composition update', () { + test('should set composing text to event composing text', () { + const String fakeEventText = 'IAmComposingThis'; + final MockWithCompositionAwareMixin mockWithCompositionAwareMixin = + MockWithCompositionAwareMixin(); + mockWithCompositionAwareMixin.composingText = fakeComposingText; + mockWithCompositionAwareMixin.addCompositionEventHandlers(inputElement, + onCompositionEndCallback: (_) {}); + + inputElement.dispatchEvent(html.CompositionEvent( + CompositionAwareMixin.kCompositionUpdate, + data: fakeEventText)); + + expect(mockWithCompositionAwareMixin.composingText, fakeEventText); + }); + }); + + group('determine composition state', () { + test('should return new composition state if valid new composition', () { + const int baseOffset = 100; + const String composingText = 'composeMe'; + + final EditingState editingState = EditingState( + extentOffset: baseOffset, + text: 'testing', + baseOffset: baseOffset, + ); + + final MockWithCompositionAwareMixin mockWithCompositionAwareMixin = + MockWithCompositionAwareMixin(); + mockWithCompositionAwareMixin.composingText = composingText; + + const int expectedComposingBase = baseOffset - composingText.length; + + expect( + mockWithCompositionAwareMixin + .determineCompositionState(editingState), + editingState.copyWith( + composingBaseOffset: expectedComposingBase, + composingExtentOffset: expectedComposingBase + composingText.length)); + }); + }); + }); +} diff --git a/lib/web_ui/test/text_editing_test.dart b/lib/web_ui/test/text_editing_test.dart index d3956344032d1..fc661e08f6898 100644 --- a/lib/web_ui/test/text_editing_test.dart +++ b/lib/web_ui/test/text_editing_test.dart @@ -15,6 +15,7 @@ import 'package:ui/src/engine/browser_detection.dart'; import 'package:ui/src/engine/initialization.dart'; import 'package:ui/src/engine/services.dart'; import 'package:ui/src/engine/text_editing/autofill_hint.dart'; +import 'package:ui/src/engine/text_editing/composition_aware_mixin.dart'; import 'package:ui/src/engine/text_editing/input_type.dart'; import 'package:ui/src/engine/text_editing/text_editing.dart'; import 'package:ui/src/engine/util.dart'; @@ -1551,7 +1552,9 @@ Future testMain() async { { 'text': 'something', 'selectionBase': 9, - 'selectionExtent': 9 + 'selectionExtent': 9, + 'composingBase': -1, + 'composingExtent': -1 } ], ); @@ -1575,12 +1578,17 @@ Future testMain() async { { 'text': 'something', 'selectionBase': 2, - 'selectionExtent': 5 + 'selectionExtent': 5, + 'composingBase': -1, + 'composingExtent': -1 } ], ); spy.messages.clear(); + input.dispatchEvent(CompositionEvent(CompositionAwareMixin.kCompositionEnd)); + + hideKeyboard(); }); @@ -1709,7 +1717,9 @@ Future testMain() async { hintForFirstElement: { 'text': 'something', 'selectionBase': 9, - 'selectionExtent': 9 + 'selectionExtent': 9, + 'composingBase': -1, + 'composingExtent': -1 } }, ], @@ -1748,6 +1758,8 @@ Future testMain() async { 'text': 'foo\nbar', 'selectionBase': 2, 'selectionExtent': 3, + 'composingBase': -1, + 'composingExtent': -1 }); sendFrameworkMessage(codec.encodeMethodCall(setEditingState)); checkTextAreaEditingState(textarea, 'foo\nbar', 2, 3); @@ -1777,6 +1789,8 @@ Future testMain() async { 'text': 'something\nelse', 'selectionBase': 14, 'selectionExtent': 14, + 'composingBase': -1, + 'composingExtent': -1 } ], ); @@ -1791,6 +1805,8 @@ Future testMain() async { 'text': 'something\nelse', 'selectionBase': 2, 'selectionExtent': 5, + 'composingBase': -1, + 'composingExtent': -1 } ], ); @@ -2269,21 +2285,42 @@ Future testMain() async { expect(_editingState.extentOffset, 2); }); - test('Compare two editing states', () { - final InputElement input = defaultTextEditingRoot.querySelector('input')! as InputElement; - input.value = 'Test'; - input.selectionStart = 1; - input.selectionEnd = 2; + group('comparing editing states', () { + test('From dom element', () { + final InputElement input = defaultTextEditingRoot.querySelector('input')! as InputElement; + input.value = 'Test'; + input.selectionStart = 1; + input.selectionEnd = 2; + + final EditingState editingState1 = EditingState.fromDomElement(input); + final EditingState editingState2 = EditingState.fromDomElement(input); - final EditingState editingState1 = EditingState.fromDomElement(input); - final EditingState editingState2 = EditingState.fromDomElement(input); + input.setSelectionRange(1, 3); - input.setSelectionRange(1, 3); + final EditingState editingState3 = EditingState.fromDomElement(input); + + expect(editingState1 == editingState2, isTrue); + expect(editingState1 != editingState3, isTrue); + }); + + test('takes composition range into account', () { + final EditingState editingState1 = EditingState(composingBaseOffset: 1, composingExtentOffset: 2); + final EditingState editingState2 = EditingState(composingBaseOffset: 1, composingExtentOffset: 2); + final EditingState editingState3 = EditingState(composingBaseOffset: 4, composingExtentOffset: 8); + + expect(editingState1, editingState2); + expect(editingState1, isNot(editingState3)); + }); + }); + + group('composing range', () { + test('is correct on new composing text', () { + //ANCHOR do me + }); - final EditingState editingState3 = EditingState.fromDomElement(input); + test('is -1, -1 on no composing text', () {}); - expect(editingState1 == editingState2, isTrue); - expect(editingState1 != editingState3, isTrue); + test('is correct on composing in the middle of text', () {}); }); }); From 8eb9d7980747c4e78e168a2979ae1742c5d1d1a4 Mon Sep 17 00:00:00 2001 From: Anthony Oleinik Date: Tue, 24 May 2022 09:56:59 -0700 Subject: [PATCH 04/16] polish composition changes --- .../text_editing/composition_aware_mixin.dart | 34 ++---- .../src/engine/text_editing/text_editing.dart | 8 +- ..._mixin_test.dart => composition_test.dart} | 104 +++++++++++------- lib/web_ui/test/text_editing_test.dart | 13 --- 4 files changed, 79 insertions(+), 80 deletions(-) rename lib/web_ui/test/{composition_aware_mixin_test.dart => composition_test.dart} (55%) diff --git a/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart b/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart index 168635c92c4db..7d0053caae1b2 100644 --- a/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart +++ b/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart @@ -2,32 +2,27 @@ import 'dart:html' as html; import 'text_editing.dart'; -typedef OnCompositionEndCallback = void Function(html.Event); mixin CompositionAwareMixin { static const String kCompositionStart = 'compositionstart'; static const String kCompositionUpdate = 'compositionupdate'; static const String kCompositionEnd = 'compositionend'; - String? composingText; - - OnCompositionEndCallback? _onCompositionEndCallback; + late final html.EventListener _compositionStartListener = _handleCompositionStart; + late final html.EventListener _compositionUpdateListener = _handleCompositionUpdate; + late final html.EventListener _compositionEndListener = _handleCompositionEnd; - void addCompositionEventHandlers(html.HtmlElement domElement, - {required OnCompositionEndCallback onCompositionEndCallback}) { - domElement.addEventListener(kCompositionStart, _handleCompositionStart); - domElement.addEventListener(kCompositionUpdate, _handleCompositionUpdate); - domElement.addEventListener(kCompositionEnd, _handleCompositionEnd); + String? composingText; - _onCompositionEndCallback = onCompositionEndCallback; + 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, _handleCompositionStart); - domElement.removeEventListener( - kCompositionUpdate, _handleCompositionUpdate); - domElement.removeEventListener(kCompositionEnd, _handleCompositionEnd); - - _onCompositionEndCallback = null; + domElement.removeEventListener(kCompositionStart, _compositionStartListener); + domElement.removeEventListener(kCompositionUpdate, _compositionUpdateListener); + domElement.removeEventListener(kCompositionEnd, _compositionEndListener); } void _handleCompositionStart(html.Event event) { @@ -41,15 +36,11 @@ mixin CompositionAwareMixin { } void _handleCompositionEnd(html.Event event) { - assert(_onCompositionEndCallback != null, - 'cannot call composition update without having registered composition handlers'); - - _onCompositionEndCallback!(event); composingText = null; } EditingState determineCompositionState(EditingState editingState) { - if (editingState.baseOffset == null || editingState.baseOffset == -1) { + if (editingState.baseOffset == null) { return editingState; } @@ -66,7 +57,6 @@ mixin CompositionAwareMixin { if (composingBase < 0) { return editingState; } - return editingState.copyWith( composingBaseOffset: composingBase, composingExtentOffset: composingBase + composingText!.length, diff --git a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart index 1cf15e16bb10b..7cac6a5c27eac 100644 --- a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart +++ b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart @@ -1209,7 +1209,7 @@ abstract class DefaultTextEditingStrategy with CompositionAwareMixin implements activeDomElement.addEventListener('beforeinput', handleBeforeInput); - addCompositionEventHandlers(activeDomElement, onCompositionEndCallback: handleChange); + addCompositionEventHandlers(activeDomElement); // Refocus on the activeDomElement after blur, so that user can keep editing the // text field. @@ -1492,7 +1492,7 @@ class IOSTextEditingStrategy extends GloballyPositionedTextEditingStrategy { activeDomElement.addEventListener('beforeinput', handleBeforeInput); - addCompositionEventHandlers(activeDomElement, onCompositionEndCallback: handleChange); + addCompositionEventHandlers(activeDomElement); // Position the DOM element after it is focused. subscriptions.add(activeDomElement.onFocus.listen((_) { @@ -1636,7 +1636,7 @@ class AndroidTextEditingStrategy extends GloballyPositionedTextEditingStrategy { activeDomElement.addEventListener('beforeinput', handleBeforeInput); - addCompositionEventHandlers(activeDomElement, onCompositionEndCallback: handleChange); + addCompositionEventHandlers(activeDomElement); subscriptions.add(activeDomElement.onBlur.listen((_) { if (windowHasFocus) { @@ -1692,7 +1692,7 @@ class FirefoxTextEditingStrategy extends GloballyPositionedTextEditingStrategy { activeDomElement.addEventListener('beforeinput', handleBeforeInput); - addCompositionEventHandlers(activeDomElement, onCompositionEndCallback: handleChange); + addCompositionEventHandlers(activeDomElement); // Detects changes in text selection. // diff --git a/lib/web_ui/test/composition_aware_mixin_test.dart b/lib/web_ui/test/composition_test.dart similarity index 55% rename from lib/web_ui/test/composition_aware_mixin_test.dart rename to lib/web_ui/test/composition_test.dart index 97a451bf5ee54..1d21c005c3c34 100644 --- a/lib/web_ui/test/composition_aware_mixin_test.dart +++ b/lib/web_ui/test/composition_test.dart @@ -21,9 +21,8 @@ void main() { class MockWithCompositionAwareMixin with CompositionAwareMixin {} -html.HtmlElement get inputElement { - return defaultTextEditingRoot.querySelectorAll('input').first - as html.HtmlElement; +html.InputElement get inputElement { + return defaultTextEditingRoot.querySelectorAll('input').first as html.InputElement; } Future testMain() async { @@ -40,8 +39,8 @@ Future testMain() async { setUp(() { editingStrategy.enable( singlelineConfig, - onChange: trackEditingState, - onAction: trackInputAction, + onChange: (_, __) {}, + onAction: (_) {}, ); }); @@ -55,31 +54,12 @@ Future testMain() async { final MockWithCompositionAwareMixin mockWithCompositionAwareMixin = MockWithCompositionAwareMixin(); mockWithCompositionAwareMixin.composingText = fakeComposingText; - mockWithCompositionAwareMixin.addCompositionEventHandlers(inputElement, - onCompositionEndCallback: (_) {}); + mockWithCompositionAwareMixin.addCompositionEventHandlers(inputElement); - inputElement - .dispatchEvent(html.Event(CompositionAwareMixin.kCompositionEnd)); + inputElement.dispatchEvent(html.Event(CompositionAwareMixin.kCompositionEnd)); expect(mockWithCompositionAwareMixin.composingText, null); }); - - test('should call onCompositionEndCallback', () { - const String fakeEventText = 'IAmComposingThis'; - final MockWithCompositionAwareMixin mockWithCompositionAwareMixin = - MockWithCompositionAwareMixin(); - mockWithCompositionAwareMixin.composingText = fakeComposingText; - final Completer didCallOnCompositionEndCallback = - Completer(); - mockWithCompositionAwareMixin.addCompositionEventHandlers(inputElement, - onCompositionEndCallback: didCallOnCompositionEndCallback.complete); - - inputElement.dispatchEvent(html.CompositionEvent( - CompositionAwareMixin.kCompositionEnd, - data: fakeEventText)); - - expect(didCallOnCompositionEndCallback.isCompleted, isTrue); - }); }); group('composition start', () { @@ -87,11 +67,9 @@ Future testMain() async { final MockWithCompositionAwareMixin mockWithCompositionAwareMixin = MockWithCompositionAwareMixin(); mockWithCompositionAwareMixin.composingText = fakeComposingText; - mockWithCompositionAwareMixin.addCompositionEventHandlers(inputElement, - onCompositionEndCallback: (_) {}); + mockWithCompositionAwareMixin.addCompositionEventHandlers(inputElement); - inputElement - .dispatchEvent(html.Event(CompositionAwareMixin.kCompositionStart)); + inputElement.dispatchEvent(html.Event(CompositionAwareMixin.kCompositionStart)); expect(mockWithCompositionAwareMixin.composingText, null); }); @@ -103,12 +81,10 @@ Future testMain() async { final MockWithCompositionAwareMixin mockWithCompositionAwareMixin = MockWithCompositionAwareMixin(); mockWithCompositionAwareMixin.composingText = fakeComposingText; - mockWithCompositionAwareMixin.addCompositionEventHandlers(inputElement, - onCompositionEndCallback: (_) {}); + mockWithCompositionAwareMixin.addCompositionEventHandlers(inputElement); - inputElement.dispatchEvent(html.CompositionEvent( - CompositionAwareMixin.kCompositionUpdate, - data: fakeEventText)); + inputElement.dispatchEvent( + html.CompositionEvent(CompositionAwareMixin.kCompositionUpdate, data: fakeEventText)); expect(mockWithCompositionAwareMixin.composingText, fakeEventText); }); @@ -120,10 +96,10 @@ Future testMain() async { const String composingText = 'composeMe'; final EditingState editingState = EditingState( - extentOffset: baseOffset, - text: 'testing', - baseOffset: baseOffset, - ); + extentOffset: baseOffset, + text: 'testing', + baseOffset: baseOffset, + ); final MockWithCompositionAwareMixin mockWithCompositionAwareMixin = MockWithCompositionAwareMixin(); @@ -132,12 +108,58 @@ Future testMain() async { const int expectedComposingBase = baseOffset - composingText.length; expect( - mockWithCompositionAwareMixin - .determineCompositionState(editingState), + mockWithCompositionAwareMixin.determineCompositionState(editingState), editingState.copyWith( composingBaseOffset: expectedComposingBase, composingExtentOffset: expectedComposingBase + composingText.length)); }); }); }); + + group('composing range', () { + test('should be [0, compostionStrLength] on new composition', () { + const String composingText = 'hi'; + + inputElement.dispatchEvent( + html.CompositionEvent(CompositionAwareMixin.kCompositionUpdate, data: composingText)); + + //set the selection text + inputElement.value = composingText; + inputElement.dispatchEvent(html.Event.eventType('Event', 'input')); + + expect( + editingStrategy.lastEditingState, + isA() + .having((EditingState editingState) => editingState.composingBaseOffset, + 'composingBaseOffset', 0) + .having((EditingState editingState) => editingState.composingExtentOffset, + 'composingExtentOffset', composingText.length)); + }); + + test( + 'should be [beforeComposingText - composingText, compostionStrLength] on composition in the middle of text', + () { + const String composingText = 'hi'; + const String beforeComposingText = 'beforeComposingText'; + const String afterComposingText = 'afterComposingText'; + + //type in the text box, then move cursor to the middle + inputElement.value = '$beforeComposingText$afterComposingText'; + inputElement.setSelectionRange(beforeComposingText.length, beforeComposingText.length); + + inputElement.dispatchEvent( + html.CompositionEvent(CompositionAwareMixin.kCompositionUpdate, data: composingText)); + + //flush editing state (since we did not compositionend) + inputElement.dispatchEvent(html.Event.eventType('Event', 'input')); + + expect( + editingStrategy.lastEditingState, + isA() + .having((EditingState editingState) => editingState.composingBaseOffset!, + 'composingBaseOffset', beforeComposingText.length - composingText.length) + .having((EditingState editingState) => editingState.composingExtentOffset, + 'composingExtentOffset', beforeComposingText.length)); + }); + }); } diff --git a/lib/web_ui/test/text_editing_test.dart b/lib/web_ui/test/text_editing_test.dart index fc661e08f6898..fcf0d05b5c3a5 100644 --- a/lib/web_ui/test/text_editing_test.dart +++ b/lib/web_ui/test/text_editing_test.dart @@ -1586,9 +1586,6 @@ Future testMain() async { ); spy.messages.clear(); - input.dispatchEvent(CompositionEvent(CompositionAwareMixin.kCompositionEnd)); - - hideKeyboard(); }); @@ -2312,16 +2309,6 @@ Future testMain() async { expect(editingState1, isNot(editingState3)); }); }); - - group('composing range', () { - test('is correct on new composing text', () { - //ANCHOR do me - }); - - test('is -1, -1 on no composing text', () {}); - - test('is correct on composing in the middle of text', () {}); - }); }); group('TextEditingDeltaState', () { From 301d4a6aac21691b1a60afd596b23eb2dbcf7552 Mon Sep 17 00:00:00 2001 From: Anthony Oleinik Date: Tue, 24 May 2022 10:35:30 -0700 Subject: [PATCH 05/16] added comments to composition_aware_mixin --- .../text_editing/composition_aware_mixin.dart | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart b/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart index 7d0053caae1b2..6c13c5638ca0b 100644 --- a/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart +++ b/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart @@ -2,15 +2,39 @@ 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 HTML composition event type that triggers on updating a composition. static const String kCompositionUpdate = 'compositionupdate'; + + /// the name of the HTML 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) { From aae7f6158653c24e8d0efe68c35274c3303c181f Mon Sep 17 00:00:00 2001 From: Anthony Oleinik Date: Tue, 24 May 2022 11:40:45 -0700 Subject: [PATCH 06/16] added license in composition_aware_mixin.dart file --- .../lib/src/engine/text_editing/composition_aware_mixin.dart | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart b/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart index 6c13c5638ca0b..84f042cbe6c34 100644 --- a/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart +++ b/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart @@ -1,3 +1,7 @@ +// 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'; From cc7786a7f22dfa8b8e62b64337a37b5ddf4c7963 Mon Sep 17 00:00:00 2001 From: Anthony Oleinik Date: Tue, 24 May 2022 11:40:45 -0700 Subject: [PATCH 07/16] added license in licenses_flutter --- ci/licenses_golden/licenses_flutter | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index b32f820ed1985..a7eadeb3e5fa7 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -1136,6 +1136,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 From d5376e6dbc2a15c33263ae72d1bb668f263a34f2 Mon Sep 17 00:00:00 2001 From: Anthony Oleinik Date: Tue, 24 May 2022 13:09:43 -0700 Subject: [PATCH 08/16] fixed unused import --- lib/web_ui/test/text_editing_test.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/web_ui/test/text_editing_test.dart b/lib/web_ui/test/text_editing_test.dart index fcf0d05b5c3a5..dc137a6f5a4bc 100644 --- a/lib/web_ui/test/text_editing_test.dart +++ b/lib/web_ui/test/text_editing_test.dart @@ -15,7 +15,6 @@ import 'package:ui/src/engine/browser_detection.dart'; import 'package:ui/src/engine/initialization.dart'; import 'package:ui/src/engine/services.dart'; import 'package:ui/src/engine/text_editing/autofill_hint.dart'; -import 'package:ui/src/engine/text_editing/composition_aware_mixin.dart'; import 'package:ui/src/engine/text_editing/input_type.dart'; import 'package:ui/src/engine/text_editing/text_editing.dart'; import 'package:ui/src/engine/util.dart'; From 8826b18dcaebc9fd0cf7fc9fb5c0689aa7235741 Mon Sep 17 00:00:00 2001 From: Anthony Oleinik Date: Wed, 25 May 2022 10:59:35 -0700 Subject: [PATCH 09/16] style fixes based on @justinmc comments --- .../text_editing/composition_aware_mixin.dart | 29 ++++++++------- .../src/engine/text_editing/text_editing.dart | 4 +-- lib/web_ui/test/composition_test.dart | 36 +++++++++++-------- 3 files changed, 38 insertions(+), 31 deletions(-) diff --git a/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart b/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart index 84f042cbe6c34..bb8bda70bdec9 100644 --- a/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart +++ b/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart @@ -6,7 +6,6 @@ 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 @@ -16,25 +15,25 @@ import 'text_editing.dart'; /// A class with this mixin should call [addCompositionEventHandlers] on initalization, and /// [removeCompositionEventHandlers] on deinitalization. /// -/// see also: +/// 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 HTML composition event type that triggers on starting a composition. + static const String _kCompositionStart = 'compositionstart'; - /// the name of the HTML composition event type that triggers on updating a composition. - static const String kCompositionUpdate = 'compositionupdate'; + /// The name of the HTML composition event type that triggers on updating a composition. + static const String _kCompositionUpdate = 'compositionupdate'; - /// the name of the HTML composition event type that triggers on ending a composition. - static const String kCompositionEnd = 'compositionend'; + /// The name of the HTML 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. + /// 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, @@ -42,15 +41,15 @@ mixin CompositionAwareMixin { String? composingText; void addCompositionEventHandlers(html.HtmlElement domElement) { - domElement.addEventListener(kCompositionStart, _compositionStartListener); - domElement.addEventListener(kCompositionUpdate, _compositionUpdateListener); - domElement.addEventListener(kCompositionEnd, _compositionEndListener); + 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); + domElement.removeEventListener(_kCompositionStart, _compositionStartListener); + domElement.removeEventListener(_kCompositionUpdate, _compositionUpdateListener); + domElement.removeEventListener(_kCompositionEnd, _compositionEndListener); } void _handleCompositionStart(html.Event event) { diff --git a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart index 7cac6a5c27eac..5658386e84681 100644 --- a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart +++ b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart @@ -757,10 +757,10 @@ class EditingState { /// The offset at which the text selection terminates. final int? extentOffset; - /// The offset at which text is still being composed. + /// The offset at which [CompositionAwareMixin.composingText] begins if any. final int? composingBaseOffset; - /// The offset at which composing text terminates. + /// The offset at which [CompositionAwareMixin.composingText] terminate, if any. final int? composingExtentOffset; /// Whether the current editing state is valid or not. diff --git a/lib/web_ui/test/composition_test.dart b/lib/web_ui/test/composition_test.dart index 1d21c005c3c34..a3a2a2e3b1990 100644 --- a/lib/web_ui/test/composition_test.dart +++ b/lib/web_ui/test/composition_test.dart @@ -19,7 +19,13 @@ void main() { internalBootstrapBrowserTest(() => testMain); } -class MockWithCompositionAwareMixin with CompositionAwareMixin {} +class MockWithCompositionAwareMixin with CompositionAwareMixin { + // These variables should be equal to their counterparts in CompositionAwareMixin. + // Seperate so the counterparts in CompositionAwareMixin can be private. + static const String _kCompositionUpdate = 'compositionupdate'; + static const String _kCompositionStart = 'compositionstart'; + static const String _kCompositionEnd = 'compositionend'; +} html.InputElement get inputElement { return defaultTextEditingRoot.querySelectorAll('input').first as html.InputElement; @@ -56,7 +62,7 @@ Future testMain() async { mockWithCompositionAwareMixin.composingText = fakeComposingText; mockWithCompositionAwareMixin.addCompositionEventHandlers(inputElement); - inputElement.dispatchEvent(html.Event(CompositionAwareMixin.kCompositionEnd)); + inputElement.dispatchEvent(html.Event(MockWithCompositionAwareMixin._kCompositionEnd)); expect(mockWithCompositionAwareMixin.composingText, null); }); @@ -69,7 +75,7 @@ Future testMain() async { mockWithCompositionAwareMixin.composingText = fakeComposingText; mockWithCompositionAwareMixin.addCompositionEventHandlers(inputElement); - inputElement.dispatchEvent(html.Event(CompositionAwareMixin.kCompositionStart)); + inputElement.dispatchEvent(html.Event(MockWithCompositionAwareMixin._kCompositionStart)); expect(mockWithCompositionAwareMixin.composingText, null); }); @@ -83,8 +89,7 @@ Future testMain() async { mockWithCompositionAwareMixin.composingText = fakeComposingText; mockWithCompositionAwareMixin.addCompositionEventHandlers(inputElement); - inputElement.dispatchEvent( - html.CompositionEvent(CompositionAwareMixin.kCompositionUpdate, data: fakeEventText)); + inputElement.dispatchEvent(html.CompositionEvent(MockWithCompositionAwareMixin._kCompositionUpdate, data: fakeEventText)); expect(mockWithCompositionAwareMixin.composingText, fakeEventText); }); @@ -111,7 +116,9 @@ Future testMain() async { mockWithCompositionAwareMixin.determineCompositionState(editingState), editingState.copyWith( composingBaseOffset: expectedComposingBase, - composingExtentOffset: expectedComposingBase + composingText.length)); + composingExtentOffset: expectedComposingBase + composingText.length + ) + ); }); }); }); @@ -120,10 +127,9 @@ Future testMain() async { test('should be [0, compostionStrLength] on new composition', () { const String composingText = 'hi'; - inputElement.dispatchEvent( - html.CompositionEvent(CompositionAwareMixin.kCompositionUpdate, data: composingText)); + inputElement.dispatchEvent(html.CompositionEvent(MockWithCompositionAwareMixin._kCompositionUpdate, data: composingText)); - //set the selection text + // Set the selection text. inputElement.value = composingText; inputElement.dispatchEvent(html.Event.eventType('Event', 'input')); @@ -133,7 +139,8 @@ Future testMain() async { .having((EditingState editingState) => editingState.composingBaseOffset, 'composingBaseOffset', 0) .having((EditingState editingState) => editingState.composingExtentOffset, - 'composingExtentOffset', composingText.length)); + 'composingExtentOffset', composingText.length) + ); }); test( @@ -143,14 +150,14 @@ Future testMain() async { const String beforeComposingText = 'beforeComposingText'; const String afterComposingText = 'afterComposingText'; - //type in the text box, then move cursor to the middle + // Type in the text box, then move cursor to the middle. inputElement.value = '$beforeComposingText$afterComposingText'; inputElement.setSelectionRange(beforeComposingText.length, beforeComposingText.length); inputElement.dispatchEvent( - html.CompositionEvent(CompositionAwareMixin.kCompositionUpdate, data: composingText)); + html.CompositionEvent(MockWithCompositionAwareMixin._kCompositionUpdate, data: composingText)); - //flush editing state (since we did not compositionend) + // Flush editing state (since we did not compositionend). inputElement.dispatchEvent(html.Event.eventType('Event', 'input')); expect( @@ -159,7 +166,8 @@ Future testMain() async { .having((EditingState editingState) => editingState.composingBaseOffset!, 'composingBaseOffset', beforeComposingText.length - composingText.length) .having((EditingState editingState) => editingState.composingExtentOffset, - 'composingExtentOffset', beforeComposingText.length)); + 'composingExtentOffset', beforeComposingText.length) + ); }); }); } From 8e172d0b42ea6198555f44d4d8f243531b2c9405 Mon Sep 17 00:00:00 2001 From: Anthony Oleinik Date: Wed, 25 May 2022 11:03:27 -0700 Subject: [PATCH 10/16] fixed minor typos in TextEditingState composing variables --- lib/web_ui/lib/src/engine/text_editing/text_editing.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart index 5658386e84681..8ef18dbf8a68c 100644 --- a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart +++ b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart @@ -757,10 +757,10 @@ class EditingState { /// The offset at which the text selection terminates. final int? extentOffset; - /// The offset at which [CompositionAwareMixin.composingText] begins if any. + /// The offset at which [CompositionAwareMixin.composingText] begins, if any. final int? composingBaseOffset; - /// The offset at which [CompositionAwareMixin.composingText] terminate, if any. + /// The offset at which [CompositionAwareMixin.composingText] terminates, if any. final int? composingExtentOffset; /// Whether the current editing state is valid or not. From d6986374d40e888317633d40a6495335dd28dbb8 Mon Sep 17 00:00:00 2001 From: Anthony Oleinik Date: Wed, 25 May 2022 14:38:48 -0700 Subject: [PATCH 11/16] made helpers in composition_test private --- lib/web_ui/test/composition_test.dart | 48 +++++++++++++-------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/lib/web_ui/test/composition_test.dart b/lib/web_ui/test/composition_test.dart index a3a2a2e3b1990..4f7848b0344bd 100644 --- a/lib/web_ui/test/composition_test.dart +++ b/lib/web_ui/test/composition_test.dart @@ -19,7 +19,7 @@ void main() { internalBootstrapBrowserTest(() => testMain); } -class MockWithCompositionAwareMixin with CompositionAwareMixin { +class _MockWithCompositionAwareMixin with CompositionAwareMixin { // These variables should be equal to their counterparts in CompositionAwareMixin. // Seperate so the counterparts in CompositionAwareMixin can be private. static const String _kCompositionUpdate = 'compositionupdate'; @@ -27,7 +27,7 @@ class MockWithCompositionAwareMixin with CompositionAwareMixin { static const String _kCompositionEnd = 'compositionend'; } -html.InputElement get inputElement { +html.InputElement get _inputElement { return defaultTextEditingRoot.querySelectorAll('input').first as html.InputElement; } @@ -57,12 +57,12 @@ Future testMain() async { group('$CompositionAwareMixin', () { group('composition end', () { test('should reset composing text on handle composition end', () { - final MockWithCompositionAwareMixin mockWithCompositionAwareMixin = - MockWithCompositionAwareMixin(); + final _MockWithCompositionAwareMixin mockWithCompositionAwareMixin = + _MockWithCompositionAwareMixin(); mockWithCompositionAwareMixin.composingText = fakeComposingText; - mockWithCompositionAwareMixin.addCompositionEventHandlers(inputElement); + mockWithCompositionAwareMixin.addCompositionEventHandlers(_inputElement); - inputElement.dispatchEvent(html.Event(MockWithCompositionAwareMixin._kCompositionEnd)); + _inputElement.dispatchEvent(html.Event(_MockWithCompositionAwareMixin._kCompositionEnd)); expect(mockWithCompositionAwareMixin.composingText, null); }); @@ -70,12 +70,12 @@ Future testMain() async { group('composition start', () { test('should reset composing text on handle composition start', () { - final MockWithCompositionAwareMixin mockWithCompositionAwareMixin = - MockWithCompositionAwareMixin(); + final _MockWithCompositionAwareMixin mockWithCompositionAwareMixin = + _MockWithCompositionAwareMixin(); mockWithCompositionAwareMixin.composingText = fakeComposingText; - mockWithCompositionAwareMixin.addCompositionEventHandlers(inputElement); + mockWithCompositionAwareMixin.addCompositionEventHandlers(_inputElement); - inputElement.dispatchEvent(html.Event(MockWithCompositionAwareMixin._kCompositionStart)); + _inputElement.dispatchEvent(html.Event(_MockWithCompositionAwareMixin._kCompositionStart)); expect(mockWithCompositionAwareMixin.composingText, null); }); @@ -84,12 +84,12 @@ Future testMain() async { group('composition update', () { test('should set composing text to event composing text', () { const String fakeEventText = 'IAmComposingThis'; - final MockWithCompositionAwareMixin mockWithCompositionAwareMixin = - MockWithCompositionAwareMixin(); + final _MockWithCompositionAwareMixin mockWithCompositionAwareMixin = + _MockWithCompositionAwareMixin(); mockWithCompositionAwareMixin.composingText = fakeComposingText; - mockWithCompositionAwareMixin.addCompositionEventHandlers(inputElement); + mockWithCompositionAwareMixin.addCompositionEventHandlers(_inputElement); - inputElement.dispatchEvent(html.CompositionEvent(MockWithCompositionAwareMixin._kCompositionUpdate, data: fakeEventText)); + _inputElement.dispatchEvent(html.CompositionEvent(_MockWithCompositionAwareMixin._kCompositionUpdate, data: fakeEventText)); expect(mockWithCompositionAwareMixin.composingText, fakeEventText); }); @@ -106,8 +106,8 @@ Future testMain() async { baseOffset: baseOffset, ); - final MockWithCompositionAwareMixin mockWithCompositionAwareMixin = - MockWithCompositionAwareMixin(); + final _MockWithCompositionAwareMixin mockWithCompositionAwareMixin = + _MockWithCompositionAwareMixin(); mockWithCompositionAwareMixin.composingText = composingText; const int expectedComposingBase = baseOffset - composingText.length; @@ -127,11 +127,11 @@ Future testMain() async { test('should be [0, compostionStrLength] on new composition', () { const String composingText = 'hi'; - inputElement.dispatchEvent(html.CompositionEvent(MockWithCompositionAwareMixin._kCompositionUpdate, data: composingText)); + _inputElement.dispatchEvent(html.CompositionEvent(_MockWithCompositionAwareMixin._kCompositionUpdate, data: composingText)); // Set the selection text. - inputElement.value = composingText; - inputElement.dispatchEvent(html.Event.eventType('Event', 'input')); + _inputElement.value = composingText; + _inputElement.dispatchEvent(html.Event.eventType('Event', 'input')); expect( editingStrategy.lastEditingState, @@ -151,14 +151,14 @@ Future testMain() async { const String afterComposingText = 'afterComposingText'; // Type in the text box, then move cursor to the middle. - inputElement.value = '$beforeComposingText$afterComposingText'; - inputElement.setSelectionRange(beforeComposingText.length, beforeComposingText.length); + _inputElement.value = '$beforeComposingText$afterComposingText'; + _inputElement.setSelectionRange(beforeComposingText.length, beforeComposingText.length); - inputElement.dispatchEvent( - html.CompositionEvent(MockWithCompositionAwareMixin._kCompositionUpdate, data: composingText)); + _inputElement.dispatchEvent( + html.CompositionEvent(_MockWithCompositionAwareMixin._kCompositionUpdate, data: composingText)); // Flush editing state (since we did not compositionend). - inputElement.dispatchEvent(html.Event.eventType('Event', 'input')); + _inputElement.dispatchEvent(html.Event.eventType('Event', 'input')); expect( editingStrategy.lastEditingState, From 878ce0a4d9007bf7a7138954b3542eec4d6c2855 Mon Sep 17 00:00:00 2001 From: Anthony Oleinik Date: Wed, 25 May 2022 14:38:48 -0700 Subject: [PATCH 12/16] made helpers in composition_test private --- .../text_editing/composition_aware_mixin.dart | 1 + .../src/engine/text_editing/text_editing.dart | 5 +- lib/web_ui/test/composition_test.dart | 158 ++++++++++++------ lib/web_ui/test/text_editing_test.dart | 2 + 4 files changed, 113 insertions(+), 53 deletions(-) diff --git a/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart b/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart index bb8bda70bdec9..bdf1f43f48149 100644 --- a/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart +++ b/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart @@ -84,6 +84,7 @@ mixin CompositionAwareMixin { if (composingBase < 0) { return editingState; } + return editingState.copyWith( composingBaseOffset: composingBase, composingExtentOffset: composingBase + composingText!.length, diff --git a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart index 8ef18dbf8a68c..e8ecc192c5ca8 100644 --- a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart +++ b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart @@ -509,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; @@ -619,6 +618,8 @@ class TextEditingDeltaState { 'deltaEnd': deltaEnd, 'selectionBase': baseOffset, 'selectionExtent': extentOffset, + 'composingBase': composingOffset, + 'composingExtent': composingExtent }, ], }; @@ -1294,6 +1295,8 @@ abstract class DefaultTextEditingStrategy with CompositionAwareMixin implements TextEditingDeltaState? newTextEditingDeltaState; if (inputConfiguration.enableDeltaModel) { newTextEditingDeltaState = TextEditingDeltaState.inferDeltaState(newEditingState, lastEditingState, editingDeltaState); + editingDeltaState.composingOffset = newEditingState.composingBaseOffset; + editingDeltaState.composingExtent = newEditingState.composingExtentOffset; } if (newEditingState != lastEditingState) { diff --git a/lib/web_ui/test/composition_test.dart b/lib/web_ui/test/composition_test.dart index a3a2a2e3b1990..0ba022f2acfc5 100644 --- a/lib/web_ui/test/composition_test.dart +++ b/lib/web_ui/test/composition_test.dart @@ -13,13 +13,11 @@ import 'package:ui/src/engine/text_editing/composition_aware_mixin.dart'; import 'package:ui/src/engine/text_editing/input_type.dart'; import 'package:ui/src/engine/text_editing/text_editing.dart'; -import 'text_editing_test.dart'; - void main() { internalBootstrapBrowserTest(() => testMain); } -class MockWithCompositionAwareMixin with CompositionAwareMixin { +class _MockWithCompositionAwareMixin with CompositionAwareMixin { // These variables should be equal to their counterparts in CompositionAwareMixin. // Seperate so the counterparts in CompositionAwareMixin can be private. static const String _kCompositionUpdate = 'compositionupdate'; @@ -27,42 +25,51 @@ class MockWithCompositionAwareMixin with CompositionAwareMixin { static const String _kCompositionEnd = 'compositionend'; } -html.InputElement get inputElement { +html.InputElement get _inputElement { return defaultTextEditingRoot.querySelectorAll('input').first as html.InputElement; } +GloballyPositionedTextEditingStrategy _enableEditingStrategy({ + required bool deltaModel, + void Function(EditingState?, TextEditingDeltaState?)? onChange, + }) { + final HybridTextEditing owner = HybridTextEditing(); + + owner.configuration = InputConfiguration(inputType: EngineInputType.text, enableDeltaModel: deltaModel); + + final GloballyPositionedTextEditingStrategy editingStrategy = + GloballyPositionedTextEditingStrategy(owner); + + owner.debugTextEditingStrategyOverride = editingStrategy; + + editingStrategy.enable(owner.configuration!, onChange: onChange ?? (_, __) {}, onAction: (_) {}); + return editingStrategy; +} + Future testMain() async { await initializeEngine(); - final GloballyPositionedTextEditingStrategy editingStrategy = - GloballyPositionedTextEditingStrategy(HybridTextEditing() - ..configuration = InputConfiguration( - inputType: EngineInputType.text, - )); - editingStrategy.owner.debugTextEditingStrategyOverride = editingStrategy; const String fakeComposingText = 'ImComposingText'; - setUp(() { - editingStrategy.enable( - singlelineConfig, - onChange: (_, __) {}, - onAction: (_) {}, - ); - }); + group('$CompositionAwareMixin', () { + late TextEditingStrategy editingStrategy; - tearDown(() { - editingStrategy.disable(); - }); + setUp(() { + editingStrategy = _enableEditingStrategy(deltaModel: false); + }); + + tearDown(() { + editingStrategy.disable(); + }); - group('$CompositionAwareMixin', () { group('composition end', () { test('should reset composing text on handle composition end', () { - final MockWithCompositionAwareMixin mockWithCompositionAwareMixin = - MockWithCompositionAwareMixin(); + final _MockWithCompositionAwareMixin mockWithCompositionAwareMixin = + _MockWithCompositionAwareMixin(); mockWithCompositionAwareMixin.composingText = fakeComposingText; - mockWithCompositionAwareMixin.addCompositionEventHandlers(inputElement); + mockWithCompositionAwareMixin.addCompositionEventHandlers(_inputElement); - inputElement.dispatchEvent(html.Event(MockWithCompositionAwareMixin._kCompositionEnd)); + _inputElement.dispatchEvent(html.Event(_MockWithCompositionAwareMixin._kCompositionEnd)); expect(mockWithCompositionAwareMixin.composingText, null); }); @@ -70,12 +77,12 @@ Future testMain() async { group('composition start', () { test('should reset composing text on handle composition start', () { - final MockWithCompositionAwareMixin mockWithCompositionAwareMixin = - MockWithCompositionAwareMixin(); + final _MockWithCompositionAwareMixin mockWithCompositionAwareMixin = + _MockWithCompositionAwareMixin(); mockWithCompositionAwareMixin.composingText = fakeComposingText; - mockWithCompositionAwareMixin.addCompositionEventHandlers(inputElement); + mockWithCompositionAwareMixin.addCompositionEventHandlers(_inputElement); - inputElement.dispatchEvent(html.Event(MockWithCompositionAwareMixin._kCompositionStart)); + _inputElement.dispatchEvent(html.Event(_MockWithCompositionAwareMixin._kCompositionStart)); expect(mockWithCompositionAwareMixin.composingText, null); }); @@ -84,16 +91,18 @@ Future testMain() async { group('composition update', () { test('should set composing text to event composing text', () { const String fakeEventText = 'IAmComposingThis'; - final MockWithCompositionAwareMixin mockWithCompositionAwareMixin = - MockWithCompositionAwareMixin(); + final _MockWithCompositionAwareMixin mockWithCompositionAwareMixin = + _MockWithCompositionAwareMixin(); mockWithCompositionAwareMixin.composingText = fakeComposingText; - mockWithCompositionAwareMixin.addCompositionEventHandlers(inputElement); + mockWithCompositionAwareMixin.addCompositionEventHandlers(_inputElement); - inputElement.dispatchEvent(html.CompositionEvent(MockWithCompositionAwareMixin._kCompositionUpdate, data: fakeEventText)); + _inputElement.dispatchEvent(html.CompositionEvent( + _MockWithCompositionAwareMixin._kCompositionUpdate, + data: fakeEventText)); expect(mockWithCompositionAwareMixin.composingText, fakeEventText); }); - }); + }, skip: true); group('determine composition state', () { test('should return new composition state if valid new composition', () { @@ -106,8 +115,8 @@ Future testMain() async { baseOffset: baseOffset, ); - final MockWithCompositionAwareMixin mockWithCompositionAwareMixin = - MockWithCompositionAwareMixin(); + final _MockWithCompositionAwareMixin mockWithCompositionAwareMixin = + _MockWithCompositionAwareMixin(); mockWithCompositionAwareMixin.composingText = composingText; const int expectedComposingBase = baseOffset - composingText.length; @@ -116,22 +125,30 @@ Future testMain() async { mockWithCompositionAwareMixin.determineCompositionState(editingState), editingState.copyWith( composingBaseOffset: expectedComposingBase, - composingExtentOffset: expectedComposingBase + composingText.length - ) - ); + composingExtentOffset: expectedComposingBase + composingText.length)); }); }); - }); + }, skip: true); group('composing range', () { + late GloballyPositionedTextEditingStrategy editingStrategy; + + setUp(() { + editingStrategy = _enableEditingStrategy(deltaModel: false); + }); + + tearDown(() { + editingStrategy.disable(); + }); + test('should be [0, compostionStrLength] on new composition', () { const String composingText = 'hi'; - inputElement.dispatchEvent(html.CompositionEvent(MockWithCompositionAwareMixin._kCompositionUpdate, data: composingText)); + _inputElement.dispatchEvent(html.CompositionEvent(_MockWithCompositionAwareMixin._kCompositionUpdate, data: composingText)); // Set the selection text. - inputElement.value = composingText; - inputElement.dispatchEvent(html.Event.eventType('Event', 'input')); + _inputElement.value = composingText; + _inputElement.dispatchEvent(html.Event.eventType('Event', 'input')); expect( editingStrategy.lastEditingState, @@ -139,8 +156,7 @@ Future testMain() async { .having((EditingState editingState) => editingState.composingBaseOffset, 'composingBaseOffset', 0) .having((EditingState editingState) => editingState.composingExtentOffset, - 'composingExtentOffset', composingText.length) - ); + 'composingExtentOffset', composingText.length)); }); test( @@ -151,14 +167,15 @@ Future testMain() async { const String afterComposingText = 'afterComposingText'; // Type in the text box, then move cursor to the middle. - inputElement.value = '$beforeComposingText$afterComposingText'; - inputElement.setSelectionRange(beforeComposingText.length, beforeComposingText.length); + _inputElement.value = '$beforeComposingText$afterComposingText'; + _inputElement.setSelectionRange(beforeComposingText.length, beforeComposingText.length); - inputElement.dispatchEvent( - html.CompositionEvent(MockWithCompositionAwareMixin._kCompositionUpdate, data: composingText)); + _inputElement.dispatchEvent(html.CompositionEvent( + _MockWithCompositionAwareMixin._kCompositionUpdate, + data: composingText)); // Flush editing state (since we did not compositionend). - inputElement.dispatchEvent(html.Event.eventType('Event', 'input')); + _inputElement.dispatchEvent(html.Event.eventType('Event', 'input')); expect( editingStrategy.lastEditingState, @@ -166,8 +183,45 @@ Future testMain() async { .having((EditingState editingState) => editingState.composingBaseOffset!, 'composingBaseOffset', beforeComposingText.length - composingText.length) .having((EditingState editingState) => editingState.composingExtentOffset, - 'composingExtentOffset', beforeComposingText.length) - ); + 'composingExtentOffset', beforeComposingText.length)); + }); + }, skip: true); + + group('Text Editing Delta Model', () { + late GloballyPositionedTextEditingStrategy editingStrategy; + + final StreamController deltaStream = + StreamController.broadcast(); + + setUp(() { + editingStrategy = _enableEditingStrategy(deltaModel: true, onChange: (_, TextEditingDeltaState? deltaState) => deltaStream.add(deltaState)); + }); + + tearDown(() { + editingStrategy.disable(); + }); + + //ANCHOR here + test('should have newly entered composing characters', () async { + const String newComposingText = 'n'; + + editingStrategy.setEditingState(EditingState(text: newComposingText, baseOffset: 1, extentOffset: 1)); + + final Future containExpect = expectLater( + deltaStream.stream.first, + completion(isA() + .having((TextEditingDeltaState deltaState) => deltaState.composingOffset, 'composingOffset', 0) + .having((TextEditingDeltaState deltaState) => deltaState.composingExtent, 'composingExtent', newComposingText.length) + .having((TextEditingDeltaState deltaState) => deltaState.baseOffset, 'baseOffset', 0) + .having((TextEditingDeltaState deltaState) => deltaState.extentOffset, 'extentOffset', 0) + )); + + + _inputElement.dispatchEvent(html.CompositionEvent( + _MockWithCompositionAwareMixin._kCompositionUpdate, + data: newComposingText)); + + await containExpect; }); }); } diff --git a/lib/web_ui/test/text_editing_test.dart b/lib/web_ui/test/text_editing_test.dart index dc137a6f5a4bc..96c4d69f68543 100644 --- a/lib/web_ui/test/text_editing_test.dart +++ b/lib/web_ui/test/text_editing_test.dart @@ -1635,6 +1635,8 @@ Future testMain() async { 'deltaEnd': -1, 'selectionBase': 2, 'selectionExtent': 5, + 'composingBase': null, + 'composingExtent': null } ], } From c884e8b67631f1619c43989e4cc1ef8a44f96d26 Mon Sep 17 00:00:00 2001 From: Anthony Oleinik Date: Tue, 31 May 2022 21:17:36 -0700 Subject: [PATCH 13/16] completed composing tests for delta --- .../src/engine/text_editing/text_editing.dart | 2 +- lib/web_ui/test/composition_test.dart | 39 +++++++++++++++---- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart index e8ecc192c5ca8..6c6229d0b33ad 100644 --- a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart +++ b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart @@ -1294,9 +1294,9 @@ abstract class DefaultTextEditingStrategy with CompositionAwareMixin implements TextEditingDeltaState? newTextEditingDeltaState; if (inputConfiguration.enableDeltaModel) { - newTextEditingDeltaState = TextEditingDeltaState.inferDeltaState(newEditingState, lastEditingState, editingDeltaState); editingDeltaState.composingOffset = newEditingState.composingBaseOffset; editingDeltaState.composingExtent = newEditingState.composingExtentOffset; + newTextEditingDeltaState = TextEditingDeltaState.inferDeltaState(newEditingState, lastEditingState, editingDeltaState); } if (newEditingState != lastEditingState) { diff --git a/lib/web_ui/test/composition_test.dart b/lib/web_ui/test/composition_test.dart index 0ba022f2acfc5..b134181770e9a 100644 --- a/lib/web_ui/test/composition_test.dart +++ b/lib/web_ui/test/composition_test.dart @@ -102,7 +102,7 @@ Future testMain() async { expect(mockWithCompositionAwareMixin.composingText, fakeEventText); }); - }, skip: true); + }); group('determine composition state', () { test('should return new composition state if valid new composition', () { @@ -128,7 +128,7 @@ Future testMain() async { composingExtentOffset: expectedComposingBase + composingText.length)); }); }); - }, skip: true); + }); group('composing range', () { late GloballyPositionedTextEditingStrategy editingStrategy; @@ -185,7 +185,7 @@ Future testMain() async { .having((EditingState editingState) => editingState.composingExtentOffset, 'composingExtentOffset', beforeComposingText.length)); }); - }, skip: true); + }); group('Text Editing Delta Model', () { late GloballyPositionedTextEditingStrategy editingStrategy; @@ -194,14 +194,16 @@ Future testMain() async { StreamController.broadcast(); setUp(() { - editingStrategy = _enableEditingStrategy(deltaModel: true, onChange: (_, TextEditingDeltaState? deltaState) => deltaStream.add(deltaState)); + editingStrategy = _enableEditingStrategy( + deltaModel: true, + onChange: (_, TextEditingDeltaState? deltaState) => deltaStream.add(deltaState) + ); }); tearDown(() { editingStrategy.disable(); }); - //ANCHOR here test('should have newly entered composing characters', () async { const String newComposingText = 'n'; @@ -212,8 +214,6 @@ Future testMain() async { completion(isA() .having((TextEditingDeltaState deltaState) => deltaState.composingOffset, 'composingOffset', 0) .having((TextEditingDeltaState deltaState) => deltaState.composingExtent, 'composingExtent', newComposingText.length) - .having((TextEditingDeltaState deltaState) => deltaState.baseOffset, 'baseOffset', 0) - .having((TextEditingDeltaState deltaState) => deltaState.extentOffset, 'extentOffset', 0) )); @@ -223,5 +223,30 @@ Future testMain() async { await containExpect; }); + + test('should emit changed composition', () async { + const String newComposingCharsInOrder = 'hiCompose'; + + for (int currCharIndex = 0; currCharIndex < newComposingCharsInOrder.length; currCharIndex++) { + final String currComposingSubstr = newComposingCharsInOrder.substring(0, currCharIndex + 1); + + editingStrategy.setEditingState( + EditingState(text: currComposingSubstr, baseOffset: currCharIndex + 1, extentOffset: currCharIndex + 1) + ); + + final Future containExpect = expectLater( + deltaStream.stream.first, + completion(isA() + .having((TextEditingDeltaState deltaState) => deltaState.composingOffset, 'composingOffset', 0) + .having((TextEditingDeltaState deltaState) => deltaState.composingExtent, 'composingExtent', currCharIndex + 1) + )); + + _inputElement.dispatchEvent(html.CompositionEvent( + _MockWithCompositionAwareMixin._kCompositionUpdate, + data: currComposingSubstr)); + + await containExpect; + } + }); }); } From c8011d71a3b8d6db80498a29acefe4eb0b1dc0cb Mon Sep 17 00:00:00 2001 From: Anthony Oleinik Date: Thu, 2 Jun 2022 12:05:59 -0700 Subject: [PATCH 14/16] skipped failingtest on firefox bc to prexist bug --- lib/web_ui/test/composition_test.dart | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/web_ui/test/composition_test.dart b/lib/web_ui/test/composition_test.dart index b134181770e9a..fd624b6dfdced 100644 --- a/lib/web_ui/test/composition_test.dart +++ b/lib/web_ui/test/composition_test.dart @@ -7,6 +7,7 @@ import 'dart:html' as html; import 'package:test/bootstrap/browser.dart'; import 'package:test/test.dart'; +import 'package:ui/src/engine/browser_detection.dart'; import 'package:ui/src/engine/initialization.dart'; import 'package:ui/src/engine/text_editing/composition_aware_mixin.dart'; @@ -247,6 +248,11 @@ Future testMain() async { await containExpect; } - }); + }, + // TODO(antholeole): This test fails on Firefox because of how it orders events; + // it's likely that this will be fixed by https://github.com/flutter/flutter/issues/105243. + // Until the refactor gets merged, this test should run on all other browsers to prevent + // regressions in the meantime. + skip: browserEngine == BrowserEngine.firefox); }); } From a1e34aea27225461be1304df24c6e90c6a3c3296 Mon Sep 17 00:00:00 2001 From: Anthony Oleinik Date: Thu, 2 Jun 2022 15:07:46 -0700 Subject: [PATCH 15/16] reverted accidental Object.hash -> ui.hashValues change --- lib/web_ui/lib/src/engine/text_editing/text_editing.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart index 30b1d0482cc5b..3f1c9b6d9b3c0 100644 --- a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart +++ b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart @@ -768,9 +768,9 @@ class EditingState { bool get isValid => baseOffset! >= 0 && extentOffset! >= 0; @override - int get hashCode => ui.hashValues( + int get hashCode => Object.hash( text, baseOffset, extentOffset, composingBaseOffset, composingExtentOffset); - + @override bool operator ==(Object other) { if (identical(this, other)) { From 7334e0af21f75624f714e8e8eafba80e36a263af Mon Sep 17 00:00:00 2001 From: Anthony Oleinik Date: Fri, 3 Jun 2022 15:31:49 -0700 Subject: [PATCH 16/16] responded to @justinmc comments --- .../text_editing/composition_aware_mixin.dart | 16 ++++--------- .../src/engine/text_editing/text_editing.dart | 7 +++--- lib/web_ui/test/composition_test.dart | 12 ++++++---- lib/web_ui/test/text_editing_test.dart | 24 +++++++++---------- 4 files changed, 27 insertions(+), 32 deletions(-) diff --git a/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart b/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart index bdf1f43f48149..fdab6ad3b582d 100644 --- a/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart +++ b/lib/web_ui/lib/src/engine/text_editing/composition_aware_mixin.dart @@ -23,17 +23,17 @@ 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 HTML composition event type that triggers on updating a composition. + /// The name of the browser composition event type that triggers on updating a composition. static const String _kCompositionUpdate = 'compositionupdate'; - /// The name of the HTML composition event type that triggers on ending a composition. + /// 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. + /// 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, @@ -67,15 +67,7 @@ mixin CompositionAwareMixin { } EditingState determineCompositionState(EditingState editingState) { - if (editingState.baseOffset == null) { - return editingState; - } - - if (composingText == null) { - return editingState; - } - - if (editingState.text == null) { + if (editingState.baseOffset == null || composingText == null || editingState.text == null) { return editingState; } diff --git a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart index 3f1c9b6d9b3c0..d171482e13b28 100644 --- a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart +++ b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart @@ -745,8 +745,8 @@ class EditingState { 'text': text, 'selectionBase': baseOffset, 'selectionExtent': extentOffset, - 'composingBase': composingBaseOffset ?? -1, - 'composingExtent': composingExtentOffset ?? -1, + 'composingBase': composingBaseOffset, + 'composingExtent': composingExtentOffset, }; /// The current text being edited. @@ -769,7 +769,8 @@ class EditingState { @override int get hashCode => Object.hash( - text, baseOffset, extentOffset, composingBaseOffset, composingExtentOffset); + text, baseOffset, extentOffset, composingBaseOffset, composingExtentOffset + ); @override bool operator ==(Object other) { diff --git a/lib/web_ui/test/composition_test.dart b/lib/web_ui/test/composition_test.dart index fd624b6dfdced..54c543d5bd3c9 100644 --- a/lib/web_ui/test/composition_test.dart +++ b/lib/web_ui/test/composition_test.dart @@ -20,7 +20,7 @@ void main() { class _MockWithCompositionAwareMixin with CompositionAwareMixin { // These variables should be equal to their counterparts in CompositionAwareMixin. - // Seperate so the counterparts in CompositionAwareMixin can be private. + // Separate so the counterparts in CompositionAwareMixin can be private. static const String _kCompositionUpdate = 'compositionupdate'; static const String _kCompositionStart = 'compositionstart'; static const String _kCompositionEnd = 'compositionend'; @@ -31,8 +31,8 @@ html.InputElement get _inputElement { } GloballyPositionedTextEditingStrategy _enableEditingStrategy({ - required bool deltaModel, - void Function(EditingState?, TextEditingDeltaState?)? onChange, + required bool deltaModel, + void Function(EditingState?, TextEditingDeltaState?)? onChange, }) { final HybridTextEditing owner = HybridTextEditing(); @@ -99,7 +99,8 @@ Future testMain() async { _inputElement.dispatchEvent(html.CompositionEvent( _MockWithCompositionAwareMixin._kCompositionUpdate, - data: fakeEventText)); + data: fakeEventText + )); expect(mockWithCompositionAwareMixin.composingText, fakeEventText); }); @@ -173,7 +174,8 @@ Future testMain() async { _inputElement.dispatchEvent(html.CompositionEvent( _MockWithCompositionAwareMixin._kCompositionUpdate, - data: composingText)); + data: composingText + )); // Flush editing state (since we did not compositionend). _inputElement.dispatchEvent(html.Event.eventType('Event', 'input')); diff --git a/lib/web_ui/test/text_editing_test.dart b/lib/web_ui/test/text_editing_test.dart index 6eb7ca2fce712..18aa6cf107b26 100644 --- a/lib/web_ui/test/text_editing_test.dart +++ b/lib/web_ui/test/text_editing_test.dart @@ -1552,8 +1552,8 @@ Future testMain() async { 'text': 'something', 'selectionBase': 9, 'selectionExtent': 9, - 'composingBase': -1, - 'composingExtent': -1 + 'composingBase': null, + 'composingExtent': null } ], ); @@ -1578,8 +1578,8 @@ Future testMain() async { 'text': 'something', 'selectionBase': 2, 'selectionExtent': 5, - 'composingBase': -1, - 'composingExtent': -1 + 'composingBase': null, + 'composingExtent': null } ], ); @@ -1716,8 +1716,8 @@ Future testMain() async { 'text': 'something', 'selectionBase': 9, 'selectionExtent': 9, - 'composingBase': -1, - 'composingExtent': -1 + 'composingBase': null, + 'composingExtent': null } }, ], @@ -1756,8 +1756,8 @@ Future testMain() async { 'text': 'foo\nbar', 'selectionBase': 2, 'selectionExtent': 3, - 'composingBase': -1, - 'composingExtent': -1 + 'composingBase': null, + 'composingExtent': null }); sendFrameworkMessage(codec.encodeMethodCall(setEditingState)); checkTextAreaEditingState(textarea, 'foo\nbar', 2, 3); @@ -1787,8 +1787,8 @@ Future testMain() async { 'text': 'something\nelse', 'selectionBase': 14, 'selectionExtent': 14, - 'composingBase': -1, - 'composingExtent': -1 + 'composingBase': null, + 'composingExtent': null } ], ); @@ -1803,8 +1803,8 @@ Future testMain() async { 'text': 'something\nelse', 'selectionBase': 2, 'selectionExtent': 5, - 'composingBase': -1, - 'composingExtent': -1 + 'composingBase': null, + 'composingExtent': null } ], );