From d84e4c04ee474be70fbe191b771e4e8068b38415 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 1 Aug 2023 17:20:05 -0700 Subject: [PATCH 1/3] content [nfc]: Have resolveUrl return a Uri object, not a String This pushes the `toString` out to callers, so they can pass the string form to whatever other code is asking for it. ...Actually, though, in all the cases where that other code has been asking for the string form, it could easily be changed to accept the Uri form. So we'll do that, coming up. --- lib/widgets/content.dart | 13 ++++++------- lib/widgets/message_list.dart | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 5f39808fdf..ed4344d55a 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -224,7 +224,7 @@ class MessageImage extends StatelessWidget { return GestureDetector( onTap: () { Navigator.of(context).push(getLightboxRoute( - context: context, message: message, src: resolvedSrc)); + context: context, message: message, src: resolvedSrc.toString())); }, child: Align( alignment: Alignment.centerLeft, @@ -241,9 +241,9 @@ class MessageImage extends StatelessWidget { color: const Color.fromRGBO(0, 0, 0, 0.03), child: LightboxHero( message: message, - src: resolvedSrc, + src: resolvedSrc.toString(), child: RealmContentNetworkImage( - resolvedSrc, + resolvedSrc.toString(), filterQuality: FilterQuality.medium)))))); } } @@ -648,7 +648,7 @@ class MessageImageEmoji extends StatelessWidget { // too low. top: -1.5, child: RealmContentNetworkImage( - resolvedSrc, + resolvedSrc.toString(), filterQuality: FilterQuality.medium, width: size, height: size, @@ -823,10 +823,9 @@ class RealmContentNetworkImage extends StatelessWidget { /// Resolve `url` to `account`'s realm, if relative // This may dissolve when we start passing around URLs as [Uri] objects instead // of strings. -String resolveUrl(String url, Account account) { +Uri resolveUrl(String url, Account account) { final realmUrl = account.realmUrl; - final resolved = realmUrl.resolve(url); // TODO handle if fails to parse - return resolved.toString(); + return realmUrl.resolve(url); // TODO handle if fails to parse } InlineSpan _errorUnimplemented(UnimplementedNode node) { diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 28331bbae7..09aeaeb410 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -483,7 +483,7 @@ class MessageWithSender extends StatelessWidget { final avatar = (avatarUrl == null) ? const SizedBox.shrink() : RealmContentNetworkImage( - avatarUrl, + avatarUrl.toString(), filterQuality: FilterQuality.medium, ); From d624f396809792f06ab4552136fc9b42a1e596da Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 1 Aug 2023 17:29:23 -0700 Subject: [PATCH 2/3] lightbox [nfc]: Store src as Uri object, not string The most interesting consequence of this is that _LightboxHeroTag now compares `src` using Uri's == operator override, instead of string equality on the result of Uri.toString(). Discussion: https://github.com/zulip/zulip-flutter/pull/247#pullrequestreview-1559993051 --- lib/widgets/content.dart | 4 ++-- lib/widgets/lightbox.dart | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index ed4344d55a..fceb8bd8a4 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -224,7 +224,7 @@ class MessageImage extends StatelessWidget { return GestureDetector( onTap: () { Navigator.of(context).push(getLightboxRoute( - context: context, message: message, src: resolvedSrc.toString())); + context: context, message: message, src: resolvedSrc)); }, child: Align( alignment: Alignment.centerLeft, @@ -241,7 +241,7 @@ class MessageImage extends StatelessWidget { color: const Color.fromRGBO(0, 0, 0, 0.03), child: LightboxHero( message: message, - src: resolvedSrc.toString(), + src: resolvedSrc, child: RealmContentNetworkImage( resolvedSrc.toString(), filterQuality: FilterQuality.medium)))))); diff --git a/lib/widgets/lightbox.dart b/lib/widgets/lightbox.dart index e68e15a64d..2bed58e9e9 100644 --- a/lib/widgets/lightbox.dart +++ b/lib/widgets/lightbox.dart @@ -17,7 +17,7 @@ class _LightboxHeroTag { _LightboxHeroTag({required this.messageId, required this.src}); final int messageId; - final String src; + final Uri src; @override bool operator ==(Object other) { @@ -40,7 +40,7 @@ class LightboxHero extends StatelessWidget { }); final Message message; - final String src; + final Uri src; final Widget child; @override @@ -66,7 +66,7 @@ class LightboxHero extends StatelessWidget { class _CopyLinkButton extends StatelessWidget { const _CopyLinkButton({required this.url}); - final String url; + final Uri url; @override Widget build(BuildContext context) { @@ -76,7 +76,7 @@ class _CopyLinkButton extends StatelessWidget { onPressed: () async { // TODO(i18n) copyWithPopup(context: context, successContent: const Text('Link copied'), - data: ClipboardData(text: url)); + data: ClipboardData(text: url.toString())); }); } } @@ -90,7 +90,7 @@ class _LightboxPage extends StatefulWidget { final Animation routeEntranceAnimation; final Message message; - final String src; + final Uri src; @override State<_LightboxPage> createState() => _LightboxPageState(); @@ -197,7 +197,7 @@ class _LightboxPageState extends State<_LightboxPage> { child: LightboxHero( message: widget.message, src: widget.src, - child: RealmContentNetworkImage(widget.src, filterQuality: FilterQuality.medium))))))), + child: RealmContentNetworkImage(widget.src.toString(), filterQuality: FilterQuality.medium))))))), bottomNavigationBar: bottomAppBar)); } } @@ -205,7 +205,7 @@ class _LightboxPageState extends State<_LightboxPage> { Route getLightboxRoute({ required BuildContext context, required Message message, - required String src + required Uri src, }) { return AccountPageRouteBuilder( context: context, From 135cec1002e2ad4d22f3e98f8050eeec918ab62a Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 1 Aug 2023 17:25:38 -0700 Subject: [PATCH 3/3] RealmContentNetworkImage [nfc]: Take a Uri object instead of a String --- lib/widgets/content.dart | 14 +++++--------- lib/widgets/lightbox.dart | 2 +- lib/widgets/message_list.dart | 5 +---- test/widgets/content_test.dart | 8 ++++---- 4 files changed, 11 insertions(+), 18 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index fceb8bd8a4..f8df933967 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -243,7 +243,7 @@ class MessageImage extends StatelessWidget { message: message, src: resolvedSrc, child: RealmContentNetworkImage( - resolvedSrc.toString(), + resolvedSrc, filterQuality: FilterQuality.medium)))))); } } @@ -648,7 +648,7 @@ class MessageImageEmoji extends StatelessWidget { // too low. top: -1.5, child: RealmContentNetworkImage( - resolvedSrc.toString(), + resolvedSrc, filterQuality: FilterQuality.medium, width: size, height: size, @@ -749,9 +749,7 @@ class RealmContentNetworkImage extends StatelessWidget { this.cacheHeight, }); - /// An absolute URL string for the image. - // TODO: Take a [Uri] object, not a String - final String src; + final Uri src; final double scale; final ImageFrameBuilder? frameBuilder; @@ -780,10 +778,8 @@ class RealmContentNetworkImage extends StatelessWidget { Widget build(BuildContext context) { final account = PerAccountStoreWidget.of(context).account; - final Uri parsedSrc = Uri.parse(src); - return Image.network( - parsedSrc.toString(), + src.toString(), scale: scale, frameBuilder: frameBuilder, @@ -806,7 +802,7 @@ class RealmContentNetworkImage extends StatelessWidget { isAntiAlias: isAntiAlias, // Only send the auth header to the server `auth` belongs to. - headers: parsedSrc.origin == account.realmUrl.origin + headers: src.origin == account.realmUrl.origin ? authHeader(email: account.email, apiKey: account.apiKey) : null, diff --git a/lib/widgets/lightbox.dart b/lib/widgets/lightbox.dart index 2bed58e9e9..829b7ab527 100644 --- a/lib/widgets/lightbox.dart +++ b/lib/widgets/lightbox.dart @@ -197,7 +197,7 @@ class _LightboxPageState extends State<_LightboxPage> { child: LightboxHero( message: widget.message, src: widget.src, - child: RealmContentNetworkImage(widget.src.toString(), filterQuality: FilterQuality.medium))))))), + child: RealmContentNetworkImage(widget.src, filterQuality: FilterQuality.medium))))))), bottomNavigationBar: bottomAppBar)); } } diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 09aeaeb410..e38bc382e8 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -482,10 +482,7 @@ class MessageWithSender extends StatelessWidget { : resolveUrl(author.avatarUrl!, store.account); final avatar = (avatarUrl == null) ? const SizedBox.shrink() - : RealmContentNetworkImage( - avatarUrl.toString(), - filterQuality: FilterQuality.medium, - ); + : RealmContentNetworkImage(avatarUrl, filterQuality: FilterQuality.medium); final time = _kMessageTimestampFormat .format(DateTime.fromMillisecondsSinceEpoch(1000 * message.timestamp)); diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index 3bf6021807..03c407ea4a 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -121,7 +121,7 @@ void main() { group('RealmContentNetworkImage', () { final authHeaders = authHeader(email: eg.selfAccount.email, apiKey: eg.selfAccount.apiKey); - Future actualAuthHeader(WidgetTester tester, String src) async { + Future actualAuthHeader(WidgetTester tester, Uri src) async { final globalStore = TestZulipBinding.instance.globalStore; addTearDown(TestZulipBinding.instance.reset); await globalStore.add(eg.selfAccount, eg.initialSnapshot()); @@ -144,20 +144,20 @@ void main() { } testWidgets('includes auth header if `src` on-realm', (tester) async { - check(await actualAuthHeader(tester, 'https://chat.example/image.png')) + check(await actualAuthHeader(tester, Uri.parse('https://chat.example/image.png'))) .isNotNull().equals(authHeaders['Authorization']!); debugNetworkImageHttpClientProvider = null; }); testWidgets('excludes auth header if `src` off-realm', (tester) async { - check(await actualAuthHeader(tester, 'https://other.example/image.png')) + check(await actualAuthHeader(tester, Uri.parse('https://other.example/image.png'))) .isNull(); debugNetworkImageHttpClientProvider = null; }); testWidgets('throws if no `PerAccountStoreWidget` ancestor', (WidgetTester tester) async { await tester.pumpWidget( - const RealmContentNetworkImage('https://zulip.invalid/path/to/image.png', filterQuality: FilterQuality.medium)); + RealmContentNetworkImage(Uri.parse('https://zulip.invalid/path/to/image.png'), filterQuality: FilterQuality.medium)); check(tester.takeException()).isA(); }); });