From eeff96e1dba1819a92396ee92a8d62b7952b6bd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= Date: Mon, 28 Apr 2025 13:31:48 +0200 Subject: [PATCH] room-history: Remove whitespaces at the beginning and end of HTML elements To have the same behavior as in browsers. --- .../message_row/text/inline_html.rs | 220 +++++++++++------- .../room_history/message_row/text/tests.rs | 4 +- .../view/content/room_history/title.rs | 7 +- src/utils/string/mod.rs | 14 +- 4 files changed, 147 insertions(+), 98 deletions(-) diff --git a/src/session/view/content/room_history/message_row/text/inline_html.rs b/src/session/view/content/room_history/message_row/text/inline_html.rs index 8fd2cf71..4044bd57 100644 --- a/src/session/view/content/room_history/message_row/text/inline_html.rs +++ b/src/session/view/content/room_history/message_row/text/inline_html.rs @@ -17,6 +17,7 @@ use crate::{ /// Helper type to construct a Pango-compatible string from inline HTML nodes. #[derive(Debug)] +#[allow(clippy::struct_excessive_bools)] pub(super) struct InlineHtmlBuilder<'a> { /// Whether this string should be on a single line. single_line: bool, @@ -28,6 +29,8 @@ pub(super) struct InlineHtmlBuilder<'a> { inner: String, /// Whether this string was truncated because at the first newline. truncated: bool, + /// Whether to account for `truncated` when appending children. + ignore_truncated: bool, } impl<'a> InlineHtmlBuilder<'a> { @@ -45,6 +48,7 @@ impl<'a> InlineHtmlBuilder<'a> { mentions: MentionsMode::default(), inner: String::new(), truncated: false, + ignore_truncated: false, } } @@ -117,117 +121,146 @@ impl<'a> InlineHtmlBuilder<'a> { } /// Append the given inline node by converting it to Pango markup. - fn append_node(&mut self, node: &NodeRef, should_linkify: bool) { + fn append_node(&mut self, node: &NodeRef, context: NodeContext) { match node.data() { NodeData::Element(data) => { let data = data.to_matrix(); - match data.element { - MatrixElement::Del | MatrixElement::S => { - self.append_tags_and_children("s", node.children(), should_linkify); - } - MatrixElement::A(anchor) => { - // First, check if it's a mention, if we detect mentions. - if let Some(uri) = &anchor.href { - if let MentionsMode::WithMentions { pills, room, .. } = - &mut self.mentions - { - if let Some(pill) = self.inner.maybe_append_mention(uri, room) { - pills.push(pill); - - return; - } - } + self.append_element_node(node, data.element, context.should_linkify); + } + NodeData::Text(text) => { + self.append_text_node(text.borrow().as_ref(), context); + } + data => { + debug!("Unexpected HTML node: {data:?}"); + } + } + } + + /// Append the given inline element node by converting it to Pango markup. + fn append_element_node( + &mut self, + node: &NodeRef, + element: MatrixElement, + should_linkify: bool, + ) { + match element { + MatrixElement::Del | MatrixElement::S => { + self.append_tags_and_children("s", node.children(), should_linkify); + } + MatrixElement::A(anchor) => { + // First, check if it's a mention, if we detect mentions. + if let Some(uri) = &anchor.href { + if let MentionsMode::WithMentions { pills, room, .. } = &mut self.mentions { + if let Some(pill) = self.inner.maybe_append_mention(uri, room) { + pills.push(pill); + + return; } + } + } - // It's not a mention, render the link, if it has a URI. - let mut has_opening_tag = false; + // It's not a mention, render the link, if it has a URI. + let mut has_opening_tag = false; - if let Some(uri) = &anchor.href { - has_opening_tag = self.append_link_opening_tag_from_anchor_uri(uri); - } + if let Some(uri) = &anchor.href { + has_opening_tag = self.append_link_opening_tag_from_anchor_uri(uri); + } - // Always render the children. - for node in node.children() { - // Don't try to linkify text if we render the element, it does not make - // sense to nest links. - self.append_node(&node, !has_opening_tag && should_linkify); - } + // Always render the children. + self.ignore_truncated = true; - if has_opening_tag { - self.inner.push_str(""); - } - } - MatrixElement::Sup => { - self.append_tags_and_children("sup", node.children(), should_linkify); - } - MatrixElement::Sub => { - self.append_tags_and_children("sub", node.children(), should_linkify); - } - MatrixElement::B | MatrixElement::Strong => { - self.append_tags_and_children("b", node.children(), should_linkify); - } - MatrixElement::I | MatrixElement::Em => { - self.append_tags_and_children("i", node.children(), should_linkify); - } - MatrixElement::U => { - self.append_tags_and_children("u", node.children(), should_linkify); - } - MatrixElement::Code(_) => { - // Don't try to linkify text, it does not make sense to detect links inside - // code. - self.append_tags_and_children("tt", node.children(), false); - } - MatrixElement::Br => { - if self.single_line { - self.truncated = true; - } else { - self.inner.push('\n'); - } - } - MatrixElement::Span(span) => { - self.append_span(&span, node.children(), should_linkify); - } - element => { - debug!("Unexpected HTML inline element: {element:?}"); - self.append_nodes(node.children(), should_linkify); - } + // Don't try to linkify text if we render the element, it does not make + // sense to nest links. + let should_linkify = !has_opening_tag && should_linkify; + + self.append_nodes(node.children(), should_linkify); + + self.ignore_truncated = false; + + if has_opening_tag { + self.inner.push_str(""); } } - NodeData::Text(text) => { - let text = text.borrow().collapse_whitespaces(); - - if should_linkify { - if let MentionsMode::WithMentions { - pills, - room, - detect_at_room, - } = &mut self.mentions - { - Linkifier::new(&mut self.inner) - .detect_mentions(room, pills, *detect_at_room) - .linkify(&text); - } else { - Linkifier::new(&mut self.inner).linkify(&text); - } + MatrixElement::Sup => { + self.append_tags_and_children("sup", node.children(), should_linkify); + } + MatrixElement::Sub => { + self.append_tags_and_children("sub", node.children(), should_linkify); + } + MatrixElement::B | MatrixElement::Strong => { + self.append_tags_and_children("b", node.children(), should_linkify); + } + MatrixElement::I | MatrixElement::Em => { + self.append_tags_and_children("i", node.children(), should_linkify); + } + MatrixElement::U => { + self.append_tags_and_children("u", node.children(), should_linkify); + } + MatrixElement::Code(_) => { + // Don't try to linkify text, it does not make sense to detect links inside + // code. + self.append_tags_and_children("tt", node.children(), false); + } + MatrixElement::Br => { + if self.single_line { + self.truncated = true; } else { - self.inner.push_str(&text.escape_markup()); + self.inner.push('\n'); } } - data => { - debug!("Unexpected HTML node: {data:?}"); + MatrixElement::Span(span) => { + self.append_span(&span, node.children(), should_linkify); + } + element => { + debug!("Unexpected HTML inline element: {element:?}"); + self.append_nodes(node.children(), should_linkify); + } + } + } + + /// Append the given text node content. + fn append_text_node(&mut self, text: &str, context: NodeContext) { + // Remove spaces at the beginning and end of an HTML element. + let text = text.collapse_whitespaces(context.is_first_child, context.is_last_child); + + if context.should_linkify { + if let MentionsMode::WithMentions { + pills, + room, + detect_at_room, + } = &mut self.mentions + { + Linkifier::new(&mut self.inner) + .detect_mentions(room, pills, *detect_at_room) + .linkify(&text); + } else { + Linkifier::new(&mut self.inner).linkify(&text); } + } else { + self.inner.push_str(&text.escape_markup()); } } /// Append the given inline nodes, converted to Pango markup. fn append_nodes(&mut self, nodes: impl IntoIterator, should_linkify: bool) { - for node in nodes { - self.append_node(&node, should_linkify); + let mut is_first_child = true; + let mut nodes_iter = nodes.into_iter().peekable(); - if self.truncated { + while let Some(node) = nodes_iter.next() { + let child_context = NodeContext { + should_linkify, + is_first_child, + is_last_child: nodes_iter.peek().is_none(), + }; + + self.append_node(&node, child_context); + + if self.truncated && !self.ignore_truncated { // Stop as soon as the string is truncated. break; } + + is_first_child = false; } } @@ -355,3 +388,14 @@ enum MentionsMode<'a> { detect_at_room: bool, }, } + +/// Context for an HTML node. +#[derive(Debug, Clone, Copy)] +struct NodeContext { + /// Whether we should try to search for links in the text of the node. + should_linkify: bool, + /// Whether this is the first child node of an element. + is_first_child: bool, + /// Whether this is the last child node of an element. + is_last_child: bool, +} diff --git a/src/session/view/content/room_history/message_row/text/tests.rs b/src/session/view/content/room_history/message_row/text/tests.rs index 598db37f..4258b2e7 100644 --- a/src/session/view/content/room_history/message_row/text/tests.rs +++ b/src/session/view/content/room_history/message_row/text/tests.rs @@ -55,10 +55,10 @@ fn collapse_whitespace() { assert_eq!(s, "Hello you! You are my friend."); assert!(pills.is_none()); - let html = Html::parse("Hello \nyou! \n\nYou are \n my \nfriend ."); + let html = Html::parse(" Hello \nyou! \n\nYou are \n my \nfriend . "); let (s, pills) = InlineHtmlBuilder::new(false, false).build_with_nodes(html.children()); - assert_eq!(s, "Hello you! You are my friend ."); + assert_eq!(s, "Hello you! You are my friend."); assert!(pills.is_none()); } diff --git a/src/session/view/content/room_history/title.rs b/src/session/view/content/room_history/title.rs index f0a34c04..ce1e390d 100644 --- a/src/session/view/content/room_history/title.rs +++ b/src/session/view/content/room_history/title.rs @@ -119,11 +119,8 @@ mod imp { let subtitle = room .topic() .map(|s| { - // Remove newlines and empty lines. - let mut s = s.collapse_whitespaces(); - // Remove trailing spaces. - s.truncate_end_whitespaces(); - s + // Remove newlines and empty lines and trailing whitespaces. + s.collapse_whitespaces(false, true) }) .filter(|s| !s.is_empty()); diff --git a/src/utils/string/mod.rs b/src/utils/string/mod.rs index 91b9b56a..74a5149a 100644 --- a/src/utils/string/mod.rs +++ b/src/utils/string/mod.rs @@ -30,7 +30,7 @@ pub(crate) trait StrExt { fn escape_markup(&self) -> String; /// Collapse contiguous whitespaces in this string into a single space. - fn collapse_whitespaces(&self) -> String; + fn collapse_whitespaces(&self, trim_start: bool, trim_end: bool) -> String; } impl StrExt for T @@ -41,8 +41,16 @@ where markup_escape_text(self.as_ref()).into() } - fn collapse_whitespaces(&self) -> String { - let str = self.as_ref(); + fn collapse_whitespaces(&self, trim_start: bool, trim_end: bool) -> String { + let mut str = self.as_ref(); + + if trim_start { + str = str.trim_start(); + } + if trim_end { + str = str.trim_end(); + } + let mut new_string = String::with_capacity(str.len()); let mut prev_is_space = false;