Revert "markdown: Add support for inline HTML `img` tags inside text (#37264)" (#37893)

Kirill Bulatov created 2 months ago

This reverts commit e1a5d299721bf14a7cdd41a5ae1bd03ea948c00e.

This have regressed Zed release notes' wrapping which we do not want to
do on a release day:
https://github.com/zed-industries/zed/pull/37264#issuecomment-3265420442

Release Notes:

- N/A

Change summary

crates/markdown_preview/src/markdown_elements.rs |   2 
crates/markdown_preview/src/markdown_parser.rs   | 250 +++++------------
crates/markdown_preview/src/markdown_renderer.rs |   6 
3 files changed, 86 insertions(+), 172 deletions(-)

Detailed changes

crates/markdown_preview/src/markdown_elements.rs

@@ -155,7 +155,7 @@ pub struct ParsedMarkdownText {
     /// Where the text is located in the source Markdown document.
     pub source_range: Range<usize>,
     /// The text content stripped of any formatting symbols.
-    pub contents: SharedString,
+    pub contents: String,
     /// The list of highlights contained in the Markdown document.
     pub highlights: Vec<(Range<usize>, MarkdownHighlight)>,
     /// The regions of the various ranges in the Markdown document.

crates/markdown_preview/src/markdown_parser.rs

@@ -353,7 +353,7 @@ impl<'a> MarkdownParser<'a> {
                         if !text.is_empty() {
                             let parsed_regions = MarkdownParagraphChunk::Text(ParsedMarkdownText {
                                 source_range: source_range.clone(),
-                                contents: text.into(),
+                                contents: text.clone(),
                                 highlights: highlights.clone(),
                                 region_ranges: region_ranges.clone(),
                                 regions: regions.clone(),
@@ -408,7 +408,7 @@ impl<'a> MarkdownParser<'a> {
         if !text.is_empty() {
             markdown_text_like.push(MarkdownParagraphChunk::Text(ParsedMarkdownText {
                 source_range,
-                contents: text.into(),
+                contents: text,
                 highlights,
                 regions,
                 region_ranges,
@@ -808,14 +808,15 @@ impl<'a> MarkdownParser<'a> {
             markup5ever_rcdom::NodeData::Document => {
                 self.consume_children(source_range, node, elements);
             }
+            markup5ever_rcdom::NodeData::Doctype { .. } => {}
             markup5ever_rcdom::NodeData::Text { contents } => {
                 elements.push(ParsedMarkdownElement::Paragraph(vec![
                     MarkdownParagraphChunk::Text(ParsedMarkdownText {
                         source_range,
-                        regions: Vec::default(),
-                        region_ranges: Vec::default(),
+                        contents: contents.borrow().to_string(),
                         highlights: Vec::default(),
-                        contents: contents.borrow().to_string().into(),
+                        region_ranges: Vec::default(),
+                        regions: Vec::default(),
                     }),
                 ]));
             }
@@ -825,64 +826,11 @@ impl<'a> MarkdownParser<'a> {
                     if let Some(image) = self.extract_image(source_range, attrs) {
                         elements.push(ParsedMarkdownElement::Image(image));
                     }
-                } else if local_name!("p") == name.local {
-                    self.parse_paragraph(
-                        source_range,
-                        node,
-                        &mut MarkdownParagraph::new(),
-                        elements,
-                    );
                 } else {
                     self.consume_children(source_range, node, elements);
                 }
             }
-            _ => {}
-        }
-    }
-
-    fn parse_paragraph(
-        &self,
-        source_range: Range<usize>,
-        node: &Rc<markup5ever_rcdom::Node>,
-        paragraph: &mut MarkdownParagraph,
-        elements: &mut Vec<ParsedMarkdownElement>,
-    ) {
-        match &node.data {
-            markup5ever_rcdom::NodeData::Text { contents } => {
-                paragraph.push(MarkdownParagraphChunk::Text(ParsedMarkdownText {
-                    source_range,
-                    regions: Vec::default(),
-                    region_ranges: Vec::default(),
-                    highlights: Vec::default(),
-                    contents: contents.borrow().to_string().into(),
-                }));
-            }
-            markup5ever_rcdom::NodeData::Element { name, attrs, .. } => {
-                if local_name!("img") == name.local {
-                    if let Some(image) = self.extract_image(source_range, attrs) {
-                        paragraph.push(MarkdownParagraphChunk::Image(image));
-                    }
-                } else {
-                    self.consume_paragraph(source_range, node, paragraph, elements);
-
-                    if !paragraph.is_empty() {
-                        elements.push(ParsedMarkdownElement::Paragraph(std::mem::take(paragraph)));
-                    }
-                }
-            }
-            _ => {}
-        }
-    }
-
-    fn consume_paragraph(
-        &self,
-        source_range: Range<usize>,
-        node: &Rc<markup5ever_rcdom::Node>,
-        paragraph: &mut MarkdownParagraph,
-        elements: &mut Vec<ParsedMarkdownElement>,
-    ) {
-        for node in node.children.borrow().iter() {
-            self.parse_paragraph(source_range.clone(), node, paragraph, elements);
+            markup5ever_rcdom::NodeData::ProcessingInstruction { .. } => {}
         }
     }
 
@@ -947,14 +895,14 @@ impl<'a> MarkdownParser<'a> {
 
         if let Some(width) = Self::attr_value(attrs, local_name!("width"))
             .or_else(|| styles.get("width").cloned())
-            .and_then(|width| Self::parse_html_element_dimension(&width))
+            .and_then(|width| Self::parse_length(&width))
         {
             image.set_width(width);
         }
 
         if let Some(height) = Self::attr_value(attrs, local_name!("height"))
             .or_else(|| styles.get("height").cloned())
-            .and_then(|height| Self::parse_html_element_dimension(&height))
+            .and_then(|height| Self::parse_length(&height))
         {
             image.set_height(height);
         }
@@ -962,7 +910,8 @@ impl<'a> MarkdownParser<'a> {
         Some(image)
     }
 
-    fn parse_html_element_dimension(value: &str) -> Option<DefiniteLength> {
+    /// Parses the width/height attribute value of an html element (e.g. img element)
+    fn parse_length(value: &str) -> Option<DefiniteLength> {
         if value.ends_with("%") {
             value
                 .trim_end_matches("%")
@@ -1061,7 +1010,7 @@ mod tests {
             ParsedMarkdownElement::Paragraph(vec![MarkdownParagraphChunk::Text(
                 ParsedMarkdownText {
                     source_range: 0..35,
-                    contents: "Some bostrikethroughld text".into(),
+                    contents: "Some bostrikethroughld text".to_string(),
                     highlights: Vec::new(),
                     region_ranges: Vec::new(),
                     regions: Vec::new(),
@@ -1235,7 +1184,7 @@ mod tests {
                 }),
                 MarkdownParagraphChunk::Text(ParsedMarkdownText {
                     source_range: 0..81,
-                    contents: " Lorem Ipsum ".into(),
+                    contents: " Lorem Ipsum ".to_string(),
                     highlights: Vec::new(),
                     region_ranges: Vec::new(),
                     regions: Vec::new(),
@@ -1254,130 +1203,90 @@ mod tests {
     }
 
     #[test]
-    fn test_parse_html_element_dimension() {
+    fn test_parse_length() {
         // Test percentage values
         assert_eq!(
-            MarkdownParser::parse_html_element_dimension("50%"),
+            MarkdownParser::parse_length("50%"),
             Some(DefiniteLength::Fraction(0.5))
         );
         assert_eq!(
-            MarkdownParser::parse_html_element_dimension("100%"),
+            MarkdownParser::parse_length("100%"),
             Some(DefiniteLength::Fraction(1.0))
         );
         assert_eq!(
-            MarkdownParser::parse_html_element_dimension("25%"),
+            MarkdownParser::parse_length("25%"),
             Some(DefiniteLength::Fraction(0.25))
         );
         assert_eq!(
-            MarkdownParser::parse_html_element_dimension("0%"),
+            MarkdownParser::parse_length("0%"),
             Some(DefiniteLength::Fraction(0.0))
         );
 
         // Test pixel values
         assert_eq!(
-            MarkdownParser::parse_html_element_dimension("100px"),
+            MarkdownParser::parse_length("100px"),
             Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(100.0))))
         );
         assert_eq!(
-            MarkdownParser::parse_html_element_dimension("50px"),
+            MarkdownParser::parse_length("50px"),
             Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(50.0))))
         );
         assert_eq!(
-            MarkdownParser::parse_html_element_dimension("0px"),
+            MarkdownParser::parse_length("0px"),
             Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(0.0))))
         );
 
         // Test values without units (should be treated as pixels)
         assert_eq!(
-            MarkdownParser::parse_html_element_dimension("100"),
+            MarkdownParser::parse_length("100"),
             Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(100.0))))
         );
         assert_eq!(
-            MarkdownParser::parse_html_element_dimension("42"),
+            MarkdownParser::parse_length("42"),
             Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(42.0))))
         );
 
         // Test invalid values
-        assert_eq!(
-            MarkdownParser::parse_html_element_dimension("invalid"),
-            None
-        );
-        assert_eq!(MarkdownParser::parse_html_element_dimension("px"), None);
-        assert_eq!(MarkdownParser::parse_html_element_dimension("%"), None);
-        assert_eq!(MarkdownParser::parse_html_element_dimension(""), None);
-        assert_eq!(MarkdownParser::parse_html_element_dimension("abc%"), None);
-        assert_eq!(MarkdownParser::parse_html_element_dimension("abcpx"), None);
+        assert_eq!(MarkdownParser::parse_length("invalid"), None);
+        assert_eq!(MarkdownParser::parse_length("px"), None);
+        assert_eq!(MarkdownParser::parse_length("%"), None);
+        assert_eq!(MarkdownParser::parse_length(""), None);
+        assert_eq!(MarkdownParser::parse_length("abc%"), None);
+        assert_eq!(MarkdownParser::parse_length("abcpx"), None);
 
         // Test decimal values
         assert_eq!(
-            MarkdownParser::parse_html_element_dimension("50.5%"),
+            MarkdownParser::parse_length("50.5%"),
             Some(DefiniteLength::Fraction(0.505))
         );
         assert_eq!(
-            MarkdownParser::parse_html_element_dimension("100.25px"),
+            MarkdownParser::parse_length("100.25px"),
             Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(100.25))))
         );
         assert_eq!(
-            MarkdownParser::parse_html_element_dimension("42.0"),
+            MarkdownParser::parse_length("42.0"),
             Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(42.0))))
         );
     }
 
-    #[gpui::test]
-    async fn test_inline_html_image_tag() {
-        let parsed =
-            parse("<p>Some text<img src=\"http://example.com/foo.png\" /> some more text</p>")
-                .await;
-
-        assert_eq!(
-            ParsedMarkdown {
-                children: vec![ParsedMarkdownElement::Paragraph(vec![
-                    MarkdownParagraphChunk::Text(ParsedMarkdownText {
-                        source_range: 0..71,
-                        contents: "Some text".into(),
-                        highlights: Default::default(),
-                        region_ranges: Default::default(),
-                        regions: Default::default()
-                    }),
-                    MarkdownParagraphChunk::Image(Image {
-                        source_range: 0..71,
-                        link: Link::Web {
-                            url: "http://example.com/foo.png".to_string(),
-                        },
-                        alt_text: None,
-                        height: None,
-                        width: None,
-                    }),
-                    MarkdownParagraphChunk::Text(ParsedMarkdownText {
-                        source_range: 0..71,
-                        contents: " some more text".into(),
-                        highlights: Default::default(),
-                        region_ranges: Default::default(),
-                        regions: Default::default()
-                    }),
-                ])]
-            },
-            parsed
-        );
-    }
-
     #[gpui::test]
     async fn test_html_image_tag() {
         let parsed = parse("<img src=\"http://example.com/foo.png\" />").await;
 
+        let ParsedMarkdownElement::Image(image) = &parsed.children[0] else {
+            panic!("Expected a image element");
+        };
         assert_eq!(
-            ParsedMarkdown {
-                children: vec![ParsedMarkdownElement::Image(Image {
-                    source_range: 0..40,
-                    link: Link::Web {
-                        url: "http://example.com/foo.png".to_string(),
-                    },
-                    alt_text: None,
-                    height: None,
-                    width: None,
-                })]
+            image.clone(),
+            Image {
+                source_range: 0..40,
+                link: Link::Web {
+                    url: "http://example.com/foo.png".to_string(),
+                },
+                alt_text: None,
+                height: None,
+                width: None,
             },
-            parsed
         );
     }
 
@@ -1385,19 +1294,20 @@ mod tests {
     async fn test_html_image_tag_with_alt_text() {
         let parsed = parse("<img src=\"http://example.com/foo.png\" alt=\"Foo\" />").await;
 
+        let ParsedMarkdownElement::Image(image) = &parsed.children[0] else {
+            panic!("Expected a image element");
+        };
         assert_eq!(
-            ParsedMarkdown {
-                children: vec![ParsedMarkdownElement::Image(Image {
-                    source_range: 0..50,
-                    link: Link::Web {
-                        url: "http://example.com/foo.png".to_string(),
-                    },
-                    alt_text: Some("Foo".into()),
-                    height: None,
-                    width: None,
-                })]
+            image.clone(),
+            Image {
+                source_range: 0..50,
+                link: Link::Web {
+                    url: "http://example.com/foo.png".to_string(),
+                },
+                alt_text: Some("Foo".into()),
+                height: None,
+                width: None,
             },
-            parsed
         );
     }
 
@@ -1406,19 +1316,20 @@ mod tests {
         let parsed =
             parse("<img src=\"http://example.com/foo.png\" height=\"100\" width=\"200\" />").await;
 
+        let ParsedMarkdownElement::Image(image) = &parsed.children[0] else {
+            panic!("Expected a image element");
+        };
         assert_eq!(
-            ParsedMarkdown {
-                children: vec![ParsedMarkdownElement::Image(Image {
-                    source_range: 0..65,
-                    link: Link::Web {
-                        url: "http://example.com/foo.png".to_string(),
-                    },
-                    alt_text: None,
-                    height: Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(100.)))),
-                    width: Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(200.)))),
-                })]
+            image.clone(),
+            Image {
+                source_range: 0..65,
+                link: Link::Web {
+                    url: "http://example.com/foo.png".to_string(),
+                },
+                alt_text: None,
+                height: Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(100.)))),
+                width: Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(200.)))),
             },
-            parsed
         );
     }
 
@@ -1429,19 +1340,20 @@ mod tests {
         )
         .await;
 
+        let ParsedMarkdownElement::Image(image) = &parsed.children[0] else {
+            panic!("Expected a image element");
+        };
         assert_eq!(
-            ParsedMarkdown {
-                children: vec![ParsedMarkdownElement::Image(Image {
-                    source_range: 0..75,
-                    link: Link::Web {
-                        url: "http://example.com/foo.png".to_string(),
-                    },
-                    alt_text: None,
-                    height: Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(100.)))),
-                    width: Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(200.)))),
-                })]
+            image.clone(),
+            Image {
+                source_range: 0..75,
+                link: Link::Web {
+                    url: "http://example.com/foo.png".to_string(),
+                },
+                alt_text: None,
+                height: Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(100.)))),
+                width: Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(200.)))),
             },
-            parsed
         );
     }
 
@@ -1892,7 +1804,7 @@ fn main() {
             region_ranges: Vec::new(),
             regions: Vec::new(),
             source_range,
-            contents: contents.to_string().into(),
+            contents: contents.to_string(),
         })]
     }
 

crates/markdown_preview/src/markdown_renderer.rs

@@ -628,13 +628,15 @@ fn render_markdown_code_block(
 }
 
 fn render_markdown_paragraph(parsed: &MarkdownParagraph, cx: &mut RenderContext) -> AnyElement {
-    cx.with_common_p(h_flex().flex_wrap())
+    cx.with_common_p(div())
         .children(render_markdown_text(parsed, cx))
+        .flex()
+        .flex_col()
         .into_any_element()
 }
 
 fn render_markdown_text(parsed_new: &MarkdownParagraph, cx: &mut RenderContext) -> Vec<AnyElement> {
-    let mut any_element = Vec::with_capacity(parsed_new.len());
+    let mut any_element = vec![];
     // these values are cloned in-order satisfy borrow checker
     let syntax_theme = cx.syntax_theme.clone();
     let workspace_clone = cx.workspace.clone();