content: Handle blank text nodes after code blocks.

[Khader-1]

before	after

Fixes: #355

[chrisbobbe]

Thanks! Small comments below.

[Khader-1]

Thanks for the review @chrisbobbe! I believe this is ready now.

[gnprice]

Thanks! Comments below.

[gnprice]

Interesting! I wonder if this is the explanation for all the times we've seen this symptom. It hasn't felt super common in my experience — which would be consistent with that, because I think this four-space-indent syntax is not super commonly used in Zulip messages (because the triple-backticks syntax is usually more convenient).

[Khader-1]

I attempted to replicate the issue by copying the messages mentioned in the report. I can confirm that this occurs exclusively when utilizing the four-space-indent syntax. As my attempts to reproduce it using triple-backticks were unsuccessful.

Additionally, it is noteworthy that I couldn't reproduce the issue when typing on the mobile app. It was only after sending the message from the web client that the double line breaks appeared.

[gnprice]

Huh interesting. I'd be surprised if the difference between mobile and web has a direct effect on this — both clients are calling the same API to send a message, and if they send the same Markdown content I'd expect the server to turn that into the same HTML. Perhaps the two clients are somehow ending up sending different Markdown content?

If the server does for some reason end up producing different HTML for the same Markdown depending on the client, that'd be a surprising quirk it'd definitely be good to know about. Most likely it'd be a bug we'd want to fix in the server.

[Khader-1]

Upon further investigation, I've determined that this issue is associated with the 'trim' function utilized prior to dispatching the message in the mobile application:

// lib/widgets/compose_box.dart:69
@override
String _computeTextNormalized() {
  String trimmed = text.trim();
  return trimmed.isEmpty ? kNoTopicTopic : trimmed;
}

I'm uncertain whether this behavior should be altered to align with the web version or if it's acceptable to maintain its current state. I'd recommend creating a new issue to handle this. What are your thoughts, @chrisbobbe?

[chrisbobbe]

Thanks for the investigation, @Khader-1! It sounds like you've found that the web client doesn't trim leading and trailing whitespace from the raw message content sent in the send-message request. Is that right?

(I think you mean the same method of ComposeContentController:

  @override
  String _computeTextNormalized() {
    return text.trim();
  }

The one you quoted belongs to ComposeTopicController.)

I've always assumed it's a good idea for the client to trim extra whitespace from the start and end of raw message content. It just seems neater that way, to me. If it's true that trimming is preferable (and I might be wrong), I would wonder why the web client doesn't do so. 🙂 I wonder if there's some Markdown syntax that requires leading or trailing whitespace and is defeated when the whitespace is removed. That would be a good reason not to call .trim() as we do currently.

If you want, you could raise the question of why the web app doesn't do trimming, and whether clients should trim or not, on CZO. Perhaps in the #api design stream, because it's about the expectations of the send-message endpoint. But it seems like the answer doesn't need to block progress on the current issue.

[gnprice]

Thanks @Khader-1 for the revision! Just two small nits below. I'll be on vacation after today (and it's afternoon here now), so @chrisbobbe will take a look next, but I expect this will be an easy PR at this point for him to review and merge.

Also replied on your observations about different behavior between web and mobile — that'd be interesting to pin down further.

[gnprice]

Huh interesting. I'd be surprised if the difference between mobile and web has a direct effect on this — both clients are calling the same API to send a message, and if they send the same Markdown content I'd expect the server to turn that into the same HTML. Perhaps the two clients are somehow ending up sending different Markdown content?

If the server does for some reason end up producing different HTML for the same Markdown depending on the client, that'd be a surprising quirk it'd definitely be good to know about. Most likely it'd be a bug we'd want to fix in the server.

[Khader-1]

Appreciate the feedback, @gnprice. I believe it's good to go now, @chrisbobbe; would love to hear your insights on the trim behavior.

[chrisbobbe]

Thanks, @Khader-1, looks great!! Just one nit below, which I'll fix and then merge. See also my reply about the trim behavior: #604 (comment)

[chrisbobbe]

nit: leave out topic to reduce visual clutter, as we did in #603 and as specified in #617, so:

    // https://chat.zulip.org/#narrow/stream/7-test-here/near/1774823