Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[web] - fix Safari textfield selection bug #47917

Merged
auto-submit merged 2 commits into from
Nov 13, 2023
Merged

[web] - fix Safari textfield selection bug #47917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b6b9df8
Fix for Safari textfield selection issue
htoor3 Nov 10, 2023
bc7b884
fix whitespace
htoor3 Nov 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions lib/web_ui/lib/src/engine/text_editing/text_editing.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,14 @@ class EngineAutofillForm {
}

void placeForm(DomHTMLElement mainTextEditingElement) {
// Since we're disabling pointer events on the form to fix Safari autofill,
// we need to explicitly set pointer events on the active input element in
// order to calculate the correct pointer event offsets.
// See: https://github.com/flutter/flutter/issues/136006
if(textEditing.strategy is SafariDesktopTextEditingStrategy) {
mainTextEditingElement.style.pointerEvents = 'all';
Copy link
Contributor

@mdebbar mdebbar Nov 10, 2023

Choose a reason for hiding this comment

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

I have two questions here:

  1. Do we need to set this back to none after the input loses focus?
  2. Should we preemptively set pointer-events: all for all inputs in EngineAutofillForm.fromFrameworkMessage()?

Copy link
Contributor Author

@htoor3 htoor3 Nov 10, 2023

Choose a reason for hiding this comment

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

  1. We only need to ensure that it remains none if it remains on the screen and another input on the form is focused. This is being automatically handled for us in fromFrameworkMessage inside EngineAutofillForm. This happens when we click any other input in the form. The other case where it loses focus is where we just click anywhere outside of the input and the form. In this case, the input gets moved offscreen so we don't have to worry about it.
  2. We can't do this because of the Safari autofill fix where we need all the non-active inputs to be on screen but hidden from the user. We actually preemptively disable pointer events for non active inputs in EngineAutofillForm.fromFrameworkMessage() so that we don't have pointer collisions if users inadvertently hover/click over those hidden inputs

Copy link
Contributor

@mdebbar mdebbar Nov 13, 2023

Choose a reason for hiding this comment

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

Okay that makes sense. Thanks for explaining this!

}

formElement.insertBefore(mainTextEditingElement, insertionReferenceNode);
defaultTextEditingRoot.append(formElement);
}
Expand Down
40 changes: 40 additions & 0 deletions lib/web_ui/test/engine/text_editing_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2551,6 +2551,46 @@ Future<void> testMain() async {
expect(autofillForm.formElement.style.pointerEvents, 'none');
}, skip: !isSafari);

test(
'the focused element within a form should explicitly set pointer events on Safari',
() {
final List<dynamic> fields = createFieldValues(<String>[
'email',
'username',
'password',
], <String>[
'field1',
'field2',
'field3'
]);
final EngineAutofillForm autofillForm =
EngineAutofillForm.fromFrameworkMessage(
createAutofillInfo('email', 'field1'), fields)!;

final DomHTMLInputElement testInputElement = createDomHTMLInputElement();
testInputElement.name = 'email';
autofillForm.placeForm(testInputElement);

final List<DomHTMLInputElement> formChildNodes =
autofillForm.formElement.childNodes.toList()
as List<DomHTMLInputElement>;
final DomHTMLInputElement email = formChildNodes[0];
final DomHTMLInputElement username = formChildNodes[1];
final DomHTMLInputElement password = formChildNodes[2];

expect(email.name, 'email');
expect(username.name, 'username');
expect(password.name, 'current-password');

// pointer events are none on the form and all non-focused elements
expect(autofillForm.formElement.style.pointerEvents, 'none');
expect(username.style.pointerEvents, 'none');
expect(password.style.pointerEvents, 'none');

// pointer events are set to all on the activeDomElement
expect(email.style.pointerEvents, 'all');
}, skip: !isSafari);

tearDown(() {
clearForms();
});
Expand Down