Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/stream_chat_flutter/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

🐞 Fixed

- Fixed link preview enrichment failing for uppercase URL schemes (e.g. `HTTPS://`) by normalizing the scheme before enriching.
- Fixed last-message preview flicker during channel-state reloads.
- Fixed shadowed messages not hidden in channel list items.
- Fixed `StreamMessageListView` firing `markThreadRead` on a reply-less parent, which produced a guaranteed 404 every time the thread view was opened before the first reply.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1293,20 +1293,23 @@ class DefaultStreamMessageComposerState extends State<DefaultStreamMessageCompos
if (_lastSearchedContainsUrlText == value) return;
_lastSearchedContainsUrlText = value;

final matchedUrls = _urlRegex.allMatches(value).where((it) {
final _parsedMatch = Uri.tryParse(it.group(0) ?? '')?.withScheme;
if (_parsedMatch == null) return false;

return widget.props.ogPreviewFilter.call(_parsedMatch, value);
}).toList();
// Find the first url to preview, normalizing the scheme so links like
// `HTTPS://` enrich the same as `https://`.
String? firstMatchedUrl;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Question: Wouldn't it be possible to end up without an enriched URL, it the /og endpoint doesn't return in time before sending the message, or if we send the message before this method fires (350ms debounce).
I am curious if maybe the bug report was for that reason 🤔 Because I can see some messages getting enriched even with HTTPS:// prefix

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, it's a possibility. But when i tried reproducing it in the sample app i wasn't able to get any og for a link with uppercase HTTPS://. One way to solve it is to use url_enrichment flag on channel but it will still fail as the backend extracts the url from text and the text will still be with Uppercase letters. So this also needs to be fixed in the backend code in order to work correctly.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see, I was afraid that we would still see reports of this issue after the fix. But nevertheless, I see no reason to block this PR. But we should probably bring this up with the BE as well.

for (final match in _urlRegex.allMatches(value)) {
final url = Uri.tryParse(match.group(0) ?? '')?.withScheme;
if (url == null) continue;
if (!widget.props.ogPreviewFilter.call(url, value)) continue;

firstMatchedUrl = url.toString();
break;
}

// Reset the og attachment if the text doesn't contain any url
if (matchedUrls.isEmpty || !channel.canSendLinks) {
if (firstMatchedUrl == null || !channel.canSendLinks) {
return _effectiveController.clearOGAttachment();
}

final firstMatchedUrl = matchedUrls.first.group(0)!;

// If the parsed url matches the ogAttachment url, don't do anything
if (_effectiveController.ogAttachment?.titleLink == firstMatchedUrl) {
return;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
// ignore_for_file: lines_longer_than_80_chars

import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:mocktail/mocktail.dart';
import 'package:record/record.dart';
import 'package:stream_chat_flutter/stream_chat_flutter.dart';

import '../fakes.dart';
import '../mocks.dart';

void main() {
group('MessageComposer URL enrichment', () {
final originalRecordPlatform = RecordPlatform.instance;
setUp(() => RecordPlatform.instance = FakeRecordPlatform());
tearDown(() => RecordPlatform.instance = originalRecordPlatform);

late MockClient client;
late MockClientState clientState;
late MockChannel channel;
late MockChannelState channelState;

setUp(() {
registerFallbackValue(Message());

client = MockClient();
clientState = MockClientState();
channel = MockChannel(
ownCapabilities: const [
ChannelCapability.sendMessage,
ChannelCapability.sendLinks,
],
);
channelState = MockChannelState();

when(() => client.state).thenReturn(clientState);
when(() => clientState.currentUser).thenReturn(OwnUser(id: 'user-id'));
when(() => clientState.currentUserStream).thenAnswer(
(_) => Stream.value(OwnUser(id: 'user-id')),
);

when(() => channel.state).thenReturn(channelState);
when(() => channel.client).thenReturn(client);
when(channel.getRemainingCooldown).thenReturn(0);

when(() => client.enrichUrl(any())).thenAnswer(
(invocation) async => OGAttachmentResponse()..ogScrapeUrl = invocation.positionalArguments.first as String,
);
});

Future<Object?> enrichUrlFrom(WidgetTester tester, String text) async {
// Enrichment runs behind a real-clock debounce, so drive the flow with
// real timers via runAsync.
await tester.runAsync(() async {
await tester.pumpWidget(
MaterialApp(
home: StreamChat(
client: client,
connectivityStream: Stream.value([ConnectivityResult.mobile]),
child: StreamChannel(
channel: channel,
child: Scaffold(body: StreamMessageComposer()),
),
),
),
);
await tester.pumpAndSettle();

await tester.enterText(find.byType(TextField), text);
await Future<void>.delayed(const Duration(milliseconds: 500));
await tester.pumpAndSettle();
});

return verify(() => client.enrichUrl(captureAny())).captured.single;
}

// The scheme (and host) are normalized to lowercase before enriching so a
// backend that only handles lowercase schemes receives a consistent url.
// Case-sensitive path and query parts must be preserved as-is.
final cases = <String, (String, String)>{
'uppercase HTTPS scheme': ('HTTPS://example.com', 'https://example.com'),
'uppercase HTTP scheme': ('HTTP://example.com', 'http://example.com'),
'mixed-case scheme': ('HtTpS://example.com', 'https://example.com'),
'mixed-case host': ('HTTPS://Example.COM', 'https://example.com'),
'preserves case-sensitive path and query': (
'HTTPS://example.com/Path?Q=AbC',
'https://example.com/Path?Q=AbC',
),
'uppercase scheme with www and path': (
'HTTPS://www.example.com/foo',
'https://www.example.com/foo',
),
'url embedded in surrounding text': (
'look at HTTPS://example.com now',
'https://example.com',
),
'lowercase https scheme unchanged': (
'https://example.com',
'https://example.com',
),
};

for (final entry in cases.entries) {
final (input, expected) = entry.value;
testWidgets('enriches ${entry.key}', (tester) async {
expect(await enrichUrlFrom(tester, input), expected);
});
}
});
}
Loading