From 478f27a07c2b883f9b6722909f25f6091c4f8ff6 Mon Sep 17 00:00:00 2001 From: sepehr-safari Date: Fri, 25 Sep 2026 10:45:56 +0300 Subject: [PATCH] fix: a paste with CR line breaks draws as the paragraphs it is The toolkit lays text out breaking only at LF, while on macOS the host draws each laid-out row through AppKit, which also breaks at a lone CR, vertical tab, form feed, NEL, U+2028 and U+2029. Pasting paragraphs separated by any of those drew the words after each break one row lower, on top of the next row. The text was intact; only the drawing was wrong. Those separators now become LF before an edit reaches the draft, the reply box or the profile's about field, and before a draft restored from disk is shown. CRLF is left alone: its CR ends a row, so it always drew correctly. When the stored text ends up different from what the editor inserted, because a separator was replaced or the paste was cut to fit, the editor takes the stored text and moves its caret to the end, and the toolkit has no way to say otherwise. The model's caret now goes there too, so typing lands where the caret is drawn; before, a paste cut to fit in the middle of a note left the two carets apart. A cut that splits a CRLF is made plain as well. The comment that blamed the field's width for this is corrected. --- src/main.zig | 137 +++++++++++++++++++++++++++++++++++++----- src/tests.zig | 161 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 284 insertions(+), 14 deletions(-) diff --git a/src/main.zig b/src/main.zig index 606f9f1..0d51eb4 100644 --- a/src/main.zig +++ b/src/main.zig @@ -3575,6 +3575,8 @@ const engagement_widen_ms: i64 = 5_000; // moved the wall from 512 to 4096. What fixes it is saying so, which is what // `draft_dropped` is for. const compose_capacity = 4096; +/// The profile editor's "about" box. +const profile_about_capacity = 280; const refresh_timer_key: u64 = 1; const refresh_interval_ms: u64 = 1_000; // Wanted-profile fetching runs on its own cadence, decoupled from the view @@ -3851,8 +3853,9 @@ const compose_header_height: f32 = 38; /// STATED, not inherited, and that is the whole point. A text element measures /// at its natural width whatever its ancestors say, so a field that takes its /// width from a `grow` parent wraps for LAYOUT at one width and measures for -/// PAINT at another. Two wrappings of the same paragraph then land on the same -/// rows, which is what made a pasted note look shredded. +/// PAINT at another, and two wrappings of the same paragraph land on the same +/// rows. That was once blamed for a pasted note drawing scrambled (#165); it was +/// not the cause, which turned out to be CR line breaks (see `plainLineBreaks`). const compose_editor_width: f32 = compose_sheet_width - 14 * 2 - avatar_size - 12; pub const compose_editor_width_for_test = compose_editor_width; const compose_editor_height: f32 = 150; @@ -5736,8 +5739,8 @@ fn scanHelperSign(model: *Model) void { // The composer holds one draft. A reader who has started typing again // keeps what they are typing; the restored one would overwrite it. if (restorable and model.draft_empty()) { - model.draft_buffer.set(c); - saveDraft(c); + setPlain(compose_capacity, &model.draft_buffer, c); + saveDraft(model.draft()); // Said out loud, because the notice this used to rely on cannot be // read: its string lives in `Model.identity()`, which is listed in // `view_unbound` and rendered by nothing. So a reader saw "Posted", @@ -10516,7 +10519,7 @@ pub const Model = struct { profile_lud16_long: bool = false, profile_nip05_long: bool = false, profile_name_buffer: canvas.TextBuffer(64) = .{}, - profile_about_buffer: canvas.TextBuffer(280) = .{}, + profile_about_buffer: canvas.TextBuffer(profile_about_capacity) = .{}, profile_picture_buffer: canvas.TextBuffer(200) = .{}, profile_website_buffer: canvas.TextBuffer(200) = .{}, profile_banner_buffer: canvas.TextBuffer(200) = .{}, @@ -29856,7 +29859,7 @@ pub fn boot(model: *Model, fx: *Effects) void { // composer where it was left. var draft_buf: [note_content_cap]u8 = undefined; const stashed = loadDraft(&draft_buf); - if (stashed.len > 0) model.draft_buffer = @TypeOf(model.draft_buffer).init(stashed); + if (stashed.len > 0) setPlain(compose_capacity, &model.draft_buffer, stashed); // What was owed when the app last closed. Read before the first frame, so a // note written offline yesterday is visible as owed rather than lost, and // offered again as soon as a relay answers. Whose queue that is comes from @@ -29898,6 +29901,116 @@ pub fn boot(model: *Model, fx: *Effects) void { }); } +/// What `plainLineBreaks` wrote, how long the whole input comes to once its +/// breaks are plain (including whatever did not fit in `out`), and whether it +/// had to change anything. +pub const PlainLineBreaks = struct { text: []const u8, full_len: usize, changed: bool }; + +/// Every line break that macOS draws as one and the toolkit does not lay out as +/// one becomes a plain LF: CR on its own, vertical tab, form feed, NEL (U+0085), +/// and the Unicode line and paragraph separators (U+2028, U+2029). Writes what +/// fits into `out`, never cutting a UTF-8 sequence, and counts the rest. +/// +/// The toolkit breaks a line only at LF, so it lays "end.\rNext" out as one +/// word on one row. On macOS the host then draws each of those rows through +/// AppKit, which DOES break there, so the words after the break land a row +/// lower, on top of the next row. That is what made a pasted note look +/// scrambled (#165), reproduced with each of these separators by pasting into +/// the composer. +/// +/// CRLF is left as it is. The layout breaks at its LF, so its CR is the last +/// byte of a row and nothing is drawn after it: it has always drawn correctly, +/// and changing it would cost the reader the caret and undo (see +/// `applyPlainEdit`) for nothing. +pub fn plainLineBreaks(in: []const u8, out: []u8) PlainLineBreaks { + var written: usize = 0; + var full: usize = 0; + var changed = false; + var i: usize = 0; + while (i < in.len) { + var take: usize = 1; + var plain = false; + switch (in[i]) { + '\r' => plain = !(i + 1 < in.len and in[i + 1] == '\n'), + 0x0B, 0x0C => plain = true, + 0xC2 => if (i + 1 < in.len and in[i + 1] == 0x85) { + plain = true; + take = 2; + }, + 0xE2 => if (i + 2 < in.len and in[i + 1] == 0x80 and (in[i + 2] == 0xA8 or in[i + 2] == 0xA9)) { + plain = true; + take = 3; + }, + else => {}, + } + // Anything else is copied with the rest of its UTF-8 sequence, so a + // full `out` never ends halfway through a character. + if (!plain) take = @min(std.unicode.utf8ByteSequenceLength(in[i]) catch 1, in.len - i); + const len: usize = if (plain) 1 else take; + // Written only while everything before it was: the text is always a + // prefix of the whole. + if (written == full and written + len <= out.len) { + if (plain) out[written] = '\n' else @memcpy(out[written..][0..len], in[i..][0..len]); + written += len; + } + changed = changed or plain; + full += len; + i += take; + } + return .{ .text = out[0..written], .full_len = full, .changed = changed }; +} + +/// Applies an edit to a multi-line text buffer with its line breaks made plain +/// first (see `plainLineBreaks`). Returns what an insert asked to add, counted +/// after that, so a caller can tell how much a clamp refused. +/// +/// The editor keeps its own copy of the text, and it holds far more than this +/// buffer does. Whenever this buffer ends up with different text from what the +/// editor inserted, because a separator was replaced or because the paste was +/// cut to fit, the editor takes this text and puts ITS caret at the end: the +/// toolkit gives an app no way to say where the caret should be. So this +/// buffer's caret goes to the end too, or the next key would land where the +/// reader cannot see it. A cut can also split a CRLF and leave a lone CR at the +/// cut, so the whole text is made plain again then. The editor's undo history +/// for the box starts again from there, which is the price of both. +/// +/// An insert refused outright changes nothing here, and the editor keeps its +/// own copy, so nothing is moved. +fn applyPlainEdit(comptime capacity: usize, buffer: *canvas.TextBuffer(capacity), edit: canvas.TextInputEvent) usize { + switch (edit) { + .insert_text => |inserted| { + var scratch: [capacity]u8 = undefined; + const plain = plainLineBreaks(inserted, &scratch); + const before_len = buffer.len; + const before_selection = buffer.selection; + buffer.apply(.{ .insert_text = plain.text }); + // Anything past `scratch` is more than the whole buffer holds, so + // it is a clamp even when the buffer took all of `plain.text`. + const clamped = buffer.truncated or plain.full_len > plain.text.len; + buffer.truncated = clamped; + const refused = clamped and buffer.len == before_len and + std.meta.eql(buffer.selection, before_selection); + if (!refused and (plain.changed or clamped)) { + var whole: [capacity]u8 = undefined; + buffer.set(plainLineBreaks(buffer.text(), &whole).text); + } + return plain.full_len; + }, + else => { + buffer.apply(edit); + return 0; + }, + } +} + +/// Puts text into a buffer with its line breaks made plain, for text that +/// arrives other than by typing: a draft restored from disk, which an older +/// Plaza may have saved with a CR in it. +fn setPlain(comptime capacity: usize, buffer: *canvas.TextBuffer(capacity), text: []const u8) void { + var scratch: [capacity]u8 = undefined; + buffer.set(plainLineBreaks(text, &scratch).text); +} + pub fn update(model: *Model, msg: Msg, fx: *Effects) void { switch (msg) { .tick => |t| { @@ -30129,12 +30242,8 @@ pub fn update(model: *Model, msg: Msg, fx: *Effects) void { .draft_edit => |edit| { // What this edit meant to add, before it is clamped. Only an insert // can overflow; every other edit is rejected whole. - const wanted: usize = switch (edit) { - .insert_text => |t| t.len, - else => 0, - }; const before = model.draft_buffer.len; - model.draft_buffer.apply(edit); + const wanted = applyPlainEdit(compose_capacity, &model.draft_buffer, edit); // The buffer's own words for this flag are "loud seam for paste: // check after applying a clipboard insert", and nothing here ever // did. A paste past the cap lost the overflow in silence: the @@ -30272,7 +30381,7 @@ pub fn update(model: *Model, msg: Msg, fx: *Effects) void { .open_profile_edit => openProfileEdit(model), .close_profile_edit => model.editing_profile = false, .profile_name_edit => |edit| model.profile_name_buffer.apply(edit), - .profile_about_edit => |edit| model.profile_about_buffer.apply(edit), + .profile_about_edit => |edit| _ = applyPlainEdit(profile_about_capacity, &model.profile_about_buffer, edit), .profile_picture_edit => |edit| model.profile_picture_buffer.apply(edit), .profile_website_edit => |edit| model.profile_website_buffer.apply(edit), .profile_banner_edit => |edit| model.profile_banner_buffer.apply(edit), @@ -30842,7 +30951,7 @@ pub fn update(model: *Model, msg: Msg, fx: *Effects) void { closeThread(model); }, .go_home => goHome(model), - .reply_edit => |edit| model.reply_buffer.apply(edit), + .reply_edit => |edit| _ = applyPlainEdit(compose_capacity, &model.reply_buffer, edit), .reply_submit => { // The one verb that was gated nowhere: a guest could type a reply and // press send, and `publishReply` would reach for a signer that does @@ -35695,7 +35804,7 @@ fn scanPendingRemote(model: *Model, fx_for_seal: *Effects) void { } if (restore) |c| { - if (model.draft_empty()) model.draft_buffer.set(c); + if (model.draft_empty()) setPlain(compose_capacity, &model.draft_buffer, c); gpa.free(c); } if (sign_failed) g_remote_sign_notice.store(true, .release); diff --git a/src/tests.zig b/src/tests.zig index 081f3d4..b929d1d 100644 --- a/src/tests.zig +++ b/src/tests.zig @@ -22749,6 +22749,167 @@ test "a paste that does not fit says so instead of vanishing" { try testing.expectEqual(@as(usize, 0), model.draft_dropped); } +test "every line break macOS draws and the layout does not becomes a plain LF" { + // The toolkit breaks a line only at LF, and on macOS the host draws each + // row through AppKit, which also breaks at these. A paste separated by any + // of them drew the words after the break a row lower, on top of the next + // row (#165). Reproduced live with CR, U+2028 and vertical tab. + const Case = struct { in: []const u8, want: []const u8, changed: bool }; + const cases = [_]Case{ + .{ .in = "one\rtwo", .want = "one\ntwo", .changed = true }, + .{ .in = "one\r\rtwo", .want = "one\n\ntwo", .changed = true }, + .{ .in = "one\x0btwo\x0cthree", .want = "one\ntwo\nthree", .changed = true }, + .{ .in = "one\u{0085}two", .want = "one\ntwo", .changed = true }, + .{ .in = "one\u{2028}two\u{2029}three", .want = "one\ntwo\nthree", .changed = true }, + .{ .in = "trailing\r", .want = "trailing\n", .changed = true }, + .{ .in = "one\r\r\ntwo", .want = "one\n\r\ntwo", .changed = true }, + // Left alone. CRLF draws correctly, since its CR ends a row, and a + // change would cost the caret and undo for nothing. + .{ .in = "one\r\ntwo", .want = "one\r\ntwo", .changed = false }, + .{ .in = "one\r\n\r\ntwo", .want = "one\r\n\r\ntwo", .changed = false }, + .{ .in = "one\n\ntwo", .want = "one\n\ntwo", .changed = false }, + // Characters that share a leading byte with a separator. + .{ .in = "caf\u{e9} \u{2026} \u{2022} \u{1F600}", .want = "caf\u{e9} \u{2026} \u{2022} \u{1F600}", .changed = false }, + // Truncated sequences at the very end are copied, not misread. + .{ .in = "a\xe2\x80", .want = "a\xe2\x80", .changed = false }, + .{ .in = "a\xc2", .want = "a\xc2", .changed = false }, + .{ .in = "", .want = "", .changed = false }, + }; + for (cases) |c| { + var out: [64]u8 = undefined; + const got = main.plainLineBreaks(c.in, &out); + testing.expectEqualStrings(c.want, got.text) catch |err| { + std.debug.print("for input {any}\n", .{c.in}); + return err; + }; + try testing.expectEqual(c.want.len, got.full_len); + try testing.expectEqual(c.changed, got.changed); + } +} + +test "a full buffer stops at a character boundary and still counts the rest" { + // "ab" then a three-byte character: with room for four bytes, the + // character does not fit whole, so it is left out rather than cut, and + // "cd", which would fit, is not written after the gap. + var out: [4]u8 = undefined; + const got = main.plainLineBreaks("ab\u{2026}cd", &out); + try testing.expectEqualStrings("ab", got.text); + try testing.expectEqual(@as(usize, 7), got.full_len); +} + +test "a paste with CR line breaks lands in every multi-line box as LF" { + var fx: main.EffectsForTest = undefined; + var model = main.initialModel(); + model.stage = .ready; + model.composing = true; + + const pasted = "Alpha one.\rBravo two.\r\rCharlie three.\u{2028}Delta four."; + const plain = "Alpha one.\nBravo two.\n\nCharlie three.\nDelta four."; + + main.update(&model, .{ .draft_edit = .{ .insert_text = pasted } }, &fx); + try testing.expectEqualStrings(plain, model.draft()); + try testing.expectEqual(@as(usize, 0), model.draft_dropped); + + main.update(&model, .{ .reply_edit = .{ .insert_text = pasted } }, &fx); + try testing.expectEqualStrings(plain, model.reply_buffer.text()); + + main.update(&model, .{ .profile_about_edit = .{ .insert_text = pasted } }, &fx); + try testing.expectEqualStrings(plain, model.profile_about_buffer.text()); +} + +test "a paste that had to change leaves the caret where the editor puts it" { + // The editor takes the model's text when the two differ, and then puts + // its caret at the end, with no way for the app to say otherwise. The + // model's caret has to be there too, or the next key goes in somewhere + // the reader cannot see: found live, typing after a mid-text paste. + var fx: main.EffectsForTest = undefined; + var model = main.initialModel(); + model.stage = .ready; + model.composing = true; + main.update(&model, .{ .draft_edit = .{ .insert_text = "Hello world" } }, &fx); + main.update(&model, .{ .draft_edit = .{ .set_selection = canvas.TextSelection.collapsed(5) } }, &fx); + + main.update(&model, .{ .draft_edit = .{ .insert_text = "A\rB" } }, &fx); + try testing.expectEqualStrings("HelloA\nB world", model.draft()); + try testing.expectEqual(canvas.TextSelection.collapsed(model.draft().len), model.draft_buffer.selection); + main.update(&model, .{ .draft_edit = .{ .insert_text = "x" } }, &fx); + try testing.expectEqualStrings("HelloA\nB worldx", model.draft()); + + // A paste that needed no change is an ordinary edit, caret and all. + main.update(&model, .{ .draft_edit = .{ .set_selection = canvas.TextSelection.collapsed(5) } }, &fx); + main.update(&model, .{ .draft_edit = .{ .insert_text = "C\r\nD" } }, &fx); + try testing.expectEqualStrings("HelloC\r\nDA\nB worldx", model.draft()); + try testing.expectEqual(canvas.TextSelection.collapsed(9), model.draft_buffer.selection); +} + +test "a paste cut to fit keeps the caret where the editor puts it, and no lone CR" { + // The editor holds the whole paste, the draft only what fits, so the + // editor takes the draft's text and moves its caret to the end. A cut can + // also land between a CR and its LF, which would leave exactly the lone CR + // that #165 is about. + var arena_state = std.heap.ArenaAllocator.init(testing.allocator); + defer arena_state.deinit(); + const cap = main.compose_capacity_for_test; + const start = "Hello world\nSecond line"; + const room = cap - start.len; + + // The cut lands between CR and LF. + var fx: main.EffectsForTest = undefined; + var model = main.initialModel(); + model.stage = .ready; + model.composing = true; + main.update(&model, .{ .draft_edit = .{ .insert_text = start } }, &fx); + main.update(&model, .{ .draft_edit = .{ .set_selection = canvas.TextSelection.collapsed(5) } }, &fx); + const split = try arena_state.allocator().alloc(u8, room + 5); + @memset(split, 'x'); + split[room - 1] = '\r'; + split[room] = '\n'; + main.update(&model, .{ .draft_edit = .{ .insert_text = split } }, &fx); + try testing.expectEqual(cap, model.draft().len); + if (std.mem.indexOfScalar(u8, model.draft(), '\r') != null) return error.ALoneCrSurvivedTheCut; + try testing.expectEqual(canvas.TextSelection.collapsed(cap), model.draft_buffer.selection); + + // A plain paste cut to fit, in the middle of the text. + var model2 = main.initialModel(); + model2.stage = .ready; + model2.composing = true; + main.update(&model2, .{ .draft_edit = .{ .insert_text = start } }, &fx); + main.update(&model2, .{ .draft_edit = .{ .set_selection = canvas.TextSelection.collapsed(5) } }, &fx); + const long = try arena_state.allocator().alloc(u8, room + 100); + @memset(long, 'y'); + main.update(&model2, .{ .draft_edit = .{ .insert_text = long } }, &fx); + try testing.expectEqual(canvas.TextSelection.collapsed(cap), model2.draft_buffer.selection); + + // Refused outright, into a full draft: nothing here changes, and the + // caret stays where it was. + main.update(&model2, .{ .draft_edit = .{ .set_selection = canvas.TextSelection.collapsed(5) } }, &fx); + const full = try arena_state.allocator().dupe(u8, model2.draft()); + main.update(&model2, .{ .draft_edit = .{ .insert_text = "p\rq" } }, &fx); + try testing.expectEqualStrings(full, model2.draft()); + try testing.expectEqual(canvas.TextSelection.collapsed(5), model2.draft_buffer.selection); +} + +test "a paste that does not fit is counted after its line breaks are made plain" { + // Past the cap, the overflow reported is measured in what the draft would + // have held. + var arena_state = std.heap.ArenaAllocator.init(testing.allocator); + defer arena_state.deinit(); + var fx: main.EffectsForTest = undefined; + var model = main.initialModel(); + model.stage = .ready; + model.composing = true; + + const cap = main.compose_capacity_for_test; + // cap + 1 bytes of U+2028, three bytes each on the way in and one once + // plain: cap + 1 plain bytes asked for, cap of them fit. + const big = try arena_state.allocator().alloc(u8, (cap + 1) * 3); + for (0..cap + 1) |i| @memcpy(big[i * 3 ..][0..3], "\u{2028}"); + main.update(&model, .{ .draft_edit = .{ .insert_text = big } }, &fx); + try testing.expectEqual(cap, model.draft().len); + try testing.expect(std.mem.indexOfScalar(u8, model.draft(), 0xE2) == null); + try testing.expectEqual(@as(usize, 1), model.draft_dropped); +} + test "a test identity signs the way the app does, through the keyholder" { // The suite's ~400 "be somebody" tests used to hold a secret key in this // process and sign inline. Plaza does not do that any more, so neither do