diff --git a/assets/l10n/app_en.arb b/assets/l10n/app_en.arb index b4ad14149a..c83f4be852 100644 --- a/assets/l10n/app_en.arb +++ b/assets/l10n/app_en.arb @@ -661,6 +661,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." + }, "errorServerVersionUnsupportedMessage": "{url} is running Zulip Server {zulipVersion}, which is unsupported. The minimum supported version is Zulip Server {minSupportedZulipVersion}.", "@errorServerVersionUnsupportedMessage": { "description": "Error message in the dialog for when the Zulip Server version is unsupported.", diff --git a/lib/generated/l10n/zulip_localizations.dart b/lib/generated/l10n/zulip_localizations.dart index bdd5d60bdb..1fb29d9d10 100644 --- a/lib/generated/l10n/zulip_localizations.dart +++ b/lib/generated/l10n/zulip_localizations.dart @@ -1037,6 +1037,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 when the Zulip Server version is unsupported. /// /// 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 22e1f04fc4..370820a75e 100644 --- a/lib/generated/l10n/zulip_localizations_ar.dart +++ b/lib/generated/l10n/zulip_localizations_ar.dart @@ -537,6 +537,13 @@ class ZulipLocalizationsAr extends ZulipLocalizations { 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 errorServerVersionUnsupportedMessage( String url, diff --git a/lib/generated/l10n/zulip_localizations_de.dart b/lib/generated/l10n/zulip_localizations_de.dart index 74e5402693..cb4eb5447e 100644 --- a/lib/generated/l10n/zulip_localizations_de.dart +++ b/lib/generated/l10n/zulip_localizations_de.dart @@ -552,6 +552,13 @@ class ZulipLocalizationsDe extends ZulipLocalizations { String get topicValidationErrorMandatoryButEmpty => 'Themen sind in dieser Organisation erforderlich.'; + @override + String get errorContentNotInsertedTitle => 'Content not inserted'; + + @override + String get errorContentToInsertIsEmpty => + 'The file to be inserted is empty or cannot be accessed.'; + @override String errorServerVersionUnsupportedMessage( String url, diff --git a/lib/generated/l10n/zulip_localizations_en.dart b/lib/generated/l10n/zulip_localizations_en.dart index cad9183a94..05a0bb88c9 100644 --- a/lib/generated/l10n/zulip_localizations_en.dart +++ b/lib/generated/l10n/zulip_localizations_en.dart @@ -537,6 +537,13 @@ class ZulipLocalizationsEn extends ZulipLocalizations { 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 errorServerVersionUnsupportedMessage( String url, diff --git a/lib/generated/l10n/zulip_localizations_it.dart b/lib/generated/l10n/zulip_localizations_it.dart index 5f85cce756..aa157be8db 100644 --- a/lib/generated/l10n/zulip_localizations_it.dart +++ b/lib/generated/l10n/zulip_localizations_it.dart @@ -548,6 +548,13 @@ class ZulipLocalizationsIt extends ZulipLocalizations { String get topicValidationErrorMandatoryButEmpty => 'In questa organizzazione sono richiesti degli argomenti.'; + @override + String get errorContentNotInsertedTitle => 'Content not inserted'; + + @override + String get errorContentToInsertIsEmpty => + 'The file to be inserted is empty or cannot be accessed.'; + @override String errorServerVersionUnsupportedMessage( String url, diff --git a/lib/generated/l10n/zulip_localizations_ja.dart b/lib/generated/l10n/zulip_localizations_ja.dart index b2eb5fc7b0..0dc64e5d09 100644 --- a/lib/generated/l10n/zulip_localizations_ja.dart +++ b/lib/generated/l10n/zulip_localizations_ja.dart @@ -537,6 +537,13 @@ class ZulipLocalizationsJa extends ZulipLocalizations { 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 errorServerVersionUnsupportedMessage( String url, diff --git a/lib/generated/l10n/zulip_localizations_nb.dart b/lib/generated/l10n/zulip_localizations_nb.dart index 8744729a26..ac36747a5e 100644 --- a/lib/generated/l10n/zulip_localizations_nb.dart +++ b/lib/generated/l10n/zulip_localizations_nb.dart @@ -537,6 +537,13 @@ class ZulipLocalizationsNb extends ZulipLocalizations { 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 errorServerVersionUnsupportedMessage( String url, diff --git a/lib/generated/l10n/zulip_localizations_pl.dart b/lib/generated/l10n/zulip_localizations_pl.dart index 1471e2736e..a8bd4bbe91 100644 --- a/lib/generated/l10n/zulip_localizations_pl.dart +++ b/lib/generated/l10n/zulip_localizations_pl.dart @@ -546,6 +546,13 @@ class ZulipLocalizationsPl extends ZulipLocalizations { 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 errorServerVersionUnsupportedMessage( String url, diff --git a/lib/generated/l10n/zulip_localizations_ru.dart b/lib/generated/l10n/zulip_localizations_ru.dart index 20c6a54bb6..41b9b6b3a6 100644 --- a/lib/generated/l10n/zulip_localizations_ru.dart +++ b/lib/generated/l10n/zulip_localizations_ru.dart @@ -548,6 +548,13 @@ class ZulipLocalizationsRu extends ZulipLocalizations { 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 errorServerVersionUnsupportedMessage( String url, diff --git a/lib/generated/l10n/zulip_localizations_sk.dart b/lib/generated/l10n/zulip_localizations_sk.dart index 24797fad32..a7b270e212 100644 --- a/lib/generated/l10n/zulip_localizations_sk.dart +++ b/lib/generated/l10n/zulip_localizations_sk.dart @@ -537,6 +537,13 @@ class ZulipLocalizationsSk extends ZulipLocalizations { 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 errorServerVersionUnsupportedMessage( String url, diff --git a/lib/generated/l10n/zulip_localizations_sl.dart b/lib/generated/l10n/zulip_localizations_sl.dart index c298af9351..84f8f36b02 100644 --- a/lib/generated/l10n/zulip_localizations_sl.dart +++ b/lib/generated/l10n/zulip_localizations_sl.dart @@ -558,6 +558,13 @@ class ZulipLocalizationsSl extends ZulipLocalizations { String get topicValidationErrorMandatoryButEmpty => 'Teme so v tej organizaciji obvezne.'; + @override + String get errorContentNotInsertedTitle => 'Content not inserted'; + + @override + String get errorContentToInsertIsEmpty => + 'The file to be inserted is empty or cannot be accessed.'; + @override String errorServerVersionUnsupportedMessage( String url, diff --git a/lib/generated/l10n/zulip_localizations_uk.dart b/lib/generated/l10n/zulip_localizations_uk.dart index 82e07f3634..efa899255b 100644 --- a/lib/generated/l10n/zulip_localizations_uk.dart +++ b/lib/generated/l10n/zulip_localizations_uk.dart @@ -549,6 +549,13 @@ class ZulipLocalizationsUk extends ZulipLocalizations { 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 errorServerVersionUnsupportedMessage( String url, diff --git a/lib/generated/l10n/zulip_localizations_zh.dart b/lib/generated/l10n/zulip_localizations_zh.dart index a5ab16e499..a00e49f267 100644 --- a/lib/generated/l10n/zulip_localizations_zh.dart +++ b/lib/generated/l10n/zulip_localizations_zh.dart @@ -537,6 +537,13 @@ class ZulipLocalizationsZh extends ZulipLocalizations { 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 errorServerVersionUnsupportedMessage( String url, diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index 6514631955..85f277ba6a 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -6,6 +6,7 @@ import 'package:flutter/material.dart'; import 'package:flutter/scheduler.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'; @@ -508,6 +509,40 @@ class _ContentInput extends StatelessWidget { final String? hintText; final bool enabled; + void _handleContentInserted(KeyboardInsertedContent content, + BuildContext context) 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: controller.content, + contentFocusNode: controller.contentFocusNode, + files: [file]); + } + static double maxHeight(BuildContext context) { final clampingTextScaler = MediaQuery.textScalerOf(context) .clamp(maxScaleFactor: 1.5); @@ -552,6 +587,8 @@ class _ContentInput extends StatelessWidget { enabled: enabled, controller: controller.content, focusNode: controller.contentFocusNode, + contentInsertionConfiguration: ContentInsertionConfiguration( + onContentInserted: (content) => _handleContentInserted(content, context)), // 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 d1f9c33484..f52cbc9afe 100644 --- a/test/widgets/compose_box_test.dart +++ b/test/widgets/compose_box_test.dart @@ -6,6 +6,7 @@ import 'package:checks/checks.dart'; import 'package:collection/collection.dart'; import 'package:crypto/crypto.dart'; import 'package:file_picker/file_picker.dart'; +import 'package:flutter/services.dart'; import 'package:flutter_checks/flutter_checks.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; @@ -1045,20 +1046,24 @@ void main() { .isA().color.isNotNull().isSameColorAs(expectedIconColor); } - group('attach from media library', () { - testWidgets('success', (tester) async { - TypingNotifier.debugEnable = false; - addTearDown(TypingNotifier.debugReset); + Future 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( @@ -1106,18 +1111,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( @@ -1168,6 +1162,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 insertContentFromKeyboard(WidgetTester tester, { + required List? 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', [ + -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() + ..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>>((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', () {