diff --git a/assets/l10n/app_en.arb b/assets/l10n/app_en.arb index ec0de4dd8f..cff093fb82 100644 --- a/assets/l10n/app_en.arb +++ b/assets/l10n/app_en.arb @@ -522,6 +522,14 @@ "@topicValidationErrorMandatoryButEmpty": { "description": "Topic validation error when topic is required but was empty." }, + "errorContentNotInsertedTitle": "Content not inserted", + "@errorContentNotInsertedTitle": { + "description": "Title for error dialog when an attempt to insert rich content failed." + }, + "errorContentToInsertIsEmpty": "The file to be inserted is empty or cannot be accessed.", + "@errorContentToInsertIsEmpty": { + "description": "Error message when the rich content to be inserted is empty or cannot be accessed." + }, "errorInvalidApiKeyMessage": "Your account at {url} could not be authenticated. Please try logging in again or use another account.", "@errorInvalidApiKeyMessage": { "description": "Error message in the dialog for invalid API key.", diff --git a/lib/generated/l10n/zulip_localizations.dart b/lib/generated/l10n/zulip_localizations.dart index 1b339ce823..4235e8ef3c 100644 --- a/lib/generated/l10n/zulip_localizations.dart +++ b/lib/generated/l10n/zulip_localizations.dart @@ -801,6 +801,18 @@ abstract class ZulipLocalizations { /// **'Topics are required in this organization.'** String get topicValidationErrorMandatoryButEmpty; + /// Title for error dialog when an attempt to insert rich content failed. + /// + /// In en, this message translates to: + /// **'Content not inserted'** + String get errorContentNotInsertedTitle; + + /// Error message when the rich content to be inserted is empty or cannot be accessed. + /// + /// In en, this message translates to: + /// **'The file to be inserted is empty or cannot be accessed.'** + String get errorContentToInsertIsEmpty; + /// Error message in the dialog for invalid API key. /// /// In en, this message translates to: diff --git a/lib/generated/l10n/zulip_localizations_ar.dart b/lib/generated/l10n/zulip_localizations_ar.dart index 890bf68596..872593c39f 100644 --- a/lib/generated/l10n/zulip_localizations_ar.dart +++ b/lib/generated/l10n/zulip_localizations_ar.dart @@ -404,6 +404,12 @@ class ZulipLocalizationsAr extends ZulipLocalizations { @override String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.'; + @override + String get errorContentNotInsertedTitle => 'Content not inserted'; + + @override + String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.'; + @override String errorInvalidApiKeyMessage(String url) { return 'Your account at $url could not be authenticated. Please try logging in again or use another account.'; diff --git a/lib/generated/l10n/zulip_localizations_en.dart b/lib/generated/l10n/zulip_localizations_en.dart index 72b7e9ad69..5c376001b4 100644 --- a/lib/generated/l10n/zulip_localizations_en.dart +++ b/lib/generated/l10n/zulip_localizations_en.dart @@ -404,6 +404,12 @@ class ZulipLocalizationsEn extends ZulipLocalizations { @override String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.'; + @override + String get errorContentNotInsertedTitle => 'Content not inserted'; + + @override + String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.'; + @override String errorInvalidApiKeyMessage(String url) { return 'Your account at $url could not be authenticated. Please try logging in again or use another account.'; diff --git a/lib/generated/l10n/zulip_localizations_ja.dart b/lib/generated/l10n/zulip_localizations_ja.dart index 5bfacf60d9..f6ecd38522 100644 --- a/lib/generated/l10n/zulip_localizations_ja.dart +++ b/lib/generated/l10n/zulip_localizations_ja.dart @@ -404,6 +404,12 @@ class ZulipLocalizationsJa extends ZulipLocalizations { @override String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.'; + @override + String get errorContentNotInsertedTitle => 'Content not inserted'; + + @override + String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.'; + @override String errorInvalidApiKeyMessage(String url) { return 'Your account at $url could not be authenticated. Please try logging in again or use another account.'; diff --git a/lib/generated/l10n/zulip_localizations_nb.dart b/lib/generated/l10n/zulip_localizations_nb.dart index 9698d16c96..fe9a8778f9 100644 --- a/lib/generated/l10n/zulip_localizations_nb.dart +++ b/lib/generated/l10n/zulip_localizations_nb.dart @@ -404,6 +404,12 @@ class ZulipLocalizationsNb extends ZulipLocalizations { @override String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.'; + @override + String get errorContentNotInsertedTitle => 'Content not inserted'; + + @override + String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.'; + @override String errorInvalidApiKeyMessage(String url) { return 'Your account at $url could not be authenticated. Please try logging in again or use another account.'; diff --git a/lib/generated/l10n/zulip_localizations_pl.dart b/lib/generated/l10n/zulip_localizations_pl.dart index 01bee756da..e74f1dc61e 100644 --- a/lib/generated/l10n/zulip_localizations_pl.dart +++ b/lib/generated/l10n/zulip_localizations_pl.dart @@ -404,6 +404,12 @@ class ZulipLocalizationsPl extends ZulipLocalizations { @override String get topicValidationErrorMandatoryButEmpty => 'Wątki są wymagane przez tę organizację.'; + @override + String get errorContentNotInsertedTitle => 'Content not inserted'; + + @override + String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.'; + @override String errorInvalidApiKeyMessage(String url) { return 'Your account at $url could not be authenticated. Please try logging in again or use another account.'; diff --git a/lib/generated/l10n/zulip_localizations_ru.dart b/lib/generated/l10n/zulip_localizations_ru.dart index 2c01dddb61..4dac764905 100644 --- a/lib/generated/l10n/zulip_localizations_ru.dart +++ b/lib/generated/l10n/zulip_localizations_ru.dart @@ -404,6 +404,12 @@ class ZulipLocalizationsRu extends ZulipLocalizations { @override String get topicValidationErrorMandatoryButEmpty => 'Темы обязательны в этой организации.'; + @override + String get errorContentNotInsertedTitle => 'Content not inserted'; + + @override + String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.'; + @override String errorInvalidApiKeyMessage(String url) { return 'Your account at $url could not be authenticated. Please try logging in again or use another account.'; diff --git a/lib/generated/l10n/zulip_localizations_sk.dart b/lib/generated/l10n/zulip_localizations_sk.dart index a69cfc2d8a..43dc8d2b48 100644 --- a/lib/generated/l10n/zulip_localizations_sk.dart +++ b/lib/generated/l10n/zulip_localizations_sk.dart @@ -404,6 +404,12 @@ class ZulipLocalizationsSk extends ZulipLocalizations { @override String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.'; + @override + String get errorContentNotInsertedTitle => 'Content not inserted'; + + @override + String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.'; + @override String errorInvalidApiKeyMessage(String url) { return 'Your account at $url could not be authenticated. Please try logging in again or use another account.'; diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index 902d57892d..6e4e29bcf8 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -4,6 +4,7 @@ import 'package:app_settings/app_settings.dart'; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; import 'package:mime/mime.dart'; +import 'package:path/path.dart' as path; import '../api/exception.dart'; import '../api/model/model.dart'; @@ -465,6 +466,39 @@ class _ContentInputState extends State<_ContentInput> with WidgetsBindingObserve } } + void _handleContentInserted(KeyboardInsertedContent content) async { + if (content.data == null || content.data!.isEmpty) { + // As of writing, the engine implementation never leaves `content.data` as + // `null`, but ideally it should be when the data cannot be read for + // errors. + // + // When `content.data` is empty, the data is not literally empty — this + // can also happen when the data can't be read from the input stream + // provided by the Android SDK because of an IO exception. + // + // See Flutter engine implementation that prepares this data: + // https://github.com/flutter/flutter/blob/0ffc4ce00/engine/src/flutter/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java#L497-L548 + // TODO(upstream): improve the API for this + final zulipLocalizations = ZulipLocalizations.of(context); + showErrorDialog(context: context, + title: zulipLocalizations.errorContentNotInsertedTitle, + message: zulipLocalizations.errorContentToInsertIsEmpty); + return; + } + + final file = _File( + content: Stream.fromIterable([content.data!]), + length: content.data!.length, + filename: path.basename(content.uri), + mimeType: content.mimeType); + + await _uploadFiles( + context: context, + contentController: widget.controller.content, + contentFocusNode: widget.controller.contentFocusNode, + files: [file]); + } + static double maxHeight(BuildContext context) { final clampingTextScaler = MediaQuery.textScalerOf(context) .clamp(maxScaleFactor: 1.5); @@ -508,6 +542,8 @@ class _ContentInputState extends State<_ContentInput> with WidgetsBindingObserve child: TextField( controller: widget.controller.content, focusNode: widget.controller.contentFocusNode, + contentInsertionConfiguration: ContentInsertionConfiguration( + onContentInserted: _handleContentInserted), // Let the content show through the `contentPadding` so that // our [InsetShadowBox] can fade it smoothly there. clipBehavior: Clip.none, diff --git a/test/widgets/compose_box_test.dart b/test/widgets/compose_box_test.dart index 52d2d1c851..bee5336fd6 100644 --- a/test/widgets/compose_box_test.dart +++ b/test/widgets/compose_box_test.dart @@ -4,6 +4,7 @@ import 'dart:io'; import 'package:checks/checks.dart'; import 'package:file_picker/file_picker.dart'; +import 'package:flutter/services.dart'; import 'package:flutter_checks/flutter_checks.dart'; import 'package:http/http.dart' as http; import 'package:flutter/material.dart'; @@ -715,20 +716,24 @@ void main() { .isA<Icon>().color.isNotNull().isSameColorAs(expectedIconColor); } - group('attach from media library', () { - testWidgets('success', (tester) async { - TypingNotifier.debugEnable = false; - addTearDown(TypingNotifier.debugReset); + Future<void> prepare(WidgetTester tester) async { + TypingNotifier.debugEnable = false; + addTearDown(TypingNotifier.debugReset); - final channel = eg.stream(); - final narrow = ChannelNarrow(channel.streamId); - await prepareComposeBox(tester, narrow: narrow, streams: [channel]); + final channel = eg.stream(); + final narrow = ChannelNarrow(channel.streamId); + await prepareComposeBox(tester, narrow: narrow, streams: [channel]); - // (When we check that the send button looks disabled, it should be because - // the file is uploading, not a pre-existing reason.) - await enterTopic(tester, narrow: narrow, topic: 'some topic'); - controller!.content.value = const TextEditingValue(text: 'see image: '); - await tester.pump(); + // (When we check that the send button looks disabled, it should be because + // the file is uploading, not a pre-existing reason.) + await enterTopic(tester, narrow: narrow, topic: 'some topic'); + await enterContent(tester, 'see image: '); + await tester.pump(); + } + + group('attach from media library', () { + testWidgets('success', (tester) async { + await prepare(tester); checkAppearsLoading(tester, false); testBinding.pickFilesResult = FilePickerResult([PlatformFile( @@ -776,18 +781,7 @@ void main() { group('attach from camera', () { testWidgets('success', (tester) async { - TypingNotifier.debugEnable = false; - addTearDown(TypingNotifier.debugReset); - - final channel = eg.stream(); - final narrow = ChannelNarrow(channel.streamId); - await prepareComposeBox(tester, narrow: narrow, streams: [channel]); - - // (When we check that the send button looks disabled, it should be because - // the file is uploading, not a pre-existing reason.) - await enterTopic(tester, narrow: narrow, topic: 'some topic'); - controller!.content.value = const TextEditingValue(text: 'see image: '); - await tester.pump(); + await prepare(tester); checkAppearsLoading(tester, false); testBinding.pickImageResult = XFile.fromData( @@ -838,6 +832,106 @@ void main() { // target platform the test is simulating. // TODO(upstream): unskip after fix to https://github.com/flutter/flutter/issues/161073 skip: Platform.isWindows); + + group('attach from keyboard', () { + // This is adapted from: + // https://github.com/flutter/flutter/blob/0ffc4ce00/packages/flutter/test/widgets/editable_text_test.dart#L724-L740 + Future<void> insertContentFromKeyboard(WidgetTester tester, { + required List<int>? data, + required String attachedFileUrl, + required String mimeType, + }) async { + await tester.showKeyboard(contentInputFinder); + // This invokes [EditableText.performAction] on the content [TextField], + // which did not expose an API for testing. + // TODO(upstream): support a better API for testing this + await tester.binding.defaultBinaryMessenger.handlePlatformMessage( + SystemChannels.textInput.name, + SystemChannels.textInput.codec.encodeMethodCall( + MethodCall('TextInputClient.performAction', <dynamic>[ + -1, + 'TextInputAction.commitContent', + // This fakes data originally provided by the Flutter engine: + // https://github.com/flutter/flutter/blob/0ffc4ce00/engine/src/flutter/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java#L497-L548 + { + "mimeType": mimeType, + "data": data, + "uri": attachedFileUrl, + }, + ])), + (ByteData? data) {}); + } + + testWidgets('success', (tester) async { + const fileContent = [1, 0, 1, 0, 0]; + await prepare(tester); + const uploadUrl = '/user_uploads/1/4e/m2A3MSqFnWRLUf9SaPzQ0Up_/test.gif'; + connection.prepare(json: UploadFileResult(uri: uploadUrl).toJson()); + await insertContentFromKeyboard(tester, + data: fileContent, + attachedFileUrl: + 'content://com.zulip.android.zulipboard.provider' + '/root/com.zulip.android.zulipboard/candidate_temp/test.gif', + mimeType: 'image/gif'); + + await tester.pump(); + check(controller!.content.text) + .equals('see image: [Uploading test.gif…]()\n\n'); + // (the request is checked more thoroughly in API tests) + check(connection.lastRequest!).isA<http.MultipartRequest>() + ..method.equals('POST') + ..files.single.which((it) => it + ..field.equals('file') + ..length.equals(fileContent.length) + ..filename.equals('test.gif') + ..contentType.asString.equals('image/gif') + ..has<Future<List<int>>>((f) => f.finalize().toBytes(), 'contents') + .completes((it) => it.deepEquals(fileContent)) + ); + checkAppearsLoading(tester, true); + + await tester.pump(Duration.zero); + check(controller!.content.text) + .equals('see image: [test.gif]($uploadUrl)\n\n'); + checkAppearsLoading(tester, false); + }); + + testWidgets('data is null', (tester) async { + await prepare(tester); + await insertContentFromKeyboard(tester, + data: null, + attachedFileUrl: + 'content://com.zulip.android.zulipboard.provider' + '/root/com.zulip.android.zulipboard/candidate_temp/test.gif', + mimeType: 'image/jpeg'); + + await tester.pump(); + check(controller!.content.text).equals('see image: '); + check(connection.takeRequests()).isEmpty(); + checkErrorDialog(tester, + expectedTitle: 'Content not inserted', + expectedMessage: 'The file to be inserted is empty or cannot be accessed.'); + checkAppearsLoading(tester, false); + }); + + testWidgets('data is empty', (tester) async { + await prepare(tester); + await insertContentFromKeyboard(tester, + data: [], + attachedFileUrl: + 'content://com.zulip.android.zulipboard.provider' + '/root/com.zulip.android.zulipboard/candidate_temp/test.gif', + mimeType: 'image/jpeg'); + + await tester.pump(); + check(controller!.content.text).equals('see image: '); + check(connection.takeRequests()).isEmpty(); + checkErrorDialog(tester, + expectedTitle: 'Content not inserted', + expectedMessage: 'The file to be inserted is empty or cannot be accessed.'); + checkAppearsLoading(tester, false); + }); + }); }); group('error banner', () {