Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Avoid passive clipboard read on Android" #87375

Merged
merged 1 commit into from Aug 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 8 additions & 16 deletions packages/flutter/lib/src/widgets/text_selection.dart
Expand Up @@ -1655,26 +1655,18 @@ class ClipboardStatusNotifier extends ValueNotifier<ClipboardStatus> with Widget

/// Check the [Clipboard] and update [value] if needed.
Future<void> update() async {
// iOS 14 added a notification that appears when an app accesses the
// clipboard. To avoid the notification, don't access the clipboard on iOS,
// and instead always show the paste button, even when the clipboard is
// empty.
// TODO(justinmc): Use the new iOS 14 clipboard API method hasStrings that
// won't trigger the notification.
// https://github.com/flutter/flutter/issues/60145
switch (defaultTargetPlatform) {
// Android 12 introduces a toast message that appears when an app reads
// the content on the clipboard. To avoid unintended access, both the
// Flutter engine and the Flutter framework need to be updated to use the
// appropriate API to check whether the clipboard is empty.
// As a short-term workaround, always show the paste button.
// TODO(justinmc): Expose `hasStrings` in `Clipboard` and use that instead
// https://github.com/flutter/flutter/issues/74139
case TargetPlatform.android:
// Intentionally fall through.
// iOS 14 added a notification that appears when an app accesses the
// clipboard. To avoid the notification, don't access the clipboard on iOS,
// and instead always show the paste button, even when the clipboard is
// empty.
// TODO(justinmc): Use the new iOS 14 clipboard API method hasStrings that
// won't trigger the notification.
// https://github.com/flutter/flutter/issues/60145
case TargetPlatform.iOS:
value = ClipboardStatus.pasteable;
return;
case TargetPlatform.android:
case TargetPlatform.fuchsia:
case TargetPlatform.linux:
case TargetPlatform.macOS:
Expand Down
20 changes: 7 additions & 13 deletions packages/flutter/test/material/text_field_test.dart
Expand Up @@ -928,7 +928,7 @@ void main() {
),
);
focusNode.requestFocus();
await tester.pumpAndSettle();
await tester.pump();

await expectLater(
find.byType(TextField),
Expand Down Expand Up @@ -956,7 +956,7 @@ void main() {
),
);
focusNode.requestFocus();
await tester.pumpAndSettle();
await tester.pump();

await expectLater(
find.byType(TextField),
Expand Down Expand Up @@ -984,7 +984,7 @@ void main() {
),
);
focusNode.requestFocus();
await tester.pumpAndSettle();
await tester.pump();

await expectLater(
find.byType(TextField),
Expand Down Expand Up @@ -6223,13 +6223,10 @@ void main() {

semantics.dispose();

// On web, we don't check for pasteability because that triggers a
// permission dialog in the browser.
// On web (just like iOS), we don't check for pasteability because that
// triggers a permission dialog in the browser.
// https://github.com/flutter/flutter/pull/57139#issuecomment-629048058
// TODO(justinmc): Remove TargetPlatform override when Android and iOS uses
// `hasStrings` to check for pasteability.
// https://github.com/flutter/flutter/issues/74139
}, skip: isBrowser, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.linux }));
}, skip: isBrowser);

testWidgets('TextField throws when not descended from a Material widget', (WidgetTester tester) async {
const Widget textField = TextField();
Expand Down Expand Up @@ -9591,10 +9588,7 @@ void main() {
// pasted.
expect(triedToReadClipboard, true);
}
// TODO(justinmc): Eventually, all platform should call `hasStrings` instead.
// This entire test will become irrelevant after the fact.
// https://github.com/flutter/flutter/issues/74139
}, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.linux }));
});

testWidgets('TextField changes mouse cursor when hovered', (WidgetTester tester) async {
await tester.pumpWidget(
Expand Down
10 changes: 3 additions & 7 deletions packages/flutter/test/material/text_selection_test.dart
Expand Up @@ -615,9 +615,7 @@ void main() {
});
});

// TODO(justinmc): Paste should only appears when the clipboard has contents.
// https://github.com/flutter/flutter/issues/74139
testWidgets('Paste always appears regardless of clipboard content on Android', (WidgetTester tester) async {
testWidgets('Paste only appears when clipboard has contents', (WidgetTester tester) async {
final TextEditingController controller = TextEditingController(
text: 'Atwater Peel Sherbrooke Bonaventure',
);
Expand Down Expand Up @@ -645,10 +643,8 @@ void main() {
await tester.tapAt(textOffsetToPosition(tester, index));
await tester.pumpAndSettle();

// Paste is showing even though clipboard is empty.
// TODO(justinmc): Paste should not appears when the clipboard is empty.
// https://github.com/flutter/flutter/issues/74139
expect(find.text('Paste'), findsOneWidget);
// No Paste yet, because nothing has been copied.
expect(find.text('Paste'), findsNothing);
expect(find.text('Copy'), findsOneWidget);
expect(find.text('Cut'), findsOneWidget);
expect(find.text('Select all'), findsOneWidget);
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter/test/widgets/selectable_text_test.dart
Expand Up @@ -2487,7 +2487,7 @@ void main() {
));

semanticsOwner.performAction(inputFieldId, SemanticsAction.longPress);
await tester.pumpAndSettle();
await tester.pump();

expect(semantics, hasSemantics(
TestSemantics.root(
Expand Down
21 changes: 4 additions & 17 deletions packages/flutter/test/widgets/text_selection_test.dart
Expand Up @@ -2,7 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:flutter/foundation.dart';
import 'package:flutter/gestures.dart' show PointerDeviceKind, kSecondaryButton;
import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
Expand Down Expand Up @@ -766,19 +765,13 @@ void main() {
TestDefaultBinaryMessengerBinding.instance!.defaultBinaryMessenger.setMockMethodCallHandler(SystemChannels.platform, null);
});

// TODO(justinmc): See if `testWidgets` can be reverted to `test`.
testWidgets('Clipboard API failure is gracefully recovered from', (WidgetTester tester) async {
test('Clipboard API failure is gracefully recovered from', () async {
final ClipboardStatusNotifier notifier = ClipboardStatusNotifier();
expect(notifier.value, ClipboardStatus.unknown);

await expectLater(notifier.update(), completes);
expect(notifier.value, ClipboardStatus.unknown);
// TODO(justinmc): Currently on Android and iOS, ClipboardStatus.pasteable
// is always returned. Once both platforms properly use
// `hasStrings` to check whether the clipboard has any
// content, try to see if this override can be removed.
// https://github.com/flutter/flutter/issues/74139
}, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.linux }));
});
});

group('when Clipboard succeeds', () {
Expand All @@ -792,8 +785,7 @@ void main() {
TestDefaultBinaryMessengerBinding.instance!.defaultBinaryMessenger.setMockMethodCallHandler(SystemChannels.platform, null);
});

// TODO(justinmc): See if `testWidgets` can be reverted to `test`.
testWidgets('update sets value based on clipboard contents', (WidgetTester tester) async {
test('update sets value based on clipboard contents', () async {
final ClipboardStatusNotifier notifier = ClipboardStatusNotifier();
expect(notifier.value, ClipboardStatus.unknown);

Expand All @@ -808,12 +800,7 @@ void main() {
));
await expectLater(notifier.update(), completes);
expect(notifier.value, ClipboardStatus.pasteable);
// TODO(justinmc): Currently on Android and iOS, ClipboardStatus.pasteable
// is always returned. Once both platforms properly use
// `hasStrings` to check whether the clipboard has any
// content, try to see if this override can be removed.
// https://github.com/flutter/flutter/issues/74139
}, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.linux }));
});
});
});
}
Expand Down