add editableTextId - a stable unique TextEditable identifier#79683
add editableTextId - a stable unique TextEditable identifier#79683yjbanov wants to merge 2 commits into
Conversation
| } | ||
| } | ||
|
|
||
| int get editableTextId => _editableTextId; |
There was a problem hiding this comment.
here and elsewhere: missing docs
There was a problem hiding this comment.
Yeah, lots of clean up to do, lots of docs to write. For now just sending a draft. Also scheduled a VC for this Thursday to discuss if we're OK with this approach overall.
| this.tags, | ||
| this.transform, | ||
| this.customSemanticsActionIds, | ||
| required this.editableTextId, |
There was a problem hiding this comment.
Should this only be set if the flags indicate that it is a textfield? Maybe assert?
There was a problem hiding this comment.
Unfortunately, const asserts don't support the expression flag & SemanticsFlag.isTextField.value == 0. If you can think of a workaround, I'm all ears. FWIW, I'll have this assertion in dart:ui.
| assert(value != null); | ||
| if (value != _editableTextId) { | ||
| _editableTextId = value; | ||
| markNeedsPaint(); |
There was a problem hiding this comment.
Why does this require a paint update (seems to be only used in describeSemanticsConfiguration)?
There was a problem hiding this comment.
Done. For some reason I thought a semantics change always implied a repaint, but I guess that doesn't have to be the case.
| /// configurations as determined by [isCompatibleWith]. | ||
| void absorb(SemanticsConfiguration child) { | ||
| assert(!explicitChildNodes); | ||
| assert( |
There was a problem hiding this comment.
Is this condition already ensured by isCompatibleWith? isCompatibleWith should probably mark configurations that have different _editableTextIds as incompatible?
There was a problem hiding this comment.
Done. I would also like to assert that no two editables are nested. Is there a good place for such an assertion?
There was a problem hiding this comment.
Although, I suppose a single text field can be described by two nodes each supplying a different subset of semantic properties, so maybe we should allow it.
Instead of inventing a new ID could we just make clientID live longer and use that? |
If we are sure that |
|
Closing in favor of an approach that doesn't require framework-side changes (flutter-team-archive/engine#25797) |
Attach an
editableTextIdvalue to theEditableTextStatethat uniquely identifies the text field and pass it to the engine via text input channel and semantics nodes. This field will be used to fix a race between theflutter/textinputchannel and semantics.This makes progress towards fixing #52106 (engine changes are required to fully fix this).
Background
On the web, the semantics tree is responsible for hosting the
<input>elements. The semantics update happens after theTextInputConnectionis established. Conversely, the semantics tree can create<input>elements before text input connection is established. This leads to the following bad situations leading to various bugs:<input>element to sync to.<input>element.<input>is removed (due to lazy rendering) while a text input connection is still alive.Solution
editableTextIdoutlivesclientId(which is unique to a singleTextInputConfiguration), and it is the single source of truth for both semantics and the text input channel. Whether one or the other initiates the creation of an<input>element, they can connect by matching on theeditableTextId.Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.