From df6b8323f9467261a4d0a40ecf848a5df05a4091 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 13 Jun 2023 14:49:00 +0100 Subject: [PATCH 01/56] add new mention node types --- crates/wysiwyg/src/dom/nodes/mention_node.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/wysiwyg/src/dom/nodes/mention_node.rs b/crates/wysiwyg/src/dom/nodes/mention_node.rs index 7f4c9324c..f8cfd01fa 100644 --- a/crates/wysiwyg/src/dom/nodes/mention_node.rs +++ b/crates/wysiwyg/src/dom/nodes/mention_node.rs @@ -11,6 +11,7 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. +use matrix_mentions::Mention; use crate::composer_model::example_format::SelectionWriter; use crate::dom::dom_handle::DomHandle; @@ -37,6 +38,8 @@ pub enum MentionNodeKind where S: UnicodeString, { + Room { mention: Mention }, + User { mention: Mention }, MatrixUrl { display_text: S, url: S }, AtRoom, } From 93c97c8f6eb2c19a797d8e023ec3b4f353d9a897 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 13 Jun 2023 14:51:51 +0100 Subject: [PATCH 02/56] add TODOs to clear match errors --- crates/wysiwyg/src/dom/nodes/mention_node.rs | 24 ++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/crates/wysiwyg/src/dom/nodes/mention_node.rs b/crates/wysiwyg/src/dom/nodes/mention_node.rs index f8cfd01fa..7af7d8503 100644 --- a/crates/wysiwyg/src/dom/nodes/mention_node.rs +++ b/crates/wysiwyg/src/dom/nodes/mention_node.rs @@ -38,8 +38,8 @@ pub enum MentionNodeKind where S: UnicodeString, { - Room { mention: Mention }, - User { mention: Mention }, + Room { mention: Mention }, + User { mention: Mention }, MatrixUrl { display_text: S, url: S }, AtRoom, } @@ -81,6 +81,8 @@ where MentionNodeKind::MatrixUrl { display_text, .. } => { display_text.clone() } + MentionNodeKind::User { mention } => mention.display_text().clone(), + MentionNodeKind::Room { mention } => mention.display_text().clone(), MentionNodeKind::AtRoom => S::from("@room"), } } @@ -131,6 +133,12 @@ impl MentionNode { let cur_pos = formatter.len(); match self.kind() { + MentionNodeKind::User { mention } => { + // TODO do something + } + MentionNodeKind::Room { mention } => { + // TODO do something + } MentionNodeKind::MatrixUrl { display_text, url } => { // if formatting as a message, only include the href attribute let attributes = if as_message { @@ -199,6 +207,12 @@ where description.push("\""); match self.kind() { + MentionNodeKind::User { mention } => { + // TODO do something + } + MentionNodeKind::Room { mention } => { + // TODO do something + } MentionNodeKind::MatrixUrl { url, .. } => { description.push(", "); description.push(url.clone()); @@ -229,6 +243,12 @@ where // There are two different functions to allow for fact one will use mxId later on match self.kind() { + User => { + // TODO do something + } + Room => { + // TODO do something + } MatrixUrl { .. } => { fmt_user_or_room_mention(self, buffer)?; } From 1e2b95f5fc3810fc9afe642dfe9ec728cb265d52 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 13 Jun 2023 17:39:43 +0100 Subject: [PATCH 03/56] bit more type manipulation --- crates/wysiwyg/src/dom/nodes/mention_node.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/crates/wysiwyg/src/dom/nodes/mention_node.rs b/crates/wysiwyg/src/dom/nodes/mention_node.rs index 7af7d8503..1089fb06e 100644 --- a/crates/wysiwyg/src/dom/nodes/mention_node.rs +++ b/crates/wysiwyg/src/dom/nodes/mention_node.rs @@ -38,8 +38,8 @@ pub enum MentionNodeKind where S: UnicodeString, { - Room { mention: Mention }, - User { mention: Mention }, + Room { mention: Mention }, + User { mention: Mention }, MatrixUrl { display_text: S, url: S }, AtRoom, } @@ -81,8 +81,12 @@ where MentionNodeKind::MatrixUrl { display_text, .. } => { display_text.clone() } - MentionNodeKind::User { mention } => mention.display_text().clone(), - MentionNodeKind::Room { mention } => mention.display_text().clone(), + MentionNodeKind::User { mention } => { + S::from(mention.display_text()) + } + MentionNodeKind::Room { mention } => { + S::from(mention.display_text()) + } MentionNodeKind::AtRoom => S::from("@room"), } } @@ -243,10 +247,10 @@ where // There are two different functions to allow for fact one will use mxId later on match self.kind() { - User => { + User { .. } => { // TODO do something } - Room => { + Room { .. } => { // TODO do something } MatrixUrl { .. } => { From 437feee3ec8a5f50fa7fc488b680f38bf3ef20e2 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 14 Jun 2023 09:35:45 +0100 Subject: [PATCH 04/56] handle invalid URIs early in `insert*` methods --- crates/matrix_mentions/src/mention.rs | 5 ++++ crates/wysiwyg/src/composer_model/mentions.rs | 25 +++++++++++++------ crates/wysiwyg/src/dom/nodes/mention_node.rs | 6 +++++ 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/crates/matrix_mentions/src/mention.rs b/crates/matrix_mentions/src/mention.rs index a59875cc1..9720d24d5 100644 --- a/crates/matrix_mentions/src/mention.rs +++ b/crates/matrix_mentions/src/mention.rs @@ -59,6 +59,11 @@ impl Mention { &self.kind } + /// Determine if a uri is a valid matrix uri + pub fn is_matrix_uri(uri: &str) -> bool { + parse_matrix_id(uri).is_some() + } + /// Create a mention from a URI /// /// If the URI is a valid room or user, it creates a mention using the diff --git a/crates/wysiwyg/src/composer_model/mentions.rs b/crates/wysiwyg/src/composer_model/mentions.rs index 5c4c8e745..23d2c41a2 100644 --- a/crates/wysiwyg/src/composer_model/mentions.rs +++ b/crates/wysiwyg/src/composer_model/mentions.rs @@ -11,6 +11,7 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. +use matrix_mentions::Mention; use crate::{ dom::DomLocation, ComposerModel, ComposerUpdate, DomNode, Location, @@ -22,6 +23,7 @@ where S: UnicodeString, { /// Remove the suggestion text and then insert a mention into the composer, using the following rules + /// - Do not insert a mention if the uri is invalid /// - Do not insert a mention if the range includes link or code leaves /// - If the composer contains a selection, remove the contents of the selection /// prior to inserting a mention at the cursor. @@ -33,7 +35,7 @@ where suggestion: SuggestionPattern, attributes: Vec<(S, S)>, ) -> ComposerUpdate { - if self.should_not_insert_mention() { + if self.should_not_insert_mention(&url) { return ComposerUpdate::keep(); } @@ -45,6 +47,7 @@ where } /// Inserts a mention into the composer. It uses the following rules: + /// - Do not insert a mention if the uri is invalid /// - Do not insert a mention if the range includes link or code leaves /// - If the composer contains a selection, remove the contents of the selection /// prior to inserting a mention at the cursor. @@ -55,7 +58,7 @@ where text: S, attributes: Vec<(S, S)>, ) -> ComposerUpdate { - if self.should_not_insert_mention() { + if self.should_not_insert_mention(&url) { return ComposerUpdate::keep(); } @@ -103,19 +106,25 @@ where } } - /// Utility function for the insert_mention* methods. It returns false if the range - /// includes any link or code type leaves. + /// Utility function for the insert_mention* methods. It returns false if: + /// - the range includes any link or code type leaves + /// - the url is not a valid matrix uri /// /// Related issue is here: /// https://github.com/matrix-org/matrix-rich-text-editor/issues/702 /// We do not allow mentions to be inserted into links, the planned behaviour is /// detailed in the above issue. - fn should_not_insert_mention(&self) -> bool { + fn should_not_insert_mention(&self, url: &S) -> bool { let (start, end) = self.safe_selection(); let range = self.state.dom.find_range(start, end); - range.locations.iter().any(|l: &DomLocation| { - l.kind.is_link_kind() || l.kind.is_code_kind() - }) + let invalid_uri = !Mention::is_matrix_uri(url.to_string().as_str()); + + let range_contains_link_or_code_leaves = + range.locations.iter().any(|l: &DomLocation| { + l.kind.is_link_kind() || l.kind.is_code_kind() + }); + + invalid_uri || range_contains_link_or_code_leaves } } diff --git a/crates/wysiwyg/src/dom/nodes/mention_node.rs b/crates/wysiwyg/src/dom/nodes/mention_node.rs index 1089fb06e..473b119e1 100644 --- a/crates/wysiwyg/src/dom/nodes/mention_node.rs +++ b/crates/wysiwyg/src/dom/nodes/mention_node.rs @@ -55,6 +55,12 @@ where pub fn new(url: S, display_text: S, attributes: Vec<(S, S)>) -> Self { let handle = DomHandle::new_unset(); + // convert S to whatever is needed by doing S.to_string.as_str() + let thing = Mention::from_uri_with_display_text( + &url.to_string(), + &display_text.to_string(), + ); + Self { kind: MentionNodeKind::MatrixUrl { display_text, url }, attributes, From 5fb76eb2d1907d2216067e9e75e862079a96c0b1 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 14 Jun 2023 09:44:27 +0100 Subject: [PATCH 05/56] move at-room logic check to dom node layer --- crates/wysiwyg/src/composer_model/mentions.rs | 10 +--------- crates/wysiwyg/src/dom/nodes/dom_node.rs | 8 +++++++- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/crates/wysiwyg/src/composer_model/mentions.rs b/crates/wysiwyg/src/composer_model/mentions.rs index 23d2c41a2..337c4338e 100644 --- a/crates/wysiwyg/src/composer_model/mentions.rs +++ b/crates/wysiwyg/src/composer_model/mentions.rs @@ -80,15 +80,7 @@ where let (start, end) = self.safe_selection(); let range = self.state.dom.find_range(start, end); - // use the display text decide the mention type - // TODO extract this into a util function if it is reused when parsing the html prior to editing a message - // TODO decide if this do* function should be separated to handle mention vs at-room mention - // TODO handle invalid mention urls after permalink parsing methods have been created - let new_node = if text == "@room".into() { - DomNode::new_at_room_mention(attributes) - } else { - DomNode::new_mention(url, text, attributes) - }; + let new_node = DomNode::new_mention(url, text, attributes); let new_cursor_index = start + new_node.text_len(); diff --git a/crates/wysiwyg/src/dom/nodes/dom_node.rs b/crates/wysiwyg/src/dom/nodes/dom_node.rs index c77811e5a..df28a69e7 100644 --- a/crates/wysiwyg/src/dom/nodes/dom_node.rs +++ b/crates/wysiwyg/src/dom/nodes/dom_node.rs @@ -138,12 +138,18 @@ where DomNode::Container(ContainerNode::new_link(url, children, attributes)) } + /// Create a new mention node. This function will perform a single check of the display + /// text and return an at-room mention if that text exactly matches `@room` pub fn new_mention( url: S, display_text: S, attributes: Vec<(S, S)>, ) -> DomNode { - DomNode::Mention(MentionNode::new(url, display_text, attributes)) + if display_text == "@room".into() { + DomNode::Mention(MentionNode::new_at_room(attributes)) + } else { + DomNode::Mention(MentionNode::new(url, display_text, attributes)) + } } pub fn new_at_room_mention(attributes: Vec<(S, S)>) -> DomNode { From 64de4f6d2b7185bc23260fcd679b398de3d464db Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 14 Jun 2023 10:21:13 +0100 Subject: [PATCH 06/56] export MentionKind --- crates/matrix_mentions/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/matrix_mentions/src/lib.rs b/crates/matrix_mentions/src/lib.rs index d9f8229a0..09b6ae493 100644 --- a/crates/matrix_mentions/src/lib.rs +++ b/crates/matrix_mentions/src/lib.rs @@ -14,4 +14,4 @@ mod mention; -pub use crate::mention::Mention; +pub use crate::mention::{Mention, MentionKind}; From d60112f4a838e260c10701e8f257f8720ef6d444 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 14 Jun 2023 10:22:21 +0100 Subject: [PATCH 07/56] move to `Room` and `User` mention types --- crates/wysiwyg/src/dom/nodes/mention_node.rs | 109 ++++++++++++------- 1 file changed, 68 insertions(+), 41 deletions(-) diff --git a/crates/wysiwyg/src/dom/nodes/mention_node.rs b/crates/wysiwyg/src/dom/nodes/mention_node.rs index 473b119e1..8acf20c91 100644 --- a/crates/wysiwyg/src/dom/nodes/mention_node.rs +++ b/crates/wysiwyg/src/dom/nodes/mention_node.rs @@ -11,7 +11,7 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. -use matrix_mentions::Mention; +use matrix_mentions::{Mention, MentionKind}; use crate::composer_model::example_format::SelectionWriter; use crate::dom::dom_handle::DomHandle; @@ -28,20 +28,18 @@ pub struct MentionNode where S: UnicodeString, { - kind: MentionNodeKind, + display_text: S, + kind: MentionNodeKind, attributes: Vec<(S, S)>, handle: DomHandle, } #[derive(Clone, Debug, PartialEq, Eq)] -pub enum MentionNodeKind -where - S: UnicodeString, -{ +pub enum MentionNodeKind { Room { mention: Mention }, User { mention: Mention }, - MatrixUrl { display_text: S, url: S }, AtRoom, + Failed, } impl MentionNode @@ -55,16 +53,27 @@ where pub fn new(url: S, display_text: S, attributes: Vec<(S, S)>) -> Self { let handle = DomHandle::new_unset(); - // convert S to whatever is needed by doing S.to_string.as_str() - let thing = Mention::from_uri_with_display_text( + if let Some(mention) = Mention::from_uri_with_display_text( &url.to_string(), &display_text.to_string(), - ); - - Self { - kind: MentionNodeKind::MatrixUrl { display_text, url }, - attributes, - handle, + ) { + let kind = match mention.kind() { + MentionKind::Room => MentionNodeKind::Room { mention }, + MentionKind::User => MentionNodeKind::User { mention }, + }; + Self { + display_text, + kind, + attributes, + handle, + } + } else { + Self { + display_text: S::from("failed to parse"), + kind: MentionNodeKind::Failed, + attributes: vec![], + handle, + } } } @@ -72,6 +81,7 @@ where let handle = DomHandle::new_unset(); Self { + display_text: S::from("@room"), kind: MentionNodeKind::AtRoom, attributes, handle, @@ -84,16 +94,10 @@ where pub fn display_text(&self) -> S { match self.kind() { - MentionNodeKind::MatrixUrl { display_text, .. } => { - display_text.clone() - } - MentionNodeKind::User { mention } => { - S::from(mention.display_text()) - } - MentionNodeKind::Room { mention } => { - S::from(mention.display_text()) - } + MentionNodeKind::User { mention } => self.display_text.clone(), + MentionNodeKind::Room { mention } => self.display_text.clone(), MentionNodeKind::AtRoom => S::from("@room"), + MentionNodeKind::Failed => S::from("failed parsing"), } } @@ -111,7 +115,7 @@ where 1 } - pub fn kind(&self) -> &MentionNodeKind { + pub fn kind(&self) -> &MentionNodeKind { &self.kind } } @@ -144,27 +148,42 @@ impl MentionNode { let cur_pos = formatter.len(); match self.kind() { MentionNodeKind::User { mention } => { - // TODO do something + // if formatting as a message, only include the href attribute + let attributes = if as_message { + vec![("href".into(), S::from(mention.uri()))] + } else { + let mut attributes_for_composer = self.attributes.clone(); + attributes_for_composer + .push(("href".into(), S::from(mention.uri()))); + attributes_for_composer + .push(("contenteditable".into(), "false".into())); + attributes_for_composer + }; + + self.fmt_tag_open(tag, formatter, &Some(attributes)); + formatter.push(self.display_text()); + self.fmt_tag_close(tag, formatter); } MentionNodeKind::Room { mention } => { - // TODO do something - } - MentionNodeKind::MatrixUrl { display_text, url } => { // if formatting as a message, only include the href attribute let attributes = if as_message { - vec![("href".into(), url.clone())] + vec![("href".into(), S::from(mention.uri()))] } else { let mut attributes_for_composer = self.attributes.clone(); - attributes_for_composer.push(("href".into(), url.clone())); + attributes_for_composer + .push(("href".into(), S::from(mention.uri()))); attributes_for_composer .push(("contenteditable".into(), "false".into())); attributes_for_composer }; self.fmt_tag_open(tag, formatter, &Some(attributes)); - formatter.push(display_text.clone()); + formatter.push(self.display_text()); self.fmt_tag_close(tag, formatter); } + MentionNodeKind::Failed => { + // TODO do something + } MentionNodeKind::AtRoom => { // if formatting as a message, simply use the display text (@room) if as_message { @@ -218,14 +237,16 @@ where match self.kind() { MentionNodeKind::User { mention } => { - // TODO do something + description.push(", "); + description.push(mention.uri().clone()); } MentionNodeKind::Room { mention } => { - // TODO do something + description.push(", "); + description.push(mention.uri().clone()); } - MentionNodeKind::MatrixUrl { url, .. } => { + MentionNodeKind::Failed => { description.push(", "); - description.push(url.clone()); + description.push("failed parsing"); } MentionNodeKind::AtRoom => {} } @@ -253,15 +274,20 @@ where // There are two different functions to allow for fact one will use mxId later on match self.kind() { - User { .. } => { + User { mention } => { // TODO do something + fmt_user_or_room_mention(self, mention, buffer)?; } - Room { .. } => { + Room { mention } => { // TODO do something + fmt_user_or_room_mention(self, mention, buffer)?; } - MatrixUrl { .. } => { - fmt_user_or_room_mention(self, buffer)?; + Failed => { + // TODO do something } + // MatrixUrl { .. } => { + // fmt_user_or_room_mention(self, buffer)?; + // } AtRoom => { fmt_at_room_mention(self, buffer)?; } @@ -272,13 +298,14 @@ where #[inline(always)] fn fmt_user_or_room_mention( this: &MentionNode, + mention: &Mention, buffer: &mut S, ) -> Result<(), MarkdownError> where S: UnicodeString, { // TODO make this use mxId, for now we use display_text - buffer.push(this.display_text()); + buffer.push(mention.mx_id()); Ok(()) } From 2d2cdad2854fb316370fc6b4b9e9f7a386e4c2e6 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 14 Jun 2023 10:38:56 +0100 Subject: [PATCH 08/56] fix most tests --- crates/wysiwyg/src/composer_model/mentions.rs | 2 +- crates/wysiwyg/src/dom/nodes/mention_node.rs | 23 ++++++++++--------- .../wysiwyg/src/tests/test_to_message_html.rs | 13 ++++++----- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/crates/wysiwyg/src/composer_model/mentions.rs b/crates/wysiwyg/src/composer_model/mentions.rs index 337c4338e..829db03a0 100644 --- a/crates/wysiwyg/src/composer_model/mentions.rs +++ b/crates/wysiwyg/src/composer_model/mentions.rs @@ -100,7 +100,7 @@ where /// Utility function for the insert_mention* methods. It returns false if: /// - the range includes any link or code type leaves - /// - the url is not a valid matrix uri + /// - the url is not a valid matrix uri OR the `@room` special case /// /// Related issue is here: /// https://github.com/matrix-org/matrix-rich-text-editor/issues/702 diff --git a/crates/wysiwyg/src/dom/nodes/mention_node.rs b/crates/wysiwyg/src/dom/nodes/mention_node.rs index 8acf20c91..f0db0792e 100644 --- a/crates/wysiwyg/src/dom/nodes/mention_node.rs +++ b/crates/wysiwyg/src/dom/nodes/mention_node.rs @@ -270,25 +270,20 @@ where buffer: &mut S, _: &MarkdownOptions, ) -> Result<(), MarkdownError> { - use MentionNodeKind::*; - // There are two different functions to allow for fact one will use mxId later on match self.kind() { - User { mention } => { + MentionNodeKind::User { mention } => { // TODO do something fmt_user_or_room_mention(self, mention, buffer)?; } - Room { mention } => { + MentionNodeKind::Room { mention } => { // TODO do something fmt_user_or_room_mention(self, mention, buffer)?; } - Failed => { + MentionNodeKind::Failed => { // TODO do something } - // MatrixUrl { .. } => { - // fmt_user_or_room_mention(self, buffer)?; - // } - AtRoom => { + MentionNodeKind::AtRoom => { fmt_at_room_mention(self, buffer)?; } } @@ -304,8 +299,14 @@ where where S: UnicodeString, { - // TODO make this use mxId, for now we use display_text - buffer.push(mention.mx_id()); + // use the mx_id for a room mention, but the display text (name) for a user + let text = match this.kind() { + MentionNodeKind::User { .. } => this.display_text(), + MentionNodeKind::Room { mention } => S::from(mention.mx_id()), + _ => S::from("catch all"), + }; + + buffer.push(S::from(text)); Ok(()) } diff --git a/crates/wysiwyg/src/tests/test_to_message_html.rs b/crates/wysiwyg/src/tests/test_to_message_html.rs index 337097db8..4993d414a 100644 --- a/crates/wysiwyg/src/tests/test_to_message_html.rs +++ b/crates/wysiwyg/src/tests/test_to_message_html.rs @@ -31,23 +31,24 @@ fn replaces_empty_paragraphs_with_newline_characters() { let message_output = model.get_content_as_message_html(); assert_eq!(message_output, "

hello

\n\n\n

Alice

"); } + #[test] fn only_outputs_href_attribute_on_user_mention() { let mut model = cm("|"); model.insert_mention( - "www.url.com".into(), + "https://matrix.to/#/@alice:matrix.org".into(), "inner text".into(), vec![ ("data-mention-type".into(), "user".into()), ("style".into(), "some css".into()), ], ); - assert_eq!(tx(&model), "inner text |"); + assert_eq!(tx(&model), "inner text |"); let message_output = model.get_content_as_message_html(); assert_eq!( message_output, - "inner text\u{a0}" + "inner text\u{a0}" ); } @@ -55,19 +56,19 @@ fn only_outputs_href_attribute_on_user_mention() { fn only_outputs_href_attribute_on_room_mention() { let mut model = cm("|"); model.insert_mention( - "www.url.com".into(), + "https://matrix.to/#/#alice:matrix.org".into(), "inner text".into(), vec![ ("data-mention-type".into(), "room".into()), ("style".into(), "some css".into()), ], ); - assert_eq!(tx(&model), "inner text |"); + assert_eq!(tx(&model), "inner text |"); let message_output = model.get_content_as_message_html(); assert_eq!( message_output, - "inner text\u{a0}" + "inner text\u{a0}" ); } From 6e161142a9a7ac816749bd8fe7fc4fd226180b66 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 14 Jun 2023 10:41:13 +0100 Subject: [PATCH 09/56] get all tests passing --- crates/wysiwyg/src/composer_model/mentions.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/wysiwyg/src/composer_model/mentions.rs b/crates/wysiwyg/src/composer_model/mentions.rs index 829db03a0..f638c6c59 100644 --- a/crates/wysiwyg/src/composer_model/mentions.rs +++ b/crates/wysiwyg/src/composer_model/mentions.rs @@ -35,7 +35,8 @@ where suggestion: SuggestionPattern, attributes: Vec<(S, S)>, ) -> ComposerUpdate { - if self.should_not_insert_mention(&url) { + // we _do_ want to insert a room mention, regardss or url + if text != "@room".into() && self.should_not_insert_mention(&url) { return ComposerUpdate::keep(); } @@ -58,7 +59,8 @@ where text: S, attributes: Vec<(S, S)>, ) -> ComposerUpdate { - if self.should_not_insert_mention(&url) { + // we _do_ want to insert a room mention, regardss or url + if text != "@room".into() && self.should_not_insert_mention(&url) { return ComposerUpdate::keep(); } From 4a564958d1d0ea30a8255fba7a201e59efa3a345 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 14 Jun 2023 11:06:37 +0100 Subject: [PATCH 10/56] make `new` Mention return a `Result`, fix all tests/uses --- crates/wysiwyg/src/composer_model/mentions.rs | 24 ++++++++++--------- crates/wysiwyg/src/dom/nodes/dom_node.rs | 15 +++++++++--- crates/wysiwyg/src/dom/nodes/mention_node.rs | 24 +++++++++++-------- crates/wysiwyg/src/dom/parser/parse.rs | 11 +++++++-- 4 files changed, 48 insertions(+), 26 deletions(-) diff --git a/crates/wysiwyg/src/composer_model/mentions.rs b/crates/wysiwyg/src/composer_model/mentions.rs index f638c6c59..bfd30f00a 100644 --- a/crates/wysiwyg/src/composer_model/mentions.rs +++ b/crates/wysiwyg/src/composer_model/mentions.rs @@ -82,21 +82,23 @@ where let (start, end) = self.safe_selection(); let range = self.state.dom.find_range(start, end); - let new_node = DomNode::new_mention(url, text, attributes); + if let Ok(new_node) = DomNode::new_mention(url, text, attributes) { + let new_cursor_index = start + new_node.text_len(); - let new_cursor_index = start + new_node.text_len(); + let handle = self.state.dom.insert_node_at_cursor(&range, new_node); - let handle = self.state.dom.insert_node_at_cursor(&range, new_node); + // manually move the cursor to the end of the mention + self.state.start = Location::from(new_cursor_index); + self.state.end = self.state.start; - // manually move the cursor to the end of the mention - self.state.start = Location::from(new_cursor_index); - self.state.end = self.state.start; - - // add a trailing space in cases when we do not have a next sibling - if self.state.dom.is_last_in_parent(&handle) { - self.do_replace_text(" ".into()) + // add a trailing space in cases when we do not have a next sibling + if self.state.dom.is_last_in_parent(&handle) { + self.do_replace_text(" ".into()) + } else { + self.create_update_replace_all() + } } else { - self.create_update_replace_all() + ComposerUpdate::keep() } } diff --git a/crates/wysiwyg/src/dom/nodes/dom_node.rs b/crates/wysiwyg/src/dom/nodes/dom_node.rs index df28a69e7..d3f973953 100644 --- a/crates/wysiwyg/src/dom/nodes/dom_node.rs +++ b/crates/wysiwyg/src/dom/nodes/dom_node.rs @@ -140,15 +140,24 @@ where /// Create a new mention node. This function will perform a single check of the display /// text and return an at-room mention if that text exactly matches `@room` + /// + /// Returns a result as creating a mention node can fail with an invalid uri pub fn new_mention( url: S, display_text: S, attributes: Vec<(S, S)>, - ) -> DomNode { + ) -> Result, ()> { + // special case for at-room if display_text == "@room".into() { - DomNode::Mention(MentionNode::new_at_room(attributes)) + return Ok(DomNode::Mention(MentionNode::new_at_room(attributes))); + } + + if let Ok(mention_node) = + MentionNode::new(url, display_text, attributes) + { + Ok(DomNode::Mention(mention_node)) } else { - DomNode::Mention(MentionNode::new(url, display_text, attributes)) + Err(()) } } diff --git a/crates/wysiwyg/src/dom/nodes/mention_node.rs b/crates/wysiwyg/src/dom/nodes/mention_node.rs index f0db0792e..d0fe500d8 100644 --- a/crates/wysiwyg/src/dom/nodes/mention_node.rs +++ b/crates/wysiwyg/src/dom/nodes/mention_node.rs @@ -46,11 +46,16 @@ impl MentionNode where S: UnicodeString, { - /// Create a new MentionNode + /// Create a new MentionNode. This may fail if the uri can not be parsed, so + /// it will return `Result` /// /// NOTE: Its handle() will be unset until you call set_handle() or /// append() it to another node. - pub fn new(url: S, display_text: S, attributes: Vec<(S, S)>) -> Self { + pub fn new( + url: S, + display_text: S, + attributes: Vec<(S, S)>, + ) -> Result { let handle = DomHandle::new_unset(); if let Some(mention) = Mention::from_uri_with_display_text( @@ -61,22 +66,21 @@ where MentionKind::Room => MentionNodeKind::Room { mention }, MentionKind::User => MentionNodeKind::User { mention }, }; - Self { + Ok(Self { display_text, kind, attributes, handle, - } + }) } else { - Self { - display_text: S::from("failed to parse"), - kind: MentionNodeKind::Failed, - attributes: vec![], - handle, - } + Err(()) } } + /// Create a new at-room MentionNode. + /// + /// NOTE: Its handle() will be unset until you call set_handle() or + /// append() it to another node. pub fn new_at_room(attributes: Vec<(S, S)>) -> Self { let handle = DomHandle::new_unset(); diff --git a/crates/wysiwyg/src/dom/parser/parse.rs b/crates/wysiwyg/src/dom/parser/parse.rs index f3625c332..35ec916bd 100644 --- a/crates/wysiwyg/src/dom/parser/parse.rs +++ b/crates/wysiwyg/src/dom/parser/parse.rs @@ -316,12 +316,19 @@ mod sys { { let text = &text.content; - DomNode::new_mention( + // as creating a new mention might fail, we need to do something in the case where it fails + + let try_create = DomNode::new_mention( link.get_attr("href").unwrap_or("").into(), text.as_str().into(), // custom attributes are not required when cfg feature != "js" vec![], - ) + ); + + match try_create { + Ok(node) => node, + Err(_) => Self::new_link(link), + } } /// Create a list node From 3a82ef86f093ba2b4a3c6deb770ceac4d25df2c9 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 14 Jun 2023 12:10:05 +0100 Subject: [PATCH 11/56] tidy up the logic --- crates/wysiwyg/src/composer_model/mentions.rs | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/crates/wysiwyg/src/composer_model/mentions.rs b/crates/wysiwyg/src/composer_model/mentions.rs index bfd30f00a..2c90c9415 100644 --- a/crates/wysiwyg/src/composer_model/mentions.rs +++ b/crates/wysiwyg/src/composer_model/mentions.rs @@ -35,8 +35,7 @@ where suggestion: SuggestionPattern, attributes: Vec<(S, S)>, ) -> ComposerUpdate { - // we _do_ want to insert a room mention, regardss or url - if text != "@room".into() && self.should_not_insert_mention(&url) { + if self.should_not_insert_mention(&url, &text) { return ComposerUpdate::keep(); } @@ -59,8 +58,7 @@ where text: S, attributes: Vec<(S, S)>, ) -> ComposerUpdate { - // we _do_ want to insert a room mention, regardss or url - if text != "@room".into() && self.should_not_insert_mention(&url) { + if self.should_not_insert_mention(&url, &text) { return ComposerUpdate::keep(); } @@ -71,8 +69,9 @@ where self.do_insert_mention(url, text, attributes) } - /// Creates a new mention node then inserts the node at the cursor position. It adds a trailing space when the inserted - /// mention is the last node in it's parent. + /// Creates a new mention node then inserts the node at the cursor position. If creation fails due to + /// an invalid uri, it will return `ComposerUpdate::keep()`. + /// It adds a trailing space when the inserted mention is the last node in it's parent. fn do_insert_mention( &mut self, url: S, @@ -110,7 +109,7 @@ where /// https://github.com/matrix-org/matrix-rich-text-editor/issues/702 /// We do not allow mentions to be inserted into links, the planned behaviour is /// detailed in the above issue. - fn should_not_insert_mention(&self, url: &S) -> bool { + fn should_not_insert_mention(&self, url: &S, text: &S) -> bool { let (start, end) = self.safe_selection(); let range = self.state.dom.find_range(start, end); @@ -121,6 +120,12 @@ where l.kind.is_link_kind() || l.kind.is_code_kind() }); - invalid_uri || range_contains_link_or_code_leaves + // when we have an at-room mention, it doesn't matter about the url as we do not use + // it, rendering the mention as raw text in the html output + if text == &S::from("@room") { + range_contains_link_or_code_leaves + } else { + invalid_uri || range_contains_link_or_code_leaves + } } } From 65bfbaf275de39b4d2483ee296a88b3d0e0eab55 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 14 Jun 2023 12:11:37 +0100 Subject: [PATCH 12/56] rename function --- crates/matrix_mentions/src/mention.rs | 2 +- crates/wysiwyg/src/composer_model/mentions.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/matrix_mentions/src/mention.rs b/crates/matrix_mentions/src/mention.rs index 9720d24d5..cefe802ed 100644 --- a/crates/matrix_mentions/src/mention.rs +++ b/crates/matrix_mentions/src/mention.rs @@ -60,7 +60,7 @@ impl Mention { } /// Determine if a uri is a valid matrix uri - pub fn is_matrix_uri(uri: &str) -> bool { + pub fn is_valid_uri(uri: &str) -> bool { parse_matrix_id(uri).is_some() } diff --git a/crates/wysiwyg/src/composer_model/mentions.rs b/crates/wysiwyg/src/composer_model/mentions.rs index 2c90c9415..83bf69830 100644 --- a/crates/wysiwyg/src/composer_model/mentions.rs +++ b/crates/wysiwyg/src/composer_model/mentions.rs @@ -113,7 +113,7 @@ where let (start, end) = self.safe_selection(); let range = self.state.dom.find_range(start, end); - let invalid_uri = !Mention::is_matrix_uri(url.to_string().as_str()); + let invalid_uri = !Mention::is_valid_uri(url.to_string().as_str()); let range_contains_link_or_code_leaves = range.locations.iter().any(|l: &DomLocation| { From 8712fc1255b1e6f3a05648737b9c1b13b996f942 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 14 Jun 2023 12:13:43 +0100 Subject: [PATCH 13/56] update comment --- crates/wysiwyg/src/composer_model/mentions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/wysiwyg/src/composer_model/mentions.rs b/crates/wysiwyg/src/composer_model/mentions.rs index 83bf69830..1e22c9203 100644 --- a/crates/wysiwyg/src/composer_model/mentions.rs +++ b/crates/wysiwyg/src/composer_model/mentions.rs @@ -103,7 +103,7 @@ where /// Utility function for the insert_mention* methods. It returns false if: /// - the range includes any link or code type leaves - /// - the url is not a valid matrix uri OR the `@room` special case + /// - the url is not a valid matrix uri (with special case for at-room) /// /// Related issue is here: /// https://github.com/matrix-org/matrix-rich-text-editor/issues/702 From 8e159f86269336c7e9dc79d336642f8b49e1c7b7 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 14 Jun 2023 12:19:35 +0100 Subject: [PATCH 14/56] update comments, strip out unused code --- crates/wysiwyg/src/dom/nodes/dom_node.rs | 6 ++---- crates/wysiwyg/src/dom/nodes/mention_node.rs | 14 ++------------ 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/crates/wysiwyg/src/dom/nodes/dom_node.rs b/crates/wysiwyg/src/dom/nodes/dom_node.rs index d3f973953..d4667dbe2 100644 --- a/crates/wysiwyg/src/dom/nodes/dom_node.rs +++ b/crates/wysiwyg/src/dom/nodes/dom_node.rs @@ -149,10 +149,8 @@ where ) -> Result, ()> { // special case for at-room if display_text == "@room".into() { - return Ok(DomNode::Mention(MentionNode::new_at_room(attributes))); - } - - if let Ok(mention_node) = + Ok(DomNode::Mention(MentionNode::new_at_room(attributes))) + } else if let Ok(mention_node) = MentionNode::new(url, display_text, attributes) { Ok(DomNode::Mention(mention_node)) diff --git a/crates/wysiwyg/src/dom/nodes/mention_node.rs b/crates/wysiwyg/src/dom/nodes/mention_node.rs index d0fe500d8..70860297c 100644 --- a/crates/wysiwyg/src/dom/nodes/mention_node.rs +++ b/crates/wysiwyg/src/dom/nodes/mention_node.rs @@ -28,6 +28,8 @@ pub struct MentionNode where S: UnicodeString, { + // `display_text` refers to that passed by the client which may, in some cases, be different + // from the ruma derived `Mention.display_text` display_text: S, kind: MentionNodeKind, attributes: Vec<(S, S)>, @@ -39,7 +41,6 @@ pub enum MentionNodeKind { Room { mention: Mention }, User { mention: Mention }, AtRoom, - Failed, } impl MentionNode @@ -101,7 +102,6 @@ where MentionNodeKind::User { mention } => self.display_text.clone(), MentionNodeKind::Room { mention } => self.display_text.clone(), MentionNodeKind::AtRoom => S::from("@room"), - MentionNodeKind::Failed => S::from("failed parsing"), } } @@ -185,9 +185,6 @@ impl MentionNode { formatter.push(self.display_text()); self.fmt_tag_close(tag, formatter); } - MentionNodeKind::Failed => { - // TODO do something - } MentionNodeKind::AtRoom => { // if formatting as a message, simply use the display text (@room) if as_message { @@ -248,10 +245,6 @@ where description.push(", "); description.push(mention.uri().clone()); } - MentionNodeKind::Failed => { - description.push(", "); - description.push("failed parsing"); - } MentionNodeKind::AtRoom => {} } @@ -284,9 +277,6 @@ where // TODO do something fmt_user_or_room_mention(self, mention, buffer)?; } - MentionNodeKind::Failed => { - // TODO do something - } MentionNodeKind::AtRoom => { fmt_at_room_mention(self, buffer)?; } From 8e7c42d331ffa1fb1f0c4862d049add1d340b6fb Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 14 Jun 2023 12:32:31 +0100 Subject: [PATCH 15/56] make the message output use mx_id for room mentions --- crates/wysiwyg/src/dom/nodes/mention_node.rs | 32 +++++++++---------- .../wysiwyg/src/tests/test_to_message_html.rs | 4 +-- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/crates/wysiwyg/src/dom/nodes/mention_node.rs b/crates/wysiwyg/src/dom/nodes/mention_node.rs index 70860297c..7aca3f388 100644 --- a/crates/wysiwyg/src/dom/nodes/mention_node.rs +++ b/crates/wysiwyg/src/dom/nodes/mention_node.rs @@ -156,12 +156,10 @@ impl MentionNode { let attributes = if as_message { vec![("href".into(), S::from(mention.uri()))] } else { - let mut attributes_for_composer = self.attributes.clone(); - attributes_for_composer - .push(("href".into(), S::from(mention.uri()))); - attributes_for_composer - .push(("contenteditable".into(), "false".into())); - attributes_for_composer + let mut attrs = self.attributes.clone(); + attrs.push(("href".into(), S::from(mention.uri()))); + attrs.push(("contenteditable".into(), "false".into())); + attrs }; self.fmt_tag_open(tag, formatter, &Some(attributes)); @@ -169,20 +167,22 @@ impl MentionNode { self.fmt_tag_close(tag, formatter); } MentionNodeKind::Room { mention } => { - // if formatting as a message, only include the href attribute - let attributes = if as_message { - vec![("href".into(), S::from(mention.uri()))] + // if formatting as a message, only include the href attribute and also use the mx_id as + // the display text + let (attributes, display_text) = if as_message { + ( + vec![("href".into(), S::from(mention.uri()))], + S::from(mention.mx_id()), + ) } else { - let mut attributes_for_composer = self.attributes.clone(); - attributes_for_composer - .push(("href".into(), S::from(mention.uri()))); - attributes_for_composer - .push(("contenteditable".into(), "false".into())); - attributes_for_composer + let mut attrs = self.attributes.clone(); + attrs.push(("href".into(), S::from(mention.uri()))); + attrs.push(("contenteditable".into(), "false".into())); + (attrs, self.display_text()) }; self.fmt_tag_open(tag, formatter, &Some(attributes)); - formatter.push(self.display_text()); + formatter.push(display_text); self.fmt_tag_close(tag, formatter); } MentionNodeKind::AtRoom => { diff --git a/crates/wysiwyg/src/tests/test_to_message_html.rs b/crates/wysiwyg/src/tests/test_to_message_html.rs index 4993d414a..aeb9e4577 100644 --- a/crates/wysiwyg/src/tests/test_to_message_html.rs +++ b/crates/wysiwyg/src/tests/test_to_message_html.rs @@ -53,7 +53,7 @@ fn only_outputs_href_attribute_on_user_mention() { } #[test] -fn only_outputs_href_attribute_on_room_mention() { +fn only_outputs_href_attribute_on_room_mention_and_uses_mx_id() { let mut model = cm("|"); model.insert_mention( "https://matrix.to/#/#alice:matrix.org".into(), @@ -68,7 +68,7 @@ fn only_outputs_href_attribute_on_room_mention() { let message_output = model.get_content_as_message_html(); assert_eq!( message_output, - "inner text\u{a0}" + "#alice:matrix.org\u{a0}" ); } From e0d2f1c283eb0b7464ec0f0e529e489b82578216 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 14 Jun 2023 12:48:19 +0100 Subject: [PATCH 16/56] tidy up parsing --- crates/wysiwyg/src/dom/nodes/mention_node.rs | 2 +- crates/wysiwyg/src/dom/parser/parse.rs | 36 ++++++++------------ crates/wysiwyg/src/tests/test_to_markdown.rs | 4 +-- 3 files changed, 18 insertions(+), 24 deletions(-) diff --git a/crates/wysiwyg/src/dom/nodes/mention_node.rs b/crates/wysiwyg/src/dom/nodes/mention_node.rs index 7aca3f388..4566a1d31 100644 --- a/crates/wysiwyg/src/dom/nodes/mention_node.rs +++ b/crates/wysiwyg/src/dom/nodes/mention_node.rs @@ -295,7 +295,7 @@ where { // use the mx_id for a room mention, but the display text (name) for a user let text = match this.kind() { - MentionNodeKind::User { .. } => this.display_text(), + MentionNodeKind::User { mention } => S::from(mention.mx_id()), MentionNodeKind::Room { mention } => S::from(mention.mx_id()), _ => S::from("catch all"), }; diff --git a/crates/wysiwyg/src/dom/parser/parse.rs b/crates/wysiwyg/src/dom/parser/parse.rs index 35ec916bd..1f3640a52 100644 --- a/crates/wysiwyg/src/dom/parser/parse.rs +++ b/crates/wysiwyg/src/dom/parser/parse.rs @@ -11,6 +11,7 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. +use matrix_mentions::Mention; use crate::dom::dom_creation_error::HtmlParseError; use crate::dom::nodes::dom_node::DomNodeKind::CodeBlock; @@ -178,14 +179,8 @@ mod sys { self.current_path.remove(cur_path_idx); } "a" => { - // TODO: Replace this logic with real mention detection - // The only mention that is currently detected is the - // example mxid, @test:example.org. let is_mention = child.attrs.iter().any(|(k, v)| { - k == &String::from("href") - && v.starts_with( - "https://matrix.to/#/@test:example.org", - ) + k == &String::from("href") && Mention::is_valid_uri(v) }); let text = @@ -734,6 +729,7 @@ mod js { dom::nodes::{ContainerNode, DomNode}, InlineFormatType, ListType, }; + use matrix_mentions::Mention; use std::fmt; use wasm_bindgen::JsCast; use web_sys::{Document, DomParser, Element, NodeList, SupportedType}; @@ -856,12 +852,7 @@ mod js { .get_attribute("href") .unwrap_or_default(); - // TODO: Replace this logic with real mention detection - // The only mention that is currently detected is the - // example mxid, @test:example.org. - let is_mention = url.starts_with( - "https://matrix.to/#/@test:example.org", - ); + let is_mention = Matrix::is_valid_uri(url.to_string()); let text = node.child_nodes().get(0); let has_text = match text.clone() { Some(node) => { @@ -870,14 +861,17 @@ mod js { None => false, }; if has_text && is_mention { - dom.append_child(DomNode::new_mention( - url.into(), - text.unwrap() - .node_value() - .unwrap_or_default() - .into(), - attributes, - )); + dom.append_child( + DomNode::new_mention( + url.into(), + text.unwrap() + .node_value() + .unwrap_or_default() + .into(), + attributes, + ) + .unwrap(), // we unwrap because we have already confirmed the uri is valid + ); } else { let children = self .convert(node.child_nodes())? diff --git a/crates/wysiwyg/src/tests/test_to_markdown.rs b/crates/wysiwyg/src/tests/test_to_markdown.rs index af9f3b6d1..a4b2d895b 100644 --- a/crates/wysiwyg/src/tests/test_to_markdown.rs +++ b/crates/wysiwyg/src/tests/test_to_markdown.rs @@ -206,8 +206,8 @@ fn list_ordered_and_unordered() { #[test] fn mention() { assert_to_md_no_roundtrip( - r#"test"#, - r#"test"#, + r#"test"#, + r#"@alice:matrix.org"#, ); } From 834e5cc2d33098f6cd30fb3ce565eb94d0b8a8d5 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 14 Jun 2023 13:00:38 +0100 Subject: [PATCH 17/56] tidy up match branches and functions --- crates/wysiwyg/src/dom/nodes/mention_node.rs | 41 ++++---------------- 1 file changed, 7 insertions(+), 34 deletions(-) diff --git a/crates/wysiwyg/src/dom/nodes/mention_node.rs b/crates/wysiwyg/src/dom/nodes/mention_node.rs index 4566a1d31..ceac3a627 100644 --- a/crates/wysiwyg/src/dom/nodes/mention_node.rs +++ b/crates/wysiwyg/src/dom/nodes/mention_node.rs @@ -267,52 +267,25 @@ where buffer: &mut S, _: &MarkdownOptions, ) -> Result<(), MarkdownError> { - // There are two different functions to allow for fact one will use mxId later on - match self.kind() { - MentionNodeKind::User { mention } => { - // TODO do something - fmt_user_or_room_mention(self, mention, buffer)?; - } - MentionNodeKind::Room { mention } => { - // TODO do something - fmt_user_or_room_mention(self, mention, buffer)?; - } - MentionNodeKind::AtRoom => { - fmt_at_room_mention(self, buffer)?; - } - } - + fmt_mention(self, buffer)?; return Ok(()); #[inline(always)] - fn fmt_user_or_room_mention( + fn fmt_mention( this: &MentionNode, - mention: &Mention, buffer: &mut S, ) -> Result<(), MarkdownError> where S: UnicodeString, { - // use the mx_id for a room mention, but the display text (name) for a user let text = match this.kind() { - MentionNodeKind::User { mention } => S::from(mention.mx_id()), - MentionNodeKind::Room { mention } => S::from(mention.mx_id()), - _ => S::from("catch all"), + // for User/Room type, we use the mx_id in the md output + MentionNodeKind::User { mention } + | MentionNodeKind::Room { mention } => S::from(mention.mx_id()), + MentionNodeKind::AtRoom => this.display_text(), }; - buffer.push(S::from(text)); - Ok(()) - } - - #[inline(always)] - fn fmt_at_room_mention( - this: &MentionNode, - buffer: &mut S, - ) -> Result<(), MarkdownError> - where - S: UnicodeString, - { - buffer.push(this.display_text()); + buffer.push(text); Ok(()) } } From 1b0568bf53f886ad45ff377c4565bdf3af67030e Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 14 Jun 2023 13:07:42 +0100 Subject: [PATCH 18/56] tidy --- crates/wysiwyg/src/dom/nodes/mention_node.rs | 4 ++-- crates/wysiwyg/src/dom/parser/parse.rs | 7 +++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/crates/wysiwyg/src/dom/nodes/mention_node.rs b/crates/wysiwyg/src/dom/nodes/mention_node.rs index ceac3a627..ddda9ba29 100644 --- a/crates/wysiwyg/src/dom/nodes/mention_node.rs +++ b/crates/wysiwyg/src/dom/nodes/mention_node.rs @@ -99,8 +99,8 @@ where pub fn display_text(&self) -> S { match self.kind() { - MentionNodeKind::User { mention } => self.display_text.clone(), - MentionNodeKind::Room { mention } => self.display_text.clone(), + MentionNodeKind::User { mention } + | MentionNodeKind::Room { mention } => self.display_text.clone(), MentionNodeKind::AtRoom => S::from("@room"), } } diff --git a/crates/wysiwyg/src/dom/parser/parse.rs b/crates/wysiwyg/src/dom/parser/parse.rs index 1f3640a52..ddd7c4ef8 100644 --- a/crates/wysiwyg/src/dom/parser/parse.rs +++ b/crates/wysiwyg/src/dom/parser/parse.rs @@ -311,16 +311,15 @@ mod sys { { let text = &text.content; - // as creating a new mention might fail, we need to do something in the case where it fails - - let try_create = DomNode::new_mention( + // creating a mention node could fail if the uri is invalid + let creation_result = DomNode::new_mention( link.get_attr("href").unwrap_or("").into(), text.as_str().into(), // custom attributes are not required when cfg feature != "js" vec![], ); - match try_create { + match creation_result { Ok(node) => node, Err(_) => Self::new_link(link), } From 1cd484a69d3e9b0b790e3b677f08aa882dcf6a31 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 14 Jun 2023 13:23:47 +0100 Subject: [PATCH 19/56] fix clippy errors --- crates/wysiwyg/src/dom/nodes/dom_node.rs | 5 +++-- crates/wysiwyg/src/dom/nodes/mention_node.rs | 16 ++++++++++------ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/crates/wysiwyg/src/dom/nodes/dom_node.rs b/crates/wysiwyg/src/dom/nodes/dom_node.rs index d4667dbe2..1f35e0f28 100644 --- a/crates/wysiwyg/src/dom/nodes/dom_node.rs +++ b/crates/wysiwyg/src/dom/nodes/dom_node.rs @@ -26,6 +26,7 @@ use crate::dom::unicode_string::UnicodeStrExt; use crate::dom::{self, UnicodeString}; use crate::{InlineFormatType, ListType}; +use super::mention_node::UriParseError; use super::MentionNode; #[derive(Clone, Debug, PartialEq)] @@ -146,7 +147,7 @@ where url: S, display_text: S, attributes: Vec<(S, S)>, - ) -> Result, ()> { + ) -> Result, UriParseError> { // special case for at-room if display_text == "@room".into() { Ok(DomNode::Mention(MentionNode::new_at_room(attributes))) @@ -155,7 +156,7 @@ where { Ok(DomNode::Mention(mention_node)) } else { - Err(()) + Err(UriParseError) } } diff --git a/crates/wysiwyg/src/dom/nodes/mention_node.rs b/crates/wysiwyg/src/dom/nodes/mention_node.rs index ddda9ba29..1ee822294 100644 --- a/crates/wysiwyg/src/dom/nodes/mention_node.rs +++ b/crates/wysiwyg/src/dom/nodes/mention_node.rs @@ -23,6 +23,9 @@ use crate::dom::to_tree::ToTree; use crate::dom::unicode_string::{UnicodeStrExt, UnicodeStringExt}; use crate::dom::UnicodeString; +#[derive(Debug)] +pub struct UriParseError; + #[derive(Clone, Debug, PartialEq, Eq)] pub struct MentionNode where @@ -56,7 +59,7 @@ where url: S, display_text: S, attributes: Vec<(S, S)>, - ) -> Result { + ) -> Result { let handle = DomHandle::new_unset(); if let Some(mention) = Mention::from_uri_with_display_text( @@ -74,7 +77,7 @@ where handle, }) } else { - Err(()) + Err(UriParseError) } } @@ -99,8 +102,9 @@ where pub fn display_text(&self) -> S { match self.kind() { - MentionNodeKind::User { mention } - | MentionNodeKind::Room { mention } => self.display_text.clone(), + MentionNodeKind::User { .. } | MentionNodeKind::Room { .. } => { + self.display_text.clone() + } MentionNodeKind::AtRoom => S::from("@room"), } } @@ -239,11 +243,11 @@ where match self.kind() { MentionNodeKind::User { mention } => { description.push(", "); - description.push(mention.uri().clone()); + description.push(S::from(mention.uri())); } MentionNodeKind::Room { mention } => { description.push(", "); - description.push(mention.uri().clone()); + description.push(S::from(mention.uri())); } MentionNodeKind::AtRoom => {} } From d011a5b231934dfc72bafc762eb18da64f06d82a Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 14 Jun 2023 13:35:09 +0100 Subject: [PATCH 20/56] fix typo --- crates/wysiwyg/src/dom/parser/parse.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/wysiwyg/src/dom/parser/parse.rs b/crates/wysiwyg/src/dom/parser/parse.rs index ddd7c4ef8..fbc8e5545 100644 --- a/crates/wysiwyg/src/dom/parser/parse.rs +++ b/crates/wysiwyg/src/dom/parser/parse.rs @@ -851,7 +851,7 @@ mod js { .get_attribute("href") .unwrap_or_default(); - let is_mention = Matrix::is_valid_uri(url.to_string()); + let is_mention = Mention::is_valid_uri(url.to_string()); let text = node.child_nodes().get(0); let has_text = match text.clone() { Some(node) => { From d3a0b74f820583b81fcfdba45c2f61f2542e1771 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 14 Jun 2023 13:58:37 +0100 Subject: [PATCH 21/56] extract `@room` checking/setting --- crates/wysiwyg/src/composer_model/mentions.rs | 12 +++++++++++- crates/wysiwyg/src/dom/nodes/dom_node.rs | 3 +-- crates/wysiwyg/src/dom/nodes/mention_node.rs | 14 ++++++++++++-- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/crates/wysiwyg/src/composer_model/mentions.rs b/crates/wysiwyg/src/composer_model/mentions.rs index 1e22c9203..638bf3836 100644 --- a/crates/wysiwyg/src/composer_model/mentions.rs +++ b/crates/wysiwyg/src/composer_model/mentions.rs @@ -122,10 +122,20 @@ where // when we have an at-room mention, it doesn't matter about the url as we do not use // it, rendering the mention as raw text in the html output - if text == &S::from("@room") { + if Self::is_at_room_display_text(text) { range_contains_link_or_code_leaves } else { invalid_uri || range_contains_link_or_code_leaves } } + + /// Util function to check if the display text is that of an at-room mention + pub fn is_at_room_display_text(text: &S) -> bool { + text == &S::from("@room") + } + + /// Util function to get the display text for an at-room mention + pub fn get_at_room_display_text() -> S { + S::from("@room") + } } diff --git a/crates/wysiwyg/src/dom/nodes/dom_node.rs b/crates/wysiwyg/src/dom/nodes/dom_node.rs index 1f35e0f28..de2378bd9 100644 --- a/crates/wysiwyg/src/dom/nodes/dom_node.rs +++ b/crates/wysiwyg/src/dom/nodes/dom_node.rs @@ -11,7 +11,6 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. - use crate::composer_model::example_format::SelectionWriter; use crate::dom::dom_handle::DomHandle; use crate::dom::nodes::{ @@ -149,7 +148,7 @@ where attributes: Vec<(S, S)>, ) -> Result, UriParseError> { // special case for at-room - if display_text == "@room".into() { + if MentionNode::is_at_room_display_text(&display_text) { Ok(DomNode::Mention(MentionNode::new_at_room(attributes))) } else if let Ok(mention_node) = MentionNode::new(url, display_text, attributes) diff --git a/crates/wysiwyg/src/dom/nodes/mention_node.rs b/crates/wysiwyg/src/dom/nodes/mention_node.rs index 1ee822294..ed494e34a 100644 --- a/crates/wysiwyg/src/dom/nodes/mention_node.rs +++ b/crates/wysiwyg/src/dom/nodes/mention_node.rs @@ -89,7 +89,7 @@ where let handle = DomHandle::new_unset(); Self { - display_text: S::from("@room"), + display_text: MentionNode::get_at_room_display_text(), kind: MentionNodeKind::AtRoom, attributes, handle, @@ -105,7 +105,7 @@ where MentionNodeKind::User { .. } | MentionNodeKind::Room { .. } => { self.display_text.clone() } - MentionNodeKind::AtRoom => S::from("@room"), + MentionNodeKind::AtRoom => MentionNode::get_at_room_display_text(), } } @@ -126,6 +126,16 @@ where pub fn kind(&self) -> &MentionNodeKind { &self.kind } + + /// Util function to check if the display text is that of an at-room mention + pub fn is_at_room_display_text(text: &S) -> bool { + text == &S::from("@room") + } + + /// Util function to get the display text for an at-room mention + pub fn get_at_room_display_text() -> S { + S::from("@room") + } } impl ToHtml for MentionNode From d2f0edfbb311a6b8a4ae46dd5a314e6bea3be0f9 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 14 Jun 2023 13:59:59 +0100 Subject: [PATCH 22/56] add import --- crates/wysiwyg/src/composer_model/mentions.rs | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/crates/wysiwyg/src/composer_model/mentions.rs b/crates/wysiwyg/src/composer_model/mentions.rs index 638bf3836..bbff20315 100644 --- a/crates/wysiwyg/src/composer_model/mentions.rs +++ b/crates/wysiwyg/src/composer_model/mentions.rs @@ -14,8 +14,9 @@ use matrix_mentions::Mention; use crate::{ - dom::DomLocation, ComposerModel, ComposerUpdate, DomNode, Location, - SuggestionPattern, UnicodeString, + dom::{nodes::MentionNode, DomLocation}, + ComposerModel, ComposerUpdate, DomNode, Location, SuggestionPattern, + UnicodeString, }; impl ComposerModel @@ -122,20 +123,10 @@ where // when we have an at-room mention, it doesn't matter about the url as we do not use // it, rendering the mention as raw text in the html output - if Self::is_at_room_display_text(text) { + if MentionNode::is_at_room_display_text(text) { range_contains_link_or_code_leaves } else { invalid_uri || range_contains_link_or_code_leaves } } - - /// Util function to check if the display text is that of an at-room mention - pub fn is_at_room_display_text(text: &S) -> bool { - text == &S::from("@room") - } - - /// Util function to get the display text for an at-room mention - pub fn get_at_room_display_text() -> S { - S::from("@room") - } } From 1108eca47c2cbc5f8bf11c86faa121883980cc66 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 14 Jun 2023 14:06:00 +0100 Subject: [PATCH 23/56] add test --- crates/wysiwyg/src/tests/test_mentions.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/crates/wysiwyg/src/tests/test_mentions.rs b/crates/wysiwyg/src/tests/test_mentions.rs index e1a861191..eee3ae215 100644 --- a/crates/wysiwyg/src/tests/test_mentions.rs +++ b/crates/wysiwyg/src/tests/test_mentions.rs @@ -18,6 +18,15 @@ use crate::{ tests::testutils_composer_model::{cm, tx}, ComposerModel, MenuAction, }; +/** + * INSERTING INVALID URL + */ +#[test] +fn inserting_with_invalid_mention_url_does_nothing() { + let mut model = cm("|"); + model.insert_mention("invalid mention url".into(), "@Alice".into(), vec![]); + assert_eq!(tx(&model), "|"); +} /** * ATTRIBUTE TESTS From e018d42231719e6d0e9fff01a861e3198974fae2 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 14 Jun 2023 14:10:11 +0100 Subject: [PATCH 24/56] try to fix wasm error --- crates/wysiwyg/src/dom/parser/parse.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/wysiwyg/src/dom/parser/parse.rs b/crates/wysiwyg/src/dom/parser/parse.rs index fbc8e5545..2efbb4287 100644 --- a/crates/wysiwyg/src/dom/parser/parse.rs +++ b/crates/wysiwyg/src/dom/parser/parse.rs @@ -851,7 +851,8 @@ mod js { .get_attribute("href") .unwrap_or_default(); - let is_mention = Mention::is_valid_uri(url.to_string()); + let is_mention = + Mention::is_valid_uri(&url.to_string()); let text = node.child_nodes().get(0); let has_text = match text.clone() { Some(node) => { From b6d86e60d55b6ba480ba3fe796b1ebcc46a7281b Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 14 Jun 2023 14:50:02 +0100 Subject: [PATCH 25/56] put import in correct place --- crates/wysiwyg/src/dom/parser/parse.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/wysiwyg/src/dom/parser/parse.rs b/crates/wysiwyg/src/dom/parser/parse.rs index 2efbb4287..ec3b78ee5 100644 --- a/crates/wysiwyg/src/dom/parser/parse.rs +++ b/crates/wysiwyg/src/dom/parser/parse.rs @@ -11,7 +11,6 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. -use matrix_mentions::Mention; use crate::dom::dom_creation_error::HtmlParseError; use crate::dom::nodes::dom_node::DomNodeKind::CodeBlock; @@ -36,6 +35,8 @@ where #[cfg(feature = "sys")] mod sys { + use matrix_mentions::Mention; + use super::super::padom_node::PaDomNode; use super::super::PaNodeContainer; use super::super::{PaDom, PaDomCreationError, PaDomCreator}; From 7a6bf6f92f436f62752ce1e136550a2827bef324 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 10:21:21 +0100 Subject: [PATCH 26/56] get solution for custom links working --- crates/matrix_mentions/src/mention.rs | 47 +++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/crates/matrix_mentions/src/mention.rs b/crates/matrix_mentions/src/mention.rs index cefe802ed..26a47e942 100644 --- a/crates/matrix_mentions/src/mention.rs +++ b/crates/matrix_mentions/src/mention.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use ruma_common::{matrix_uri::MatrixId, MatrixToUri, MatrixUri}; +use ruma_common::{matrix_uri::MatrixId, MatrixToUri, MatrixUri, UserId}; #[derive(Clone, Debug, PartialEq, Eq)] pub struct Mention { @@ -147,11 +147,42 @@ impl Mention { } } +/// Determines if a uri can be parsed for a matrix id. Attempts to treat the uri in three +/// ways when parsing: +/// 1 - As a matrix uri +/// 2 - As a matrix to uri +/// 3 - As a custom uri +/// +/// If any of the above succeed, return Some Option { if let Ok(matrix_uri) = MatrixUri::parse(uri) { Some(matrix_uri.id().to_owned()) } else if let Ok(matrix_to_uri) = MatrixToUri::parse(uri) { Some(matrix_to_uri.id().to_owned()) + } else if let Some(matrix_id) = parse_external_id(uri) { + Some(matrix_id.to_owned()) + } else { + None + } +} + +/// Splits an external uri on `/#/` and then attempts to find the relevant matrix information +/// from after the split +/// +/// If successful return Some. Else return None. +fn parse_external_id(uri: &str) -> Option { + // first split the string into the parts we need + let parts: Vec<&str> = uri.split("/#/").collect(); + let before_hash = parts[0]; + let after_hash = parts[1]; + + // now rebuild the uri and try again... + let hacked_uri = format!("{}{}", "https://matrix.to/#/", after_hash); + + if let Ok(matrix_uri) = MatrixUri::parse(&*hacked_uri) { + Some(matrix_uri.id().to_owned()) + } else if let Ok(matrix_to_uri) = MatrixToUri::parse(&*hacked_uri) { + Some(matrix_to_uri.id().to_owned()) } else { None } @@ -260,6 +291,18 @@ mod test { assert!(Mention::from_uri("hello").is_none()); } + #[test] + fn parse_uri_external_room() { + let uri = + "https://custom.custom.com/?secretstuff/#/!roomid:example.org"; + let parsed = Mention::from_uri(uri).unwrap(); + + assert_eq!(parsed.uri(), uri); + assert_eq!(parsed.mx_id(), "!roomid:example.org"); + assert_eq!(parsed.display_text(), "!roomid:example.org"); + assert_eq!(parsed.kind(), &MentionKind::Room); + } + #[test] fn parse_link_user_text() { let uri = "https://matrix.to/#/@alice:example.org"; @@ -284,7 +327,7 @@ mod test { assert_eq!(parsed.uri(), uri); assert_eq!(parsed.mx_id(), "!room:example.org"); - assert_eq!(parsed.display_text(), "!room:example.org"); // note the display_text is overridden + assert_eq!(parsed.display_text(), "!room:example.org"); assert_eq!(parsed.kind(), &MentionKind::Room); } From 09e6c5d2c0258adee4ce27473c09264dbffb35cb Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 10:40:35 +0100 Subject: [PATCH 27/56] add test and get it passing --- crates/matrix_mentions/src/mention.rs | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/crates/matrix_mentions/src/mention.rs b/crates/matrix_mentions/src/mention.rs index 26a47e942..29f932079 100644 --- a/crates/matrix_mentions/src/mention.rs +++ b/crates/matrix_mentions/src/mention.rs @@ -14,6 +14,8 @@ use ruma_common::{matrix_uri::MatrixId, MatrixToUri, MatrixUri, UserId}; +const MATRIX_TO_BASE_URL: &str = "https://matrix.to/#/"; + #[derive(Clone, Debug, PartialEq, Eq)] pub struct Mention { uri: String, @@ -173,15 +175,17 @@ fn parse_matrix_id(uri: &str) -> Option { fn parse_external_id(uri: &str) -> Option { // first split the string into the parts we need let parts: Vec<&str> = uri.split("/#/").collect(); - let before_hash = parts[0]; + + // we expect this to split the uri into exactly two parts, if it's anything else, return early + if parts.len() != 2 { + return None; + } let after_hash = parts[1]; // now rebuild the uri and try again... - let hacked_uri = format!("{}{}", "https://matrix.to/#/", after_hash); + let hacked_uri = format!("{}{}", MATRIX_TO_BASE_URL, after_hash); - if let Ok(matrix_uri) = MatrixUri::parse(&*hacked_uri) { - Some(matrix_uri.id().to_owned()) - } else if let Ok(matrix_to_uri) = MatrixToUri::parse(&*hacked_uri) { + if let Ok(matrix_to_uri) = MatrixToUri::parse(&hacked_uri) { Some(matrix_to_uri.id().to_owned()) } else { None @@ -291,6 +295,17 @@ mod test { assert!(Mention::from_uri("hello").is_none()); } + #[test] + fn parse_uri_external_user() { + let uri = "https://custom.custom.com/?secretstuff/#/@alice:example.org"; + let parsed = Mention::from_uri(uri).unwrap(); + + assert_eq!(parsed.uri(), uri); + assert_eq!(parsed.mx_id(), "@alice:example.org"); + assert_eq!(parsed.display_text(), "@alice:example.org"); + assert_eq!(parsed.kind(), &MentionKind::User); + } + #[test] fn parse_uri_external_room() { let uri = From e28717c47b2368f57d24cb7a7d10169bb667409d Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 10:49:07 +0100 Subject: [PATCH 28/56] tidy up code --- crates/matrix_mentions/src/mention.rs | 28 +++++++++++++-------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/crates/matrix_mentions/src/mention.rs b/crates/matrix_mentions/src/mention.rs index 29f932079..1f3d0d321 100644 --- a/crates/matrix_mentions/src/mention.rs +++ b/crates/matrix_mentions/src/mention.rs @@ -12,7 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -use ruma_common::{matrix_uri::MatrixId, MatrixToUri, MatrixUri, UserId}; +use ruma_common::{ + matrix_uri::MatrixId, IdParseError, MatrixToUri, MatrixUri, UserId, +}; const MATRIX_TO_BASE_URL: &str = "https://matrix.to/#/"; @@ -161,35 +163,31 @@ fn parse_matrix_id(uri: &str) -> Option { Some(matrix_uri.id().to_owned()) } else if let Ok(matrix_to_uri) = MatrixToUri::parse(uri) { Some(matrix_to_uri.id().to_owned()) - } else if let Some(matrix_id) = parse_external_id(uri) { - Some(matrix_id.to_owned()) + } else if let Ok(matrix_to_uri) = parse_external_id(uri) { + Some(matrix_to_uri.id().to_owned()) } else { None } } -/// Splits an external uri on `/#/` and then attempts to find the relevant matrix information -/// from after the split +/// Attempts to split an external id on `/#/`, rebuild as a matrix to style permalink then parse +/// using ruma. /// -/// If successful return Some. Else return None. -fn parse_external_id(uri: &str) -> Option { +/// Returns the result of calling `parse` in ruma. +fn parse_external_id(uri: &str) -> Result { // first split the string into the parts we need let parts: Vec<&str> = uri.split("/#/").collect(); // we expect this to split the uri into exactly two parts, if it's anything else, return early if parts.len() != 2 { - return None; + return Err(IdParseError::Empty); } let after_hash = parts[1]; - // now rebuild the uri and try again... - let hacked_uri = format!("{}{}", MATRIX_TO_BASE_URL, after_hash); + // now rebuild the string as if it were a matrix to type link, then use ruma to parse + let uri_for_ruma = format!("{}{}", MATRIX_TO_BASE_URL, after_hash); - if let Ok(matrix_to_uri) = MatrixToUri::parse(&hacked_uri) { - Some(matrix_to_uri.id().to_owned()) - } else { - None - } + MatrixToUri::parse(&uri_for_ruma) } #[cfg(test)] From dde0295ff592800f6fcce1e1eb0c60a4dd37ffde Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 10:54:35 +0100 Subject: [PATCH 29/56] add more tests --- crates/wysiwyg/src/tests/test_mentions.rs | 25 +++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/crates/wysiwyg/src/tests/test_mentions.rs b/crates/wysiwyg/src/tests/test_mentions.rs index eee3ae215..e76162843 100644 --- a/crates/wysiwyg/src/tests/test_mentions.rs +++ b/crates/wysiwyg/src/tests/test_mentions.rs @@ -28,6 +28,31 @@ fn inserting_with_invalid_mention_url_does_nothing() { assert_eq!(tx(&model), "|"); } +/** + * INSERTING EXTERNAL LINKS + */ +#[test] +fn inserting_with_external_user_works() { + let mut model = cm("|"); + model.insert_mention( + "https://custom.custom.com/?secretstuff/#/@alice:example.org".into(), + "@Alice".into(), + vec![], + ); + assert_eq!(tx(&model), "@Alice |"); +} + +#[test] +fn inserting_with_external_room_works() { + let mut model = cm("|"); + model.insert_mention( + "https://custom.custom.com/?secretstuff/#/!roomid:example.org".into(), + "some room".into(), + vec![], + ); + assert_eq!(tx(&model), "some room |"); +} + /** * ATTRIBUTE TESTS */ From e383123c0ef44f3abcb5df9b7f9d4239c260d041 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 10:58:43 +0100 Subject: [PATCH 30/56] fix clippy error --- crates/matrix_mentions/src/mention.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/matrix_mentions/src/mention.rs b/crates/matrix_mentions/src/mention.rs index 1f3d0d321..b8179ddb5 100644 --- a/crates/matrix_mentions/src/mention.rs +++ b/crates/matrix_mentions/src/mention.rs @@ -12,9 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use ruma_common::{ - matrix_uri::MatrixId, IdParseError, MatrixToUri, MatrixUri, UserId, -}; +use ruma_common::{matrix_uri::MatrixId, IdParseError, MatrixToUri, MatrixUri}; const MATRIX_TO_BASE_URL: &str = "https://matrix.to/#/"; From 4c08a5f76b24664f09499af70bb3b0ba15b442d1 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 11:09:30 +0100 Subject: [PATCH 31/56] reinstate removed comment --- crates/matrix_mentions/src/mention.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/matrix_mentions/src/mention.rs b/crates/matrix_mentions/src/mention.rs index b8179ddb5..c3d90d114 100644 --- a/crates/matrix_mentions/src/mention.rs +++ b/crates/matrix_mentions/src/mention.rs @@ -338,7 +338,7 @@ mod test { assert_eq!(parsed.uri(), uri); assert_eq!(parsed.mx_id(), "!room:example.org"); - assert_eq!(parsed.display_text(), "!room:example.org"); + assert_eq!(parsed.display_text(), "!room:example.org"); // note the display_text is overridden assert_eq!(parsed.kind(), &MentionKind::Room); } From a2e336146bac9ba49d380c279f7813345508f66d Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 13:44:06 +0100 Subject: [PATCH 32/56] separate out user and room in markdown --- crates/wysiwyg/src/dom/nodes/mention_node.rs | 4 ++-- crates/wysiwyg/src/tests/test_to_markdown.rs | 12 ++++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/crates/wysiwyg/src/dom/nodes/mention_node.rs b/crates/wysiwyg/src/dom/nodes/mention_node.rs index ed494e34a..bbb319411 100644 --- a/crates/wysiwyg/src/dom/nodes/mention_node.rs +++ b/crates/wysiwyg/src/dom/nodes/mention_node.rs @@ -294,8 +294,8 @@ where { let text = match this.kind() { // for User/Room type, we use the mx_id in the md output - MentionNodeKind::User { mention } - | MentionNodeKind::Room { mention } => S::from(mention.mx_id()), + MentionNodeKind::User { .. } => this.display_text(), + MentionNodeKind::Room { mention } => S::from(mention.mx_id()), MentionNodeKind::AtRoom => this.display_text(), }; diff --git a/crates/wysiwyg/src/tests/test_to_markdown.rs b/crates/wysiwyg/src/tests/test_to_markdown.rs index a4b2d895b..e2921ec12 100644 --- a/crates/wysiwyg/src/tests/test_to_markdown.rs +++ b/crates/wysiwyg/src/tests/test_to_markdown.rs @@ -204,10 +204,18 @@ fn list_ordered_and_unordered() { } #[test] -fn mention() { +fn user_mention() { assert_to_md_no_roundtrip( r#"test"#, - r#"@alice:matrix.org"#, + r#"test"#, + ); +} + +#[test] +fn room_mention() { + assert_to_md_no_roundtrip( + r#"test"#, + r#"#alice:matrix.org"#, ); } From cacc965b9304912c26c25c4f374ef2755240a9d1 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 14:02:16 +0100 Subject: [PATCH 33/56] extract AT_ROOM to constant in other crate --- crates/matrix_mentions/src/lib.rs | 2 +- crates/matrix_mentions/src/mention.rs | 1 + crates/wysiwyg/src/dom/nodes/mention_node.rs | 6 +++--- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/matrix_mentions/src/lib.rs b/crates/matrix_mentions/src/lib.rs index 09b6ae493..4cf7659fe 100644 --- a/crates/matrix_mentions/src/lib.rs +++ b/crates/matrix_mentions/src/lib.rs @@ -14,4 +14,4 @@ mod mention; -pub use crate::mention::{Mention, MentionKind}; +pub use crate::mention::{Mention, MentionKind, AT_ROOM}; diff --git a/crates/matrix_mentions/src/mention.rs b/crates/matrix_mentions/src/mention.rs index c3d90d114..0281d81eb 100644 --- a/crates/matrix_mentions/src/mention.rs +++ b/crates/matrix_mentions/src/mention.rs @@ -15,6 +15,7 @@ use ruma_common::{matrix_uri::MatrixId, IdParseError, MatrixToUri, MatrixUri}; const MATRIX_TO_BASE_URL: &str = "https://matrix.to/#/"; +pub const AT_ROOM: &str = "@room"; #[derive(Clone, Debug, PartialEq, Eq)] pub struct Mention { diff --git a/crates/wysiwyg/src/dom/nodes/mention_node.rs b/crates/wysiwyg/src/dom/nodes/mention_node.rs index bbb319411..9a3caf492 100644 --- a/crates/wysiwyg/src/dom/nodes/mention_node.rs +++ b/crates/wysiwyg/src/dom/nodes/mention_node.rs @@ -11,7 +11,7 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. -use matrix_mentions::{Mention, MentionKind}; +use matrix_mentions::{Mention, MentionKind, AT_ROOM}; use crate::composer_model::example_format::SelectionWriter; use crate::dom::dom_handle::DomHandle; @@ -129,12 +129,12 @@ where /// Util function to check if the display text is that of an at-room mention pub fn is_at_room_display_text(text: &S) -> bool { - text == &S::from("@room") + text == &S::from(AT_ROOM) } /// Util function to get the display text for an at-room mention pub fn get_at_room_display_text() -> S { - S::from("@room") + S::from(AT_ROOM) } } From 66ede7e067718b0e8f1852b198370f46e2f9bece Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 14:08:25 +0100 Subject: [PATCH 34/56] extract at room utils to matrix mention crate --- crates/matrix_mentions/src/lib.rs | 5 ++++- crates/matrix_mentions/src/mention.rs | 10 +++++++++ crates/wysiwyg/src/composer_model/mentions.rs | 4 ++-- crates/wysiwyg/src/dom/nodes/dom_node.rs | 4 +++- crates/wysiwyg/src/dom/nodes/mention_node.rs | 22 +++++++++---------- 5 files changed, 30 insertions(+), 15 deletions(-) diff --git a/crates/matrix_mentions/src/lib.rs b/crates/matrix_mentions/src/lib.rs index 4cf7659fe..e457cd247 100644 --- a/crates/matrix_mentions/src/lib.rs +++ b/crates/matrix_mentions/src/lib.rs @@ -14,4 +14,7 @@ mod mention; -pub use crate::mention::{Mention, MentionKind, AT_ROOM}; +pub use crate::mention::{ + get_at_room_display_text, is_at_room_display_text, Mention, MentionKind, + AT_ROOM, +}; diff --git a/crates/matrix_mentions/src/mention.rs b/crates/matrix_mentions/src/mention.rs index 0281d81eb..c6b5548d6 100644 --- a/crates/matrix_mentions/src/mention.rs +++ b/crates/matrix_mentions/src/mention.rs @@ -17,6 +17,16 @@ use ruma_common::{matrix_uri::MatrixId, IdParseError, MatrixToUri, MatrixUri}; const MATRIX_TO_BASE_URL: &str = "https://matrix.to/#/"; pub const AT_ROOM: &str = "@room"; +/// Util function to check if the display text is that of an at-room mention +pub fn is_at_room_display_text(text: &str) -> bool { + text == AT_ROOM +} + +/// Util function to get the display text for an at-room mention +pub fn get_at_room_display_text() -> &'static str { + AT_ROOM +} + #[derive(Clone, Debug, PartialEq, Eq)] pub struct Mention { uri: String, diff --git a/crates/wysiwyg/src/composer_model/mentions.rs b/crates/wysiwyg/src/composer_model/mentions.rs index bbff20315..1c022a678 100644 --- a/crates/wysiwyg/src/composer_model/mentions.rs +++ b/crates/wysiwyg/src/composer_model/mentions.rs @@ -11,7 +11,7 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. -use matrix_mentions::Mention; +use matrix_mentions::{is_at_room_display_text, Mention}; use crate::{ dom::{nodes::MentionNode, DomLocation}, @@ -123,7 +123,7 @@ where // when we have an at-room mention, it doesn't matter about the url as we do not use // it, rendering the mention as raw text in the html output - if MentionNode::is_at_room_display_text(text) { + if is_at_room_display_text(text.to_string().as_str()) { range_contains_link_or_code_leaves } else { invalid_uri || range_contains_link_or_code_leaves diff --git a/crates/wysiwyg/src/dom/nodes/dom_node.rs b/crates/wysiwyg/src/dom/nodes/dom_node.rs index de2378bd9..9cd58f743 100644 --- a/crates/wysiwyg/src/dom/nodes/dom_node.rs +++ b/crates/wysiwyg/src/dom/nodes/dom_node.rs @@ -1,3 +1,5 @@ +use matrix_mentions::is_at_room_display_text; + // Copyright 2022 The Matrix.org Foundation C.I.C. // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -148,7 +150,7 @@ where attributes: Vec<(S, S)>, ) -> Result, UriParseError> { // special case for at-room - if MentionNode::is_at_room_display_text(&display_text) { + if is_at_room_display_text(display_text.to_string().as_str()) { Ok(DomNode::Mention(MentionNode::new_at_room(attributes))) } else if let Ok(mention_node) = MentionNode::new(url, display_text, attributes) diff --git a/crates/wysiwyg/src/dom/nodes/mention_node.rs b/crates/wysiwyg/src/dom/nodes/mention_node.rs index 9a3caf492..46e306f2c 100644 --- a/crates/wysiwyg/src/dom/nodes/mention_node.rs +++ b/crates/wysiwyg/src/dom/nodes/mention_node.rs @@ -11,7 +11,7 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. -use matrix_mentions::{Mention, MentionKind, AT_ROOM}; +use matrix_mentions::{get_at_room_display_text, Mention, MentionKind}; use crate::composer_model::example_format::SelectionWriter; use crate::dom::dom_handle::DomHandle; @@ -89,7 +89,7 @@ where let handle = DomHandle::new_unset(); Self { - display_text: MentionNode::get_at_room_display_text(), + display_text: S::from(get_at_room_display_text()), kind: MentionNodeKind::AtRoom, attributes, handle, @@ -105,7 +105,7 @@ where MentionNodeKind::User { .. } | MentionNodeKind::Room { .. } => { self.display_text.clone() } - MentionNodeKind::AtRoom => MentionNode::get_at_room_display_text(), + MentionNodeKind::AtRoom => S::from(get_at_room_display_text()), } } @@ -127,15 +127,15 @@ where &self.kind } - /// Util function to check if the display text is that of an at-room mention - pub fn is_at_room_display_text(text: &S) -> bool { - text == &S::from(AT_ROOM) - } + // /// Util function to check if the display text is that of an at-room mention + // pub fn is_at_room_display_text(text: &S) -> bool { + // text == &S::from(AT_ROOM) + // } - /// Util function to get the display text for an at-room mention - pub fn get_at_room_display_text() -> S { - S::from(AT_ROOM) - } + // /// Util function to get the display text for an at-room mention + // pub fn get_at_room_display_text() -> S { + // S::from(AT_ROOM) + // } } impl ToHtml for MentionNode From 6e90343b1f48a902236ecc309b0f46179d9fbd83 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 14:46:08 +0100 Subject: [PATCH 35/56] first hack at splitting at-room behaviour --- bindings/wysiwyg-wasm/src/lib.rs | 23 ++++++ crates/wysiwyg/src/composer_model/mentions.rs | 79 ++++++++++++++++++- 2 files changed, 99 insertions(+), 3 deletions(-) diff --git a/bindings/wysiwyg-wasm/src/lib.rs b/bindings/wysiwyg-wasm/src/lib.rs index dd865334b..8b55aefd9 100644 --- a/bindings/wysiwyg-wasm/src/lib.rs +++ b/bindings/wysiwyg-wasm/src/lib.rs @@ -312,6 +312,16 @@ impl ComposerModel { )) } + /// Creates an at-room mention node and inserts it into the composer at the current selection + pub fn insert_at_room( + &mut self, + attributes: js_sys::Map, + ) -> ComposerUpdate { + ComposerUpdate::from( + self.inner.insert_at_room_mention(attributes.into_vec()), + ) + } + /// Creates a mention node and inserts it into the composer at the current selection pub fn insert_mention( &mut self, @@ -326,6 +336,19 @@ impl ComposerModel { )) } + /// Creates an at-room mention node and inserts it into the composer, replacing the + /// text content defined by the suggestion + pub fn insert_at_room_at_suggestion( + &mut self, + suggestion: &SuggestionPattern, + attributes: js_sys::Map, + ) -> ComposerUpdate { + ComposerUpdate::from(self.inner.insert_at_room_mention_at_suggestion( + wysiwyg::SuggestionPattern::from(suggestion.clone()), + attributes.into_vec(), + )) + } + /// Creates a mention node and inserts it into the composer, replacing the /// text content defined by the suggestion pub fn insert_mention_at_suggestion( diff --git a/crates/wysiwyg/src/composer_model/mentions.rs b/crates/wysiwyg/src/composer_model/mentions.rs index 1c022a678..eb926c8c8 100644 --- a/crates/wysiwyg/src/composer_model/mentions.rs +++ b/crates/wysiwyg/src/composer_model/mentions.rs @@ -14,9 +14,8 @@ use matrix_mentions::{is_at_room_display_text, Mention}; use crate::{ - dom::{nodes::MentionNode, DomLocation}, - ComposerModel, ComposerUpdate, DomNode, Location, SuggestionPattern, - UnicodeString, + dom::DomLocation, ComposerModel, ComposerUpdate, DomNode, Location, + SuggestionPattern, UnicodeString, }; impl ComposerModel @@ -47,6 +46,22 @@ where self.do_insert_mention(url, text, attributes) } + pub fn insert_at_room_mention_at_suggestion( + &mut self, + suggestion: SuggestionPattern, + attributes: Vec<(S, S)>, + ) -> ComposerUpdate { + if self.should_not_insert_at_room_mention() { + return ComposerUpdate::keep(); + } + + self.push_state_to_history(); + self.do_replace_text_in(S::default(), suggestion.start, suggestion.end); + self.state.start = Location::from(suggestion.start); + self.state.end = self.state.start; + self.do_insert_at_room_mention(attributes) + } + /// Inserts a mention into the composer. It uses the following rules: /// - Do not insert a mention if the uri is invalid /// - Do not insert a mention if the range includes link or code leaves @@ -70,6 +85,22 @@ where self.do_insert_mention(url, text, attributes) } + pub fn insert_at_room_mention( + &mut self, + attributes: Vec<(S, S)>, + ) -> ComposerUpdate { + if self.should_not_insert_at_room_mention() { + return ComposerUpdate::keep(); + } + + self.push_state_to_history(); + if self.has_selection() { + self.do_replace_text(S::default()); + } + + self.do_insert_at_room_mention(attributes) + } + /// Creates a new mention node then inserts the node at the cursor position. If creation fails due to /// an invalid uri, it will return `ComposerUpdate::keep()`. /// It adds a trailing space when the inserted mention is the last node in it's parent. @@ -102,6 +133,34 @@ where } } + /// Creates a new mention node then inserts the node at the cursor position. If creation fails due to + /// an invalid uri, it will return `ComposerUpdate::keep()`. + /// It adds a trailing space when the inserted mention is the last node in it's parent. + fn do_insert_at_room_mention( + &mut self, + attributes: Vec<(S, S)>, + ) -> ComposerUpdate { + let (start, end) = self.safe_selection(); + let range = self.state.dom.find_range(start, end); + + let new_node = DomNode::new_at_room_mention(attributes); + + let new_cursor_index = start + new_node.text_len(); + + let handle = self.state.dom.insert_node_at_cursor(&range, new_node); + + // manually move the cursor to the end of the mention + self.state.start = Location::from(new_cursor_index); + self.state.end = self.state.start; + + // add a trailing space in cases when we do not have a next sibling + if self.state.dom.is_last_in_parent(&handle) { + self.do_replace_text(" ".into()) + } else { + self.create_update_replace_all() + } + } + /// Utility function for the insert_mention* methods. It returns false if: /// - the range includes any link or code type leaves /// - the url is not a valid matrix uri (with special case for at-room) @@ -129,4 +188,18 @@ where invalid_uri || range_contains_link_or_code_leaves } } + + fn should_not_insert_at_room_mention(&self) -> bool { + let (start, end) = self.safe_selection(); + let range = self.state.dom.find_range(start, end); + + let range_contains_link_or_code_leaves = + range.locations.iter().any(|l: &DomLocation| { + l.kind.is_link_kind() || l.kind.is_code_kind() + }); + + // when we have an at-room mention, it doesn't matter about the url as we do not use + // it, rendering the mention as raw text in the html output + range_contains_link_or_code_leaves + } } From 38dc7cc06a84515ea423e051b98418143c0d7176 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 14:47:58 +0100 Subject: [PATCH 36/56] split dom node methods apart --- crates/wysiwyg/src/dom/nodes/dom_node.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/wysiwyg/src/dom/nodes/dom_node.rs b/crates/wysiwyg/src/dom/nodes/dom_node.rs index 9cd58f743..58f4a13ff 100644 --- a/crates/wysiwyg/src/dom/nodes/dom_node.rs +++ b/crates/wysiwyg/src/dom/nodes/dom_node.rs @@ -149,10 +149,7 @@ where display_text: S, attributes: Vec<(S, S)>, ) -> Result, UriParseError> { - // special case for at-room - if is_at_room_display_text(display_text.to_string().as_str()) { - Ok(DomNode::Mention(MentionNode::new_at_room(attributes))) - } else if let Ok(mention_node) = + if let Ok(mention_node) = MentionNode::new(url, display_text, attributes) { Ok(DomNode::Mention(mention_node)) From e46895ea4a3d6446b7b8fc50ce8e9caf343b403b Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 14:52:23 +0100 Subject: [PATCH 37/56] fix broken tests --- crates/wysiwyg/src/tests/test_to_message_html.rs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/crates/wysiwyg/src/tests/test_to_message_html.rs b/crates/wysiwyg/src/tests/test_to_message_html.rs index aeb9e4577..54037a0b0 100644 --- a/crates/wysiwyg/src/tests/test_to_message_html.rs +++ b/crates/wysiwyg/src/tests/test_to_message_html.rs @@ -75,15 +75,8 @@ fn only_outputs_href_attribute_on_room_mention_and_uses_mx_id() { #[test] fn only_outputs_href_inner_text_for_at_room_mention() { let mut model = cm("|"); - model.insert_mention( - "anything".into(), // this should be ignored in favour of a # placeholder - "@room".into(), - vec![ - ("data-mention-type".into(), "at-room".into()), - ("style".into(), "some css".into()), - ], - ); - assert_eq!(tx(&model), "@room |"); + model.insert_at_room_mention(vec![("style".into(), "some css".into())]); + assert_eq!(tx(&model), "@room |"); let message_output = model.get_content_as_message_html(); assert_eq!(message_output, "@room\u{a0}"); From 554c049f010d35958dd2f24d36a73f3bc67c5f15 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 14:59:09 +0100 Subject: [PATCH 38/56] remove unused import --- crates/wysiwyg/src/dom/nodes/dom_node.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/wysiwyg/src/dom/nodes/dom_node.rs b/crates/wysiwyg/src/dom/nodes/dom_node.rs index 58f4a13ff..16a703844 100644 --- a/crates/wysiwyg/src/dom/nodes/dom_node.rs +++ b/crates/wysiwyg/src/dom/nodes/dom_node.rs @@ -1,5 +1,3 @@ -use matrix_mentions::is_at_room_display_text; - // Copyright 2022 The Matrix.org Foundation C.I.C. // // Licensed under the Apache License, Version 2.0 (the "License"); From 3720b6caa748d2f87a4a8159afc492f627bdd609 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 14:59:22 +0100 Subject: [PATCH 39/56] add special case for example app --- platforms/web/lib/composer.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/platforms/web/lib/composer.ts b/platforms/web/lib/composer.ts index db450ed27..bed08ada2 100644 --- a/platforms/web/lib/composer.ts +++ b/platforms/web/lib/composer.ts @@ -76,6 +76,16 @@ export function processInput( const { text, url, attributes } = event.data; const attributesMap = new Map(Object.entries(attributes)); + if (text === '@room' && url === '#') { + return action( + composerModel.insert_at_room_at_suggestion( + suggestion, + attributesMap, + ), + 'insert_at_room_mention_at_suggestion', + ); + } + return action( composerModel.insert_mention_at_suggestion( url, From c5802f45088b55fb12e9705afb43e354a7b407d0 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 15:03:27 +0100 Subject: [PATCH 40/56] change function call --- platforms/web/lib/composer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platforms/web/lib/composer.ts b/platforms/web/lib/composer.ts index bed08ada2..970788c9b 100644 --- a/platforms/web/lib/composer.ts +++ b/platforms/web/lib/composer.ts @@ -78,7 +78,7 @@ export function processInput( if (text === '@room' && url === '#') { return action( - composerModel.insert_at_room_at_suggestion( + composerModel.insert_at_room_mention_at_suggestion( suggestion, attributesMap, ), From c946c9422adfcd208c767c253dd208b002af1f82 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 15:03:35 +0100 Subject: [PATCH 41/56] change function name --- bindings/wysiwyg-wasm/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bindings/wysiwyg-wasm/src/lib.rs b/bindings/wysiwyg-wasm/src/lib.rs index 8b55aefd9..042720961 100644 --- a/bindings/wysiwyg-wasm/src/lib.rs +++ b/bindings/wysiwyg-wasm/src/lib.rs @@ -313,7 +313,7 @@ impl ComposerModel { } /// Creates an at-room mention node and inserts it into the composer at the current selection - pub fn insert_at_room( + pub fn insert_at_room_mention( &mut self, attributes: js_sys::Map, ) -> ComposerUpdate { @@ -338,7 +338,7 @@ impl ComposerModel { /// Creates an at-room mention node and inserts it into the composer, replacing the /// text content defined by the suggestion - pub fn insert_at_room_at_suggestion( + pub fn insert_at_room_mention_at_suggestion( &mut self, suggestion: &SuggestionPattern, attributes: js_sys::Map, From b6b07bdeb929c78727715a60ab9c0870880e4ff4 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 15:06:52 +0100 Subject: [PATCH 42/56] read new functions across to mobile --- .../wysiwyg-ffi/src/ffi_composer_model.rs | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/bindings/wysiwyg-ffi/src/ffi_composer_model.rs b/bindings/wysiwyg-ffi/src/ffi_composer_model.rs index 71ca4c26f..9909455bf 100644 --- a/bindings/wysiwyg-ffi/src/ffi_composer_model.rs +++ b/bindings/wysiwyg-ffi/src/ffi_composer_model.rs @@ -258,6 +258,25 @@ impl ComposerModel { )) } + /// Creates an at-room mention node and inserts it into the composer at the current selection + pub fn insert_at_room_mention( + &mut self, + attributes: Vec, + ) -> Arc { + let attrs = attributes + .iter() + .map(|attr| { + ( + Utf16String::from_str(&attr.key), + Utf16String::from_str(&attr.value), + ) + }) + .collect(); + Arc::new(ComposerUpdate::from( + self.inner.lock().unwrap().insert_at_room_mention(attrs), + )) + } + /// Creates a mention node and inserts it into the composer at the current selection pub fn insert_mention( self: &Arc, @@ -281,6 +300,31 @@ impl ComposerModel { )) } + /// Creates an at-room mention node and inserts it into the composer, replacing the + /// text content defined by the suggestion + pub fn insert_at_room_mention_at_suggestion( + &mut self, + suggestion: SuggestionPattern, + attributes: Vec, + ) -> Arc { + let suggestion = wysiwyg::SuggestionPattern::from(suggestion); + let attrs = attributes + .iter() + .map(|attr| { + ( + Utf16String::from_str(&attr.key), + Utf16String::from_str(&attr.value), + ) + }) + .collect(); + Arc::new(ComposerUpdate::from( + self.inner + .lock() + .unwrap() + .insert_at_room_mention_at_suggestion(suggestion, attrs), + )) + } + /// Creates a mention node and inserts it into the composer, replacing the /// text content defined by the suggestion pub fn insert_mention_at_suggestion( From ca571d70b11cda1ca20ea080ca0dd7e2e7544f3b Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 15:14:25 +0100 Subject: [PATCH 43/56] add test --- crates/wysiwyg/src/tests/test_mentions.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/crates/wysiwyg/src/tests/test_mentions.rs b/crates/wysiwyg/src/tests/test_mentions.rs index e76162843..82027279f 100644 --- a/crates/wysiwyg/src/tests/test_mentions.rs +++ b/crates/wysiwyg/src/tests/test_mentions.rs @@ -555,6 +555,16 @@ fn selection_paragraph_spanning() { ); } +/** + * AT-ROOM + */ +#[test] +fn can_insert_at_room_mention() { + let mut model = cm("|"); + model.insert_at_room_mention(vec![("style".into(), "some css".into())]); + assert_eq!(tx(&model), "@room |") +} + /** * HELPER FUNCTIONS */ From a1aaf8b7a1d073f6d3c0401b85f143ba41e08ef6 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 15:16:25 +0100 Subject: [PATCH 44/56] remove unused code --- crates/matrix_mentions/src/lib.rs | 3 +-- crates/matrix_mentions/src/mention.rs | 5 ----- crates/wysiwyg/src/composer_model/mentions.rs | 8 ++------ 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/crates/matrix_mentions/src/lib.rs b/crates/matrix_mentions/src/lib.rs index e457cd247..f22fe5e26 100644 --- a/crates/matrix_mentions/src/lib.rs +++ b/crates/matrix_mentions/src/lib.rs @@ -15,6 +15,5 @@ mod mention; pub use crate::mention::{ - get_at_room_display_text, is_at_room_display_text, Mention, MentionKind, - AT_ROOM, + get_at_room_display_text, Mention, MentionKind, AT_ROOM, }; diff --git a/crates/matrix_mentions/src/mention.rs b/crates/matrix_mentions/src/mention.rs index c6b5548d6..3c45f25bd 100644 --- a/crates/matrix_mentions/src/mention.rs +++ b/crates/matrix_mentions/src/mention.rs @@ -17,11 +17,6 @@ use ruma_common::{matrix_uri::MatrixId, IdParseError, MatrixToUri, MatrixUri}; const MATRIX_TO_BASE_URL: &str = "https://matrix.to/#/"; pub const AT_ROOM: &str = "@room"; -/// Util function to check if the display text is that of an at-room mention -pub fn is_at_room_display_text(text: &str) -> bool { - text == AT_ROOM -} - /// Util function to get the display text for an at-room mention pub fn get_at_room_display_text() -> &'static str { AT_ROOM diff --git a/crates/wysiwyg/src/composer_model/mentions.rs b/crates/wysiwyg/src/composer_model/mentions.rs index eb926c8c8..4b1b4c520 100644 --- a/crates/wysiwyg/src/composer_model/mentions.rs +++ b/crates/wysiwyg/src/composer_model/mentions.rs @@ -11,7 +11,7 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. -use matrix_mentions::{is_at_room_display_text, Mention}; +use matrix_mentions::Mention; use crate::{ dom::DomLocation, ComposerModel, ComposerUpdate, DomNode, Location, @@ -182,11 +182,7 @@ where // when we have an at-room mention, it doesn't matter about the url as we do not use // it, rendering the mention as raw text in the html output - if is_at_room_display_text(text.to_string().as_str()) { - range_contains_link_or_code_leaves - } else { - invalid_uri || range_contains_link_or_code_leaves - } + invalid_uri || range_contains_link_or_code_leaves } fn should_not_insert_at_room_mention(&self) -> bool { From 393d0d936955580cb664ab06d0af818a75261d77 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 15:26:15 +0100 Subject: [PATCH 45/56] reduce repeated code --- crates/wysiwyg/src/composer_model/mentions.rs | 49 ++++++++----------- 1 file changed, 21 insertions(+), 28 deletions(-) diff --git a/crates/wysiwyg/src/composer_model/mentions.rs b/crates/wysiwyg/src/composer_model/mentions.rs index 4b1b4c520..cfaf62e9d 100644 --- a/crates/wysiwyg/src/composer_model/mentions.rs +++ b/crates/wysiwyg/src/composer_model/mentions.rs @@ -23,8 +23,8 @@ where S: UnicodeString, { /// Remove the suggestion text and then insert a mention into the composer, using the following rules - /// - Do not insert a mention if the uri is invalid - /// - Do not insert a mention if the range includes link or code leaves + /// - Do not do anthing if the uri is invalid + /// - Do not do anthing if the range includes link or code leaves /// - If the composer contains a selection, remove the contents of the selection /// prior to inserting a mention at the cursor. /// - If the composer contains a cursor, insert a mention at the cursor @@ -35,7 +35,7 @@ where suggestion: SuggestionPattern, attributes: Vec<(S, S)>, ) -> ComposerUpdate { - if self.should_not_insert_mention(&url, &text) { + if self.should_not_insert_mention(&url) { return ComposerUpdate::keep(); } @@ -46,6 +46,11 @@ where self.do_insert_mention(url, text, attributes) } + /// Remove the suggestion text and then insert an at-room mention into the composer, using the following rules + /// - Do not do anthing if the range includes link or code leaves + /// - If the composer contains a selection, remove the contents of the selection + /// prior to inserting a mention at the cursor. + /// - If the composer contains a cursor, insert a mention at the cursor pub fn insert_at_room_mention_at_suggestion( &mut self, suggestion: SuggestionPattern, @@ -74,7 +79,7 @@ where text: S, attributes: Vec<(S, S)>, ) -> ComposerUpdate { - if self.should_not_insert_mention(&url, &text) { + if self.should_not_insert_mention(&url) { return ComposerUpdate::keep(); } @@ -161,41 +166,29 @@ where } } - /// Utility function for the insert_mention* methods. It returns false if: + /// Utility functions for the insert_mention* methods. It returns false if: /// - the range includes any link or code type leaves - /// - the url is not a valid matrix uri (with special case for at-room) + /// - the url is not a valid matrix uri /// /// Related issue is here: /// https://github.com/matrix-org/matrix-rich-text-editor/issues/702 /// We do not allow mentions to be inserted into links, the planned behaviour is /// detailed in the above issue. - fn should_not_insert_mention(&self, url: &S, text: &S) -> bool { - let (start, end) = self.safe_selection(); - let range = self.state.dom.find_range(start, end); - - let invalid_uri = !Mention::is_valid_uri(url.to_string().as_str()); - - let range_contains_link_or_code_leaves = - range.locations.iter().any(|l: &DomLocation| { - l.kind.is_link_kind() || l.kind.is_code_kind() - }); - - // when we have an at-room mention, it doesn't matter about the url as we do not use - // it, rendering the mention as raw text in the html output - invalid_uri || range_contains_link_or_code_leaves + fn should_not_insert_mention(&self, url: &S) -> bool { + !Mention::is_valid_uri(url.to_string().as_str()) + || self.range_contains_link_or_code_leaves() } fn should_not_insert_at_room_mention(&self) -> bool { + self.range_contains_link_or_code_leaves() + } + + fn range_contains_link_or_code_leaves(&self) -> bool { let (start, end) = self.safe_selection(); let range = self.state.dom.find_range(start, end); - let range_contains_link_or_code_leaves = - range.locations.iter().any(|l: &DomLocation| { - l.kind.is_link_kind() || l.kind.is_code_kind() - }); - - // when we have an at-room mention, it doesn't matter about the url as we do not use - // it, rendering the mention as raw text in the html output - range_contains_link_or_code_leaves + range.locations.iter().any(|l: &DomLocation| { + l.kind.is_link_kind() || l.kind.is_code_kind() + }) } } From afd8efbc1d21a7396742eccc3d59e86f348dd7f1 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 15:36:29 +0100 Subject: [PATCH 46/56] refactor --- crates/wysiwyg/src/composer_model/mentions.rs | 61 +++++++------------ 1 file changed, 22 insertions(+), 39 deletions(-) diff --git a/crates/wysiwyg/src/composer_model/mentions.rs b/crates/wysiwyg/src/composer_model/mentions.rs index cfaf62e9d..5559438f0 100644 --- a/crates/wysiwyg/src/composer_model/mentions.rs +++ b/crates/wysiwyg/src/composer_model/mentions.rs @@ -43,7 +43,12 @@ where self.do_replace_text_in(S::default(), suggestion.start, suggestion.end); self.state.start = Location::from(suggestion.start); self.state.end = self.state.start; - self.do_insert_mention(url, text, attributes) + + if let Ok(mention_node) = DomNode::new_mention(url, text, attributes) { + self.do_insert_mention(mention_node) + } else { + ComposerUpdate::keep() + } } /// Remove the suggestion text and then insert an at-room mention into the composer, using the following rules @@ -64,7 +69,9 @@ where self.do_replace_text_in(S::default(), suggestion.start, suggestion.end); self.state.start = Location::from(suggestion.start); self.state.end = self.state.start; - self.do_insert_at_room_mention(attributes) + + let mention_node = DomNode::new_at_room_mention(attributes); + self.do_insert_mention(mention_node) } /// Inserts a mention into the composer. It uses the following rules: @@ -87,7 +94,12 @@ where if self.has_selection() { self.do_replace_text(S::default()); } - self.do_insert_mention(url, text, attributes) + + if let Ok(mention_node) = DomNode::new_mention(url, text, attributes) { + self.do_insert_mention(mention_node) + } else { + ComposerUpdate::keep() + } } pub fn insert_at_room_mention( @@ -103,7 +115,8 @@ where self.do_replace_text(S::default()); } - self.do_insert_at_room_mention(attributes) + let mention_node = DomNode::new_at_room_mention(attributes); + self.do_insert_mention(mention_node) } /// Creates a new mention node then inserts the node at the cursor position. If creation fails due to @@ -111,48 +124,18 @@ where /// It adds a trailing space when the inserted mention is the last node in it's parent. fn do_insert_mention( &mut self, - url: S, - text: S, - attributes: Vec<(S, S)>, + mention_node: DomNode, ) -> ComposerUpdate { - let (start, end) = self.safe_selection(); - let range = self.state.dom.find_range(start, end); - - if let Ok(new_node) = DomNode::new_mention(url, text, attributes) { - let new_cursor_index = start + new_node.text_len(); - - let handle = self.state.dom.insert_node_at_cursor(&range, new_node); - - // manually move the cursor to the end of the mention - self.state.start = Location::from(new_cursor_index); - self.state.end = self.state.start; - - // add a trailing space in cases when we do not have a next sibling - if self.state.dom.is_last_in_parent(&handle) { - self.do_replace_text(" ".into()) - } else { - self.create_update_replace_all() - } - } else { - ComposerUpdate::keep() + if !mention_node.is_mention_node() { + return ComposerUpdate::keep(); } - } - /// Creates a new mention node then inserts the node at the cursor position. If creation fails due to - /// an invalid uri, it will return `ComposerUpdate::keep()`. - /// It adds a trailing space when the inserted mention is the last node in it's parent. - fn do_insert_at_room_mention( - &mut self, - attributes: Vec<(S, S)>, - ) -> ComposerUpdate { let (start, end) = self.safe_selection(); let range = self.state.dom.find_range(start, end); - let new_node = DomNode::new_at_room_mention(attributes); - - let new_cursor_index = start + new_node.text_len(); + let new_cursor_index = start + mention_node.text_len(); - let handle = self.state.dom.insert_node_at_cursor(&range, new_node); + let handle = self.state.dom.insert_node_at_cursor(&range, mention_node); // manually move the cursor to the end of the mention self.state.start = Location::from(new_cursor_index); From a5f4fd5051551bdf254496d3397b9fa5ad4aa613 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 15:49:55 +0100 Subject: [PATCH 47/56] redo comments --- crates/wysiwyg/src/composer_model/mentions.rs | 97 ++++++++----------- 1 file changed, 42 insertions(+), 55 deletions(-) diff --git a/crates/wysiwyg/src/composer_model/mentions.rs b/crates/wysiwyg/src/composer_model/mentions.rs index 5559438f0..6b9b29e7d 100644 --- a/crates/wysiwyg/src/composer_model/mentions.rs +++ b/crates/wysiwyg/src/composer_model/mentions.rs @@ -22,12 +22,8 @@ impl ComposerModel where S: UnicodeString, { - /// Remove the suggestion text and then insert a mention into the composer, using the following rules - /// - Do not do anthing if the uri is invalid - /// - Do not do anthing if the range includes link or code leaves - /// - If the composer contains a selection, remove the contents of the selection - /// prior to inserting a mention at the cursor. - /// - If the composer contains a cursor, insert a mention at the cursor + /// Checks to see if the mention should be inserted and also if the mention can be created. + /// If both of these checks are passed it will remove the suggestion and then insert a mention. pub fn insert_mention_at_suggestion( &mut self, url: S, @@ -39,23 +35,45 @@ where return ComposerUpdate::keep(); } - self.push_state_to_history(); - self.do_replace_text_in(S::default(), suggestion.start, suggestion.end); - self.state.start = Location::from(suggestion.start); - self.state.end = self.state.start; - if let Ok(mention_node) = DomNode::new_mention(url, text, attributes) { + self.push_state_to_history(); + self.do_replace_text_in( + S::default(), + suggestion.start, + suggestion.end, + ); + self.state.start = Location::from(suggestion.start); + self.state.end = self.state.start; self.do_insert_mention(mention_node) } else { ComposerUpdate::keep() } } - /// Remove the suggestion text and then insert an at-room mention into the composer, using the following rules - /// - Do not do anthing if the range includes link or code leaves - /// - If the composer contains a selection, remove the contents of the selection - /// prior to inserting a mention at the cursor. - /// - If the composer contains a cursor, insert a mention at the cursor + /// Checks to see if the mention should be inserted and also if the mention can be created. + /// If both of these checks are passed it will remove any selection if present and then insert a mention. + pub fn insert_mention( + &mut self, + url: S, + text: S, + attributes: Vec<(S, S)>, + ) -> ComposerUpdate { + if self.should_not_insert_mention(&url) { + return ComposerUpdate::keep(); + } + + if let Ok(mention_node) = DomNode::new_mention(url, text, attributes) { + self.push_state_to_history(); + if self.has_selection() { + self.do_replace_text(S::default()); + } + self.do_insert_mention(mention_node) + } else { + ComposerUpdate::keep() + } + } + /// Checks to see if the at-room mention should be inserted. + /// If so it will remove the suggestion and then insert an at-room mention. pub fn insert_at_room_mention_at_suggestion( &mut self, suggestion: SuggestionPattern, @@ -74,34 +92,8 @@ where self.do_insert_mention(mention_node) } - /// Inserts a mention into the composer. It uses the following rules: - /// - Do not insert a mention if the uri is invalid - /// - Do not insert a mention if the range includes link or code leaves - /// - If the composer contains a selection, remove the contents of the selection - /// prior to inserting a mention at the cursor. - /// - If the composer contains a cursor, insert a mention at the cursor - pub fn insert_mention( - &mut self, - url: S, - text: S, - attributes: Vec<(S, S)>, - ) -> ComposerUpdate { - if self.should_not_insert_mention(&url) { - return ComposerUpdate::keep(); - } - - self.push_state_to_history(); - if self.has_selection() { - self.do_replace_text(S::default()); - } - - if let Ok(mention_node) = DomNode::new_mention(url, text, attributes) { - self.do_insert_mention(mention_node) - } else { - ComposerUpdate::keep() - } - } - + /// Checks to see if the at-room mention should be inserted. + /// If so it will remove any selection if present and then insert an at-room mention. pub fn insert_at_room_mention( &mut self, attributes: Vec<(S, S)>, @@ -119,9 +111,8 @@ where self.do_insert_mention(mention_node) } - /// Creates a new mention node then inserts the node at the cursor position. If creation fails due to - /// an invalid uri, it will return `ComposerUpdate::keep()`. - /// It adds a trailing space when the inserted mention is the last node in it's parent. + /// Inserts the node at the cursor position. It adds a trailing space when the inserted + /// mention is the last node in it's parent. fn do_insert_mention( &mut self, mention_node: DomNode, @@ -149,19 +140,15 @@ where } } - /// Utility functions for the insert_mention* methods. It returns false if: - /// - the range includes any link or code type leaves - /// - the url is not a valid matrix uri - /// - /// Related issue is here: - /// https://github.com/matrix-org/matrix-rich-text-editor/issues/702 - /// We do not allow mentions to be inserted into links, the planned behaviour is - /// detailed in the above issue. + /// We should not insert a mention if the uri is invalid or the range contains link + /// or code leaves. See issue https://github.com/matrix-org/matrix-rich-text-editor/issues/702. fn should_not_insert_mention(&self, url: &S) -> bool { !Mention::is_valid_uri(url.to_string().as_str()) || self.range_contains_link_or_code_leaves() } + /// We should not insert an at-room mention if the range contains link or code leaves. + /// See issue https://github.com/matrix-org/matrix-rich-text-editor/issues/702. fn should_not_insert_at_room_mention(&self) -> bool { self.range_contains_link_or_code_leaves() } From 4d55895d0f01bd4e9e1991b20ccb6620866b459f Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 16:19:45 +0100 Subject: [PATCH 48/56] get all tests passing --- crates/wysiwyg/src/composer_model/mentions.rs | 1 + crates/wysiwyg/src/dom/nodes/mention_node.rs | 53 +++++++------------ 2 files changed, 19 insertions(+), 35 deletions(-) diff --git a/crates/wysiwyg/src/composer_model/mentions.rs b/crates/wysiwyg/src/composer_model/mentions.rs index 6b9b29e7d..90e5f583b 100644 --- a/crates/wysiwyg/src/composer_model/mentions.rs +++ b/crates/wysiwyg/src/composer_model/mentions.rs @@ -72,6 +72,7 @@ where ComposerUpdate::keep() } } + /// Checks to see if the at-room mention should be inserted. /// If so it will remove the suggestion and then insert an at-room mention. pub fn insert_at_room_mention_at_suggestion( diff --git a/crates/wysiwyg/src/dom/nodes/mention_node.rs b/crates/wysiwyg/src/dom/nodes/mention_node.rs index 46e306f2c..1042770ba 100644 --- a/crates/wysiwyg/src/dom/nodes/mention_node.rs +++ b/crates/wysiwyg/src/dom/nodes/mention_node.rs @@ -41,8 +41,7 @@ where #[derive(Clone, Debug, PartialEq, Eq)] pub enum MentionNodeKind { - Room { mention: Mention }, - User { mention: Mention }, + MatrixURI { mention: Mention }, AtRoom, } @@ -66,10 +65,7 @@ where &url.to_string(), &display_text.to_string(), ) { - let kind = match mention.kind() { - MentionKind::Room => MentionNodeKind::Room { mention }, - MentionKind::User => MentionNodeKind::User { mention }, - }; + let kind = MentionNodeKind::MatrixURI { mention }; Ok(Self { display_text, kind, @@ -102,9 +98,7 @@ where pub fn display_text(&self) -> S { match self.kind() { - MentionNodeKind::User { .. } | MentionNodeKind::Room { .. } => { - self.display_text.clone() - } + MentionNodeKind::MatrixURI { .. } => self.display_text.clone(), MentionNodeKind::AtRoom => S::from(get_at_room_display_text()), } } @@ -165,7 +159,7 @@ impl MentionNode { let cur_pos = formatter.len(); match self.kind() { - MentionNodeKind::User { mention } => { + MentionNodeKind::MatrixURI { mention } => { // if formatting as a message, only include the href attribute let attributes = if as_message { vec![("href".into(), S::from(mention.uri()))] @@ -176,24 +170,12 @@ impl MentionNode { attrs }; - self.fmt_tag_open(tag, formatter, &Some(attributes)); - formatter.push(self.display_text()); - self.fmt_tag_close(tag, formatter); - } - MentionNodeKind::Room { mention } => { - // if formatting as a message, only include the href attribute and also use the mx_id as - // the display text - let (attributes, display_text) = if as_message { - ( - vec![("href".into(), S::from(mention.uri()))], - S::from(mention.mx_id()), - ) - } else { - let mut attrs = self.attributes.clone(); - attrs.push(("href".into(), S::from(mention.uri()))); - attrs.push(("contenteditable".into(), "false".into())); - (attrs, self.display_text()) - }; + let display_text = + if as_message && mention.kind() == &MentionKind::Room { + S::from(mention.mx_id()) + } else { + self.display_text() + }; self.fmt_tag_open(tag, formatter, &Some(attributes)); formatter.push(display_text); @@ -251,11 +233,7 @@ where description.push("\""); match self.kind() { - MentionNodeKind::User { mention } => { - description.push(", "); - description.push(S::from(mention.uri())); - } - MentionNodeKind::Room { mention } => { + MentionNodeKind::MatrixURI { mention } => { description.push(", "); description.push(S::from(mention.uri())); } @@ -294,8 +272,13 @@ where { let text = match this.kind() { // for User/Room type, we use the mx_id in the md output - MentionNodeKind::User { .. } => this.display_text(), - MentionNodeKind::Room { mention } => S::from(mention.mx_id()), + MentionNodeKind::MatrixURI { mention } => { + if mention.kind() == &MentionKind::Room { + S::from(mention.mx_id()) + } else { + this.display_text() + } + } MentionNodeKind::AtRoom => this.display_text(), }; From 24e03cb523356b54574995eae0f0030aeafabe47 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 16:34:14 +0100 Subject: [PATCH 49/56] move AT_ROOM back where it belongs --- crates/matrix_mentions/Cargo.toml | 4 +++- crates/matrix_mentions/src/lib.rs | 4 +--- crates/matrix_mentions/src/mention.rs | 6 ------ crates/wysiwyg/src/dom/nodes/mention_node.rs | 8 +++++++- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/crates/matrix_mentions/Cargo.toml b/crates/matrix_mentions/Cargo.toml index 4f80e5825..649c36012 100644 --- a/crates/matrix_mentions/Cargo.toml +++ b/crates/matrix_mentions/Cargo.toml @@ -9,6 +9,8 @@ name = "matrix_mentions" version = "0.1.0" edition = "2021" -[dependencies] +[features] +custom_matrix_urls = [] +[dependencies] ruma-common = "0.11.3" diff --git a/crates/matrix_mentions/src/lib.rs b/crates/matrix_mentions/src/lib.rs index f22fe5e26..09b6ae493 100644 --- a/crates/matrix_mentions/src/lib.rs +++ b/crates/matrix_mentions/src/lib.rs @@ -14,6 +14,4 @@ mod mention; -pub use crate::mention::{ - get_at_room_display_text, Mention, MentionKind, AT_ROOM, -}; +pub use crate::mention::{Mention, MentionKind}; diff --git a/crates/matrix_mentions/src/mention.rs b/crates/matrix_mentions/src/mention.rs index 3c45f25bd..c3d90d114 100644 --- a/crates/matrix_mentions/src/mention.rs +++ b/crates/matrix_mentions/src/mention.rs @@ -15,12 +15,6 @@ use ruma_common::{matrix_uri::MatrixId, IdParseError, MatrixToUri, MatrixUri}; const MATRIX_TO_BASE_URL: &str = "https://matrix.to/#/"; -pub const AT_ROOM: &str = "@room"; - -/// Util function to get the display text for an at-room mention -pub fn get_at_room_display_text() -> &'static str { - AT_ROOM -} #[derive(Clone, Debug, PartialEq, Eq)] pub struct Mention { diff --git a/crates/wysiwyg/src/dom/nodes/mention_node.rs b/crates/wysiwyg/src/dom/nodes/mention_node.rs index 1042770ba..a44834983 100644 --- a/crates/wysiwyg/src/dom/nodes/mention_node.rs +++ b/crates/wysiwyg/src/dom/nodes/mention_node.rs @@ -11,7 +11,7 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. -use matrix_mentions::{get_at_room_display_text, Mention, MentionKind}; +use matrix_mentions::{Mention, MentionKind}; use crate::composer_model::example_format::SelectionWriter; use crate::dom::dom_handle::DomHandle; @@ -23,6 +23,12 @@ use crate::dom::to_tree::ToTree; use crate::dom::unicode_string::{UnicodeStrExt, UnicodeStringExt}; use crate::dom::UnicodeString; +pub const AT_ROOM: &str = "@room"; + +/// Util function to get the display text for an at-room mention +pub fn get_at_room_display_text() -> &'static str { + AT_ROOM +} #[derive(Debug)] pub struct UriParseError; From 2b81075b4d54c94ad5c6213e0b7b47e943064b9c Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 16:41:31 +0100 Subject: [PATCH 50/56] reinstate whitespace --- crates/wysiwyg/src/dom/nodes/dom_node.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/wysiwyg/src/dom/nodes/dom_node.rs b/crates/wysiwyg/src/dom/nodes/dom_node.rs index 16a703844..2cb02f325 100644 --- a/crates/wysiwyg/src/dom/nodes/dom_node.rs +++ b/crates/wysiwyg/src/dom/nodes/dom_node.rs @@ -11,6 +11,7 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. + use crate::composer_model::example_format::SelectionWriter; use crate::dom::dom_handle::DomHandle; use crate::dom::nodes::{ From 98bb0f3f51f52afc26893ddca7cc3ca7c698a867 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 17:04:48 +0100 Subject: [PATCH 51/56] correct bindings error, update udl --- bindings/wysiwyg-ffi/src/ffi_composer_model.rs | 4 ++-- bindings/wysiwyg-ffi/src/wysiwyg_composer.udl | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/bindings/wysiwyg-ffi/src/ffi_composer_model.rs b/bindings/wysiwyg-ffi/src/ffi_composer_model.rs index 9909455bf..af144b2f9 100644 --- a/bindings/wysiwyg-ffi/src/ffi_composer_model.rs +++ b/bindings/wysiwyg-ffi/src/ffi_composer_model.rs @@ -260,7 +260,7 @@ impl ComposerModel { /// Creates an at-room mention node and inserts it into the composer at the current selection pub fn insert_at_room_mention( - &mut self, + self: &Arc, attributes: Vec, ) -> Arc { let attrs = attributes @@ -303,7 +303,7 @@ impl ComposerModel { /// Creates an at-room mention node and inserts it into the composer, replacing the /// text content defined by the suggestion pub fn insert_at_room_mention_at_suggestion( - &mut self, + self: &Arc, suggestion: SuggestionPattern, attributes: Vec, ) -> Arc { diff --git a/bindings/wysiwyg-ffi/src/wysiwyg_composer.udl b/bindings/wysiwyg-ffi/src/wysiwyg_composer.udl index 36f99f41a..bcae3bc09 100644 --- a/bindings/wysiwyg-ffi/src/wysiwyg_composer.udl +++ b/bindings/wysiwyg-ffi/src/wysiwyg_composer.udl @@ -48,7 +48,9 @@ interface ComposerModel { ComposerUpdate set_link(string url, sequence attributes); ComposerUpdate set_link_with_text(string url, string text, sequence attributes); ComposerUpdate remove_links(); + ComposerUpdate insert_at_room_mention(sequence attributes); ComposerUpdate insert_mention(string url, string text, sequence attributes); + ComposerUpdate insert_at_room_mention_at_suggestion(SuggestionPattern suggestion, sequence attributes); ComposerUpdate insert_mention_at_suggestion(string url, string text, SuggestionPattern suggestion, sequence attributes); ComposerUpdate code_block(); ComposerUpdate quote(); From c7fc5ec225f0487a76cee46f1da82107832cabfb Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 15 Jun 2023 17:12:52 +0100 Subject: [PATCH 52/56] update comment --- crates/wysiwyg/src/dom/nodes/dom_node.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/wysiwyg/src/dom/nodes/dom_node.rs b/crates/wysiwyg/src/dom/nodes/dom_node.rs index 2cb02f325..7e88bc313 100644 --- a/crates/wysiwyg/src/dom/nodes/dom_node.rs +++ b/crates/wysiwyg/src/dom/nodes/dom_node.rs @@ -139,10 +139,8 @@ where DomNode::Container(ContainerNode::new_link(url, children, attributes)) } - /// Create a new mention node. This function will perform a single check of the display - /// text and return an at-room mention if that text exactly matches `@room` - /// - /// Returns a result as creating a mention node can fail with an invalid uri + /// Attempts to create a new mention node. Returns a result as creating a + /// mention node can fail if attempted with an invalid uri. pub fn new_mention( url: S, display_text: S, From 18910a0c992aa8276196ec2d935e31dfb53bcf22 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Fri, 16 Jun 2023 09:43:09 +0100 Subject: [PATCH 53/56] add TODOs, fix issues from call --- crates/wysiwyg/src/composer_model/mentions.rs | 17 ++++++------ crates/wysiwyg/src/dom/nodes/dom_node.rs | 8 +++--- crates/wysiwyg/src/dom/nodes/mention_node.rs | 10 ------- crates/wysiwyg/src/dom/parser/parse.rs | 26 +++++++++++-------- 4 files changed, 28 insertions(+), 33 deletions(-) diff --git a/crates/wysiwyg/src/composer_model/mentions.rs b/crates/wysiwyg/src/composer_model/mentions.rs index 90e5f583b..4f62163b2 100644 --- a/crates/wysiwyg/src/composer_model/mentions.rs +++ b/crates/wysiwyg/src/composer_model/mentions.rs @@ -14,8 +14,9 @@ use matrix_mentions::Mention; use crate::{ - dom::DomLocation, ComposerModel, ComposerUpdate, DomNode, Location, - SuggestionPattern, UnicodeString, + dom::{nodes::MentionNode, DomLocation}, + ComposerModel, ComposerUpdate, DomNode, Location, SuggestionPattern, + UnicodeString, }; impl ComposerModel @@ -58,6 +59,7 @@ where text: S, attributes: Vec<(S, S)>, ) -> ComposerUpdate { + // TODO change this as this will now be handled by the if let below if self.should_not_insert_mention(&url) { return ComposerUpdate::keep(); } @@ -116,18 +118,17 @@ where /// mention is the last node in it's parent. fn do_insert_mention( &mut self, - mention_node: DomNode, + mention_node: MentionNode, // TODO can we type this as a mention node somehow ) -> ComposerUpdate { - if !mention_node.is_mention_node() { - return ComposerUpdate::keep(); - } - let (start, end) = self.safe_selection(); let range = self.state.dom.find_range(start, end); let new_cursor_index = start + mention_node.text_len(); - let handle = self.state.dom.insert_node_at_cursor(&range, mention_node); + let handle = self + .state + .dom + .insert_node_at_cursor(&range, DomNode::Mention(mention_node)); // manually move the cursor to the end of the mention self.state.start = Location::from(new_cursor_index); diff --git a/crates/wysiwyg/src/dom/nodes/dom_node.rs b/crates/wysiwyg/src/dom/nodes/dom_node.rs index 7e88bc313..c9517ef38 100644 --- a/crates/wysiwyg/src/dom/nodes/dom_node.rs +++ b/crates/wysiwyg/src/dom/nodes/dom_node.rs @@ -145,18 +145,18 @@ where url: S, display_text: S, attributes: Vec<(S, S)>, - ) -> Result, UriParseError> { + ) -> Result, UriParseError> { if let Ok(mention_node) = MentionNode::new(url, display_text, attributes) { - Ok(DomNode::Mention(mention_node)) + Ok(mention_node) } else { Err(UriParseError) } } - pub fn new_at_room_mention(attributes: Vec<(S, S)>) -> DomNode { - DomNode::Mention(MentionNode::new_at_room(attributes)) + pub fn new_at_room_mention(attributes: Vec<(S, S)>) -> MentionNode { + MentionNode::new_at_room(attributes) } pub fn is_container_node(&self) -> bool { diff --git a/crates/wysiwyg/src/dom/nodes/mention_node.rs b/crates/wysiwyg/src/dom/nodes/mention_node.rs index a44834983..ab589a27b 100644 --- a/crates/wysiwyg/src/dom/nodes/mention_node.rs +++ b/crates/wysiwyg/src/dom/nodes/mention_node.rs @@ -126,16 +126,6 @@ where pub fn kind(&self) -> &MentionNodeKind { &self.kind } - - // /// Util function to check if the display text is that of an at-room mention - // pub fn is_at_room_display_text(text: &S) -> bool { - // text == &S::from(AT_ROOM) - // } - - // /// Util function to get the display text for an at-room mention - // pub fn get_at_room_display_text() -> S { - // S::from(AT_ROOM) - // } } impl ToHtml for MentionNode diff --git a/crates/wysiwyg/src/dom/parser/parse.rs b/crates/wysiwyg/src/dom/parser/parse.rs index ec3b78ee5..17dd4ffeb 100644 --- a/crates/wysiwyg/src/dom/parser/parse.rs +++ b/crates/wysiwyg/src/dom/parser/parse.rs @@ -321,7 +321,7 @@ mod sys { ); match creation_result { - Ok(node) => node, + Ok(node) => DomNode::Mention(node), Err(_) => Self::new_link(link), } } @@ -712,7 +712,9 @@ fn convert_text( for (i, part) in contents.split("@room").into_iter().enumerate() { if i > 0 { - node.append_child(DomNode::new_at_room_mention(vec![])); + node.append_child(DomNode::Mention( + DomNode::new_at_room_mention(vec![]), + )); } if !part.is_empty() { node.append_child(DomNode::new_text(part.into())); @@ -863,15 +865,17 @@ mod js { }; if has_text && is_mention { dom.append_child( - DomNode::new_mention( - url.into(), - text.unwrap() - .node_value() - .unwrap_or_default() - .into(), - attributes, - ) - .unwrap(), // we unwrap because we have already confirmed the uri is valid + DomNode::Mention( + DomNode::new_mention( + url.into(), + text.unwrap() + .node_value() + .unwrap_or_default() + .into(), + attributes, + ) + .unwrap(), + ), // we unwrap because we have already confirmed the uri is valid ); } else { let children = self From 841774099585d1f22d6302d551a06b182c63303d Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Fri, 16 Jun 2023 10:01:14 +0100 Subject: [PATCH 54/56] add the configuration for the matrix_mentions crate --- Cargo.lock | 1 + crates/matrix_mentions/Cargo.toml | 4 +++- crates/matrix_mentions/src/mention.rs | 20 ++++++++++++++------ 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6f6200f5d..b0f6d80ac 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -452,6 +452,7 @@ dependencies = [ name = "matrix_mentions" version = "0.1.0" dependencies = [ + "cfg-if", "ruma-common", ] diff --git a/crates/matrix_mentions/Cargo.toml b/crates/matrix_mentions/Cargo.toml index 649c36012..85e054fb8 100644 --- a/crates/matrix_mentions/Cargo.toml +++ b/crates/matrix_mentions/Cargo.toml @@ -10,7 +10,9 @@ version = "0.1.0" edition = "2021" [features] -custom_matrix_urls = [] +default = ["custom-matrix-urls"] +custom-matrix-urls = [] [dependencies] +cfg-if = "1.0.0" ruma-common = "0.11.3" diff --git a/crates/matrix_mentions/src/mention.rs b/crates/matrix_mentions/src/mention.rs index c3d90d114..b485340da 100644 --- a/crates/matrix_mentions/src/mention.rs +++ b/crates/matrix_mentions/src/mention.rs @@ -158,20 +158,28 @@ impl Mention { /// If any of the above succeed, return Some Option { if let Ok(matrix_uri) = MatrixUri::parse(uri) { - Some(matrix_uri.id().to_owned()) + return Some(matrix_uri.id().to_owned()); } else if let Ok(matrix_to_uri) = MatrixToUri::parse(uri) { - Some(matrix_to_uri.id().to_owned()) - } else if let Ok(matrix_to_uri) = parse_external_id(uri) { - Some(matrix_to_uri.id().to_owned()) - } else { - None + return Some(matrix_to_uri.id().to_owned()); } + + cfg_if::cfg_if! { + if #[cfg(any(test, feature = "custom-matrix-urls"))] { + if let Ok(matrix_to_uri) = parse_external_id(uri) { + return Some(matrix_to_uri.id().to_owned()); + } + } + } + + None } /// Attempts to split an external id on `/#/`, rebuild as a matrix to style permalink then parse /// using ruma. /// /// Returns the result of calling `parse` in ruma. + +#[cfg(any(test, feature = "custom-matrix-urls"))] fn parse_external_id(uri: &str) -> Result { // first split the string into the parts we need let parts: Vec<&str> = uri.split("/#/").collect(); From 4028efc359512f7fa2478401b43fb427f8beab73 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Fri, 16 Jun 2023 10:04:28 +0100 Subject: [PATCH 55/56] fix TODO --- crates/wysiwyg/src/composer_model/mentions.rs | 21 ++++--------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/crates/wysiwyg/src/composer_model/mentions.rs b/crates/wysiwyg/src/composer_model/mentions.rs index 4f62163b2..acec8d1ee 100644 --- a/crates/wysiwyg/src/composer_model/mentions.rs +++ b/crates/wysiwyg/src/composer_model/mentions.rs @@ -11,7 +11,6 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. -use matrix_mentions::Mention; use crate::{ dom::{nodes::MentionNode, DomLocation}, @@ -32,7 +31,7 @@ where suggestion: SuggestionPattern, attributes: Vec<(S, S)>, ) -> ComposerUpdate { - if self.should_not_insert_mention(&url) { + if self.range_contains_link_or_code_leaves() { return ComposerUpdate::keep(); } @@ -59,8 +58,7 @@ where text: S, attributes: Vec<(S, S)>, ) -> ComposerUpdate { - // TODO change this as this will now be handled by the if let below - if self.should_not_insert_mention(&url) { + if self.range_contains_link_or_code_leaves() { return ComposerUpdate::keep(); } @@ -82,7 +80,7 @@ where suggestion: SuggestionPattern, attributes: Vec<(S, S)>, ) -> ComposerUpdate { - if self.should_not_insert_at_room_mention() { + if self.range_contains_link_or_code_leaves() { return ComposerUpdate::keep(); } @@ -101,7 +99,7 @@ where &mut self, attributes: Vec<(S, S)>, ) -> ComposerUpdate { - if self.should_not_insert_at_room_mention() { + if self.range_contains_link_or_code_leaves() { return ComposerUpdate::keep(); } @@ -144,17 +142,6 @@ where /// We should not insert a mention if the uri is invalid or the range contains link /// or code leaves. See issue https://github.com/matrix-org/matrix-rich-text-editor/issues/702. - fn should_not_insert_mention(&self, url: &S) -> bool { - !Mention::is_valid_uri(url.to_string().as_str()) - || self.range_contains_link_or_code_leaves() - } - - /// We should not insert an at-room mention if the range contains link or code leaves. - /// See issue https://github.com/matrix-org/matrix-rich-text-editor/issues/702. - fn should_not_insert_at_room_mention(&self) -> bool { - self.range_contains_link_or_code_leaves() - } - fn range_contains_link_or_code_leaves(&self) -> bool { let (start, end) = self.safe_selection(); let range = self.state.dom.find_range(start, end); From 46f48cb0f9e7c353e673b4e1179333f54bd8342c Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Fri, 16 Jun 2023 11:25:00 +0100 Subject: [PATCH 56/56] address comments --- crates/wysiwyg/src/composer_model/mentions.rs | 2 +- crates/wysiwyg/src/dom/nodes/mention_node.rs | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/crates/wysiwyg/src/composer_model/mentions.rs b/crates/wysiwyg/src/composer_model/mentions.rs index acec8d1ee..916be1d2e 100644 --- a/crates/wysiwyg/src/composer_model/mentions.rs +++ b/crates/wysiwyg/src/composer_model/mentions.rs @@ -116,7 +116,7 @@ where /// mention is the last node in it's parent. fn do_insert_mention( &mut self, - mention_node: MentionNode, // TODO can we type this as a mention node somehow + mention_node: MentionNode, ) -> ComposerUpdate { let (start, end) = self.safe_selection(); let range = self.state.dom.find_range(start, end); diff --git a/crates/wysiwyg/src/dom/nodes/mention_node.rs b/crates/wysiwyg/src/dom/nodes/mention_node.rs index ab589a27b..abe543292 100644 --- a/crates/wysiwyg/src/dom/nodes/mention_node.rs +++ b/crates/wysiwyg/src/dom/nodes/mention_node.rs @@ -47,7 +47,7 @@ where #[derive(Clone, Debug, PartialEq, Eq)] pub enum MentionNodeKind { - MatrixURI { mention: Mention }, + MatrixUri { mention: Mention }, AtRoom, } @@ -71,7 +71,7 @@ where &url.to_string(), &display_text.to_string(), ) { - let kind = MentionNodeKind::MatrixURI { mention }; + let kind = MentionNodeKind::MatrixUri { mention }; Ok(Self { display_text, kind, @@ -104,7 +104,7 @@ where pub fn display_text(&self) -> S { match self.kind() { - MentionNodeKind::MatrixURI { .. } => self.display_text.clone(), + MentionNodeKind::MatrixUri { .. } => self.display_text.clone(), MentionNodeKind::AtRoom => S::from(get_at_room_display_text()), } } @@ -128,6 +128,8 @@ where } } +// TODO implment From trait to convert from MentionNode to DomNode to allow MentionNode.into() usage + impl ToHtml for MentionNode where S: UnicodeString, @@ -155,7 +157,7 @@ impl MentionNode { let cur_pos = formatter.len(); match self.kind() { - MentionNodeKind::MatrixURI { mention } => { + MentionNodeKind::MatrixUri { mention } => { // if formatting as a message, only include the href attribute let attributes = if as_message { vec![("href".into(), S::from(mention.uri()))] @@ -229,7 +231,7 @@ where description.push("\""); match self.kind() { - MentionNodeKind::MatrixURI { mention } => { + MentionNodeKind::MatrixUri { mention } => { description.push(", "); description.push(S::from(mention.uri())); } @@ -268,7 +270,7 @@ where { let text = match this.kind() { // for User/Room type, we use the mx_id in the md output - MentionNodeKind::MatrixURI { mention } => { + MentionNodeKind::MatrixUri { mention } => { if mention.kind() == &MentionKind::Room { S::from(mention.mx_id()) } else {