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
4 changes: 2 additions & 2 deletions docs/reviews/messaging/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
Four stacked draft changes implement the held-message menu, local selection and
deletion, encrypted reactions and sender-requested deletion, and composer/gesture
polish. Merge in that order. This is the requested messaging scope, not a claim of
complete Signal feature parity. Quoted replies, message editing and attachments
are not part of these changes.
complete Signal feature parity. The subsequent quoted-reply and editing drafts
extend this stack; attachments remain outside this work.

The implementation extends Fireplace's Flutter widgets and local store. Signal's
[iOS](https://github.com/signalapp/Signal-iOS) and
Expand Down
70 changes: 70 additions & 0 deletions docs/reviews/replies-and-edits/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
# Quoted replies and sent-message editing

These two drafts extend the messaging stack from PRs 45–48. The reply draft adds
encrypted quote snapshots, a cancellable composer preview, Reply menu and
accessibility actions, swipe-right composition, and sent quotes. The editing
draft adds double-tap/menu editing, an independent edit draft, checkmark save,
cancel restoring the ordinary draft, an Edited label and version history. It
also makes quote taps scroll within the full timeline, including lazy rows.

The interaction reference is Signal iOS. Fireplace keeps its colors, protocol,
privacy rules and attachment-free scope. This report does not claim exhaustive
pixel or feature parity. Physical iOS/Android gesture and native-font acceptance
remains a human review step.

Read [decision 0022](../../decisions/0022-quoted-replies-and-message-edits.md)
for v3 content payloads, bounded snapshots/history, author validation, concurrent
linked-device ordering, deletion dominance, compatibility and failure recovery.
Ordinary text remains v1. No new Firestore fields, rules, keys or packages.

## Renders

Actual Flutter screenshots, fictional fixtures, UTC and the installed Windows
emoji font as a test-only fallback. No font is bundled.

| State | Screenshot |
| --- | --- |
| Reply composer | [Dark](reply-compose-dark.png) |
| Sent quote | [Light](reply-sent-light.png) |
| Tap to original | [Dark](reply-original-dark.png) |
| Enlarged reply composer | [Light](reply-large-light.png) |
| Editing | [Dark](edit-compose-dark.png) |
| Edited bubble | [Light](edit-saved-light.png) |
| Edit history | [Light](edit-history-light.png) |
| Enlarged edit composer | [Dark](edit-large-dark.png) |

## Validation

Pinned Flutter 3.47.6. Each draft was checked in a fresh local clone.
The reply draft passed format/analyze, 58 targeted tests, nine menu/accessibility
edge tests and eight reply renders. The additional accessibility inventory now
includes Reply.

| Check | Result |
| --- | --- |
| Final format and `flutter analyze` | Clean |
| Full non-timing suite, including chat simulations | 834 passed, 1 existing skip |
| Timing suite, run alone with concurrency 1 | 16 passed |
| Render catalog | 168 passed; every PNG hash matched the fresh checkout |
| Layer check and checker tests | Passed; 10 checker tests |
| Android build refusal checks | All 8 passed; no signing or app build |
| Separate Firestore emulator rules suite | 74 passed, unchanged rules |

The rules run used a demo project and unused local emulator ports in the separate
backend checkout. No production endpoint or deployment was used. Tests use
fictional data. Existing long-file warnings concern the original crypto source
and the chat screen at approximately 500 lines.

Reproduce the catalog with `TZ=UTC`,
`RENDER_EMOJI_FONT=C:/Windows/Fonts/seguiemj.ttf`, and
`flutter test scripts/render/render_screens_test.dart`.
The final catalog hashes are in [render-sha256.csv](render-sha256.csv).

The earlier messaging PRs were merged during validation. The new branches were
rebased onto that main revision; Git tree hashes confirmed no source changes.

## AI assistance

OpenAI Codex wrote the implementation, design note, tests and review material.
Human review, physical-device acceptance and CLA confirmation remain pending.
Nothing was merged or deployed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
169 changes: 169 additions & 0 deletions docs/reviews/replies-and-edits/render-sha256.csv

Large diffs are not rendered by default.

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
30 changes: 29 additions & 1 deletion lib/src/db/local_messages.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import 'package:fireplace/src/db/message_edit.dart';
import 'package:fireplace/src/db/message_quote.dart';

import 'dart:async';
Expand Down Expand Up @@ -29,9 +30,13 @@ class LocalMessage {
this.remotelyDeleted = false,
this.mutation,
this.quote,
this.originalBody,
this.editClock = 0,
Map<int, MessageEdit> edits = const {},
Map<String, MessageReaction> reactions = const {},
Map<String, int> reactionClocks = const {},
}) : reactions = Map.unmodifiable(reactions),
}) : edits = Map.unmodifiable(edits),
reactions = Map.unmodifiable(reactions),
reactionClocks = Map.unmodifiable(reactionClocks);

final String id;
Expand All @@ -46,13 +51,19 @@ class LocalMessage {
final bool remotelyDeleted;
final MessageMutation? mutation;
final MessageQuote? quote;
final String? originalBody;
final int editClock;
final Map<int, MessageEdit> edits;
final Map<String, MessageReaction> reactions;
final Map<String, int> reactionClocks;
bool get visible => !locallyDeleted && mutation == null;

LocalMessage copyWith({
String? body,
MessageQuote? quote,
String? originalBody,
int? editClock,
Map<int, MessageEdit>? edits,
bool? outgoing,
MessageStatus? status,
bool? remotelyDeleted,
Expand All @@ -71,6 +82,9 @@ class LocalMessage {
remotelyDeleted: remotelyDeleted ?? this.remotelyDeleted,
mutation: mutation,
quote: quote ?? this.quote,
originalBody: originalBody ?? this.originalBody,
editClock: editClock ?? this.editClock,
edits: edits ?? this.edits,
reactions: reactions ?? this.reactions,
reactionClocks: reactionClocks ?? this.reactionClocks,
);
Expand Down Expand Up @@ -107,6 +121,12 @@ class LocalMessage {
if (remotelyDeleted) 'remotelyDeleted': true,
if (mutation != null) 'mutation': mutation!.toJson(),
if (quote != null) 'quote': quote!.toJson(),
if (originalBody != null) 'originalBody': originalBody,
if (editClock != 0) 'editClock': editClock,
if (edits.isNotEmpty)
'edits': {
for (final e in edits.entries) e.key.toString(): e.value.toJson(),
},
if (reactions.isNotEmpty)
'reactions': {for (final e in reactions.entries) e.key: e.value.toJson()},
if (reactionClocks.isNotEmpty) 'reactionClocks': reactionClocks,
Expand All @@ -120,6 +140,14 @@ class LocalMessage {
outgoing: j['outgoing'],
sentAt: DateTime.fromMillisecondsSinceEpoch(j['sentAt']),
body: j['body'],
originalBody: j['originalBody'],
editClock: j['editClock'] ?? 0,
edits: {
for (final e in (j['edits'] as Map? ?? {}).entries)
int.parse(e.key): MessageEdit.fromJson(
Map<String, dynamic>.from(e.value),
),
},
quote: j['quote'] == null
? null
: MessageQuote.fromJson(Map<String, dynamic>.from(j['quote'])),
Expand Down
32 changes: 32 additions & 0 deletions lib/src/db/message_edit.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/// One winning revision slot. Ties from linked devices have a stable ordering.
class MessageEdit {
const MessageEdit({
required this.revision,
required this.body,
required this.deviceId,
required this.eventId,
required this.sentAt,
});
final int revision;
final String body, deviceId, eventId;
final DateTime sentAt;
bool newerThan(MessageEdit other) {
final device = deviceId.compareTo(other.deviceId);
return device != 0 ? device > 0 : eventId.compareTo(other.eventId) > 0;
}

Map<String, dynamic> toJson() => {
'revision': revision,
'body': body,
'deviceId': deviceId,
'eventId': eventId,
'sentAt': sentAt.millisecondsSinceEpoch,
};
factory MessageEdit.fromJson(Map<String, dynamic> j) => MessageEdit(
revision: j['revision'],
body: j['body'],
deviceId: j['deviceId'],
eventId: j['eventId'],
sentAt: DateTime.fromMillisecondsSinceEpoch(j['sentAt']),
);
}
27 changes: 25 additions & 2 deletions lib/src/db/message_mutation.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
enum MessageMutationKind { reaction, delete }
enum MessageMutationKind { reaction, delete, edit }

enum ReactionOperation { set, clear }

Expand All @@ -7,17 +7,22 @@ class MessageMutation {
required this.kind,
required this.targetId,
this.emoji = '',
this.body = '',
this.operation = ReactionOperation.set,
this.revision = 0,
});
final MessageMutationKind kind;
final String targetId, emoji;
final String targetId, emoji, body;
final ReactionOperation operation;
final int revision;

Map<String, dynamic> toJson() => {
'type': kind.name,
'targetId': targetId,
if (kind == MessageMutationKind.edit) ...{
'body': body,
'revision': revision,
},
if (kind == MessageMutationKind.reaction) ...{
'emoji': emoji,
'operation': operation.name,
Expand All @@ -35,6 +40,24 @@ class MessageMutation {
if (kind == MessageMutationKind.delete) {
return MessageMutation(kind: kind, targetId: target);
}
if (kind == MessageMutationKind.edit) {
final body = j['body'], revision = j['revision'];
if (body is! String ||
body.length > 32768 ||
body.runes.length > 16384 ||
body.trim().isEmpty ||
revision is! int ||
revision < 1 ||
revision > 10) {
throw const FormatException('Invalid edit');
}
return MessageMutation(
kind: kind,
targetId: target,
body: body,
revision: revision,
);
}
final emoji = j['emoji'], revision = j['revision'];
if (emoji is! String ||
!validReactionEmoji(emoji) ||
Expand Down
50 changes: 49 additions & 1 deletion lib/src/db/message_projection.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import 'package:fireplace/src/db/message_edit.dart';
import 'package:fireplace/src/db/local_messages.dart';
import 'package:fireplace/src/db/message_mutation.dart';

Expand All @@ -8,6 +9,8 @@ Map<String, LocalMessage> projectMessageRecords(
) {
final next = {...records};
final deleted = <String>{};
final edits = <String, Map<int, MessageEdit>>{};
final editClocks = <String, int>{};
final reactions = <String, Map<String, MessageReaction>>{};
final clocks = <String, Map<String, int>>{};
for (final event in records.values) {
Expand All @@ -23,6 +26,31 @@ Map<String, LocalMessage> projectMessageRecords(
next[event.id] = LocalMessage.tombstone(event.chatId, event.id);
continue;
}
if (change.kind == MessageMutationKind.edit) {
if (target.senderUid != event.senderUid ||
target.status != MessageStatus.ok ||
target.remotelyDeleted ||
change.revision < 1 ||
change.revision > 10) {
continue;
}
final clock = editClocks[target.id] ?? target.editClock;
if (change.revision > clock) editClocks[target.id] = change.revision;
if (event.status != MessageStatus.ok) continue;
final versions = edits.putIfAbsent(target.id, () => {...target.edits});
final revision = MessageEdit(
revision: change.revision,
body: change.body,
deviceId: event.senderDevice,
eventId: event.id,
sentAt: event.sentAt,
);
final old = versions[change.revision];
if (old == null || revision.newerThan(old)) {
versions[change.revision] = revision;
}
continue;
}
if (change.kind == MessageMutationKind.reaction) {
final byActor = clocks.putIfAbsent(
target.id,
Expand Down Expand Up @@ -58,6 +86,9 @@ Map<String, LocalMessage> projectMessageRecords(
if (target.remotelyDeleted || deleted.contains(target.id)) {
next[target.id] = target.copyWith(
body: '',
originalBody: '',
edits: const {},
editClock: 0,
remotelyDeleted: true,
reactions: const {},
reactionClocks: const {},
Expand All @@ -70,10 +101,27 @@ Map<String, LocalMessage> projectMessageRecords(
);
}
}
for (final entry in editClocks.entries) {
final target = next[entry.key]!;
if (!target.remotelyDeleted) {
next[entry.key] = target.copyWith(editClock: entry.value);
}
}
for (final entry in edits.entries) {
final target = next[entry.key]!;
if (target.remotelyDeleted) continue;
final last = entry.value.keys.reduce((a, b) => a > b ? a : b);
next[entry.key] = target.copyWith(
originalBody: target.originalBody ?? target.body,
edits: entry.value,
body: entry.value[last]!.body,
);
}
// A deleted target must not leave reaction content in hidden control records.
for (final event in records.values) {
final change = event.mutation;
if (change?.kind == MessageMutationKind.reaction &&
if ((change?.kind == MessageMutationKind.reaction ||
change?.kind == MessageMutationKind.edit) &&
next[change!.targetId]?.remotelyDeleted == true) {
next[event.id] = LocalMessage.tombstone(event.chatId, event.id);
}
Expand Down
10 changes: 10 additions & 0 deletions lib/src/model/chat/chat_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,16 @@ class ChatService {
),
);

Future<void> editMessage(String chatId, String targetId, String body) =>
_sender.sendControl(
chatId,
MessageMutation(
kind: MessageMutationKind.edit,
targetId: targetId,
body: body,
),
);

Future<void> react(String chatId, String targetId, String emoji) =>
_sender.sendControl(
chatId,
Expand Down
9 changes: 8 additions & 1 deletion lib/src/model/chat/message_compose_target.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ import 'package:flutter_riverpod/flutter_riverpod.dart';
import 'package:fireplace/src/app/providers.dart';
import 'package:fireplace/src/db/local_messages.dart';

enum MessageComposeAction { reply }
enum MessageComposeAction { reply, edit }

class MessageComposeTarget {
const MessageComposeTarget(this.action, this.message);
final MessageComposeAction action;
final LocalMessage message;
String? get replyId =>
action == MessageComposeAction.reply ? message.id : null;
}

class MessageCompose extends Notifier<MessageComposeTarget?> {
Expand All @@ -24,7 +26,12 @@ class MessageCompose extends Notifier<MessageComposeTarget?> {

void reply(LocalMessage message) =>
state = MessageComposeTarget(MessageComposeAction.reply, message);
void edit(LocalMessage message) =>
state = MessageComposeTarget(MessageComposeAction.edit, message);
void clear() => state = null;
void clearReply() {
if (state?.action == MessageComposeAction.reply) state = null;
}
}

final messageComposeProvider = NotifierProvider.autoDispose
Expand Down
Loading
Loading