From e946a06efe06c76b7e32c2f96d3ce98f04159b95 Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Thu, 9 Oct 2025 19:11:42 +0200 Subject: [PATCH] markdown: Add Support for HTML `img` tags in text (#38107) Re-adds: https://github.com/zed-industries/zed/pull/37264 This PR re-adds basic support for showing HTML images, without touching the display mode for images. The initial PR changed the `div().flex().flex_col()` to `h_flex().flex_wrap()` but this broke the text wrapping in almost all cases. **Note**: This does not add support for showing the images inline, because we haven't figured out how they correctly do this. I'm working on adding the CSS `inline` display feature support to taffy that hopefully allows us to correctly show images/other elements inline without breaking the text wrapping. **Before (nightly) and after (dev) for the README file inside Zed. (nothing has changed, which is good)** Screenshot 2025-09-13 at 12 49 08 **Result** Screenshot 2025-09-13 at 12 51 54 cc @SomeoneToIgnore Release Notes: - markdown preview: Added support for HTML `img` tags inside paragraphs --- .../markdown_preview/src/markdown_elements.rs | 2 +- .../markdown_preview/src/markdown_parser.rs | 246 +++++++++++------- 2 files changed, 148 insertions(+), 100 deletions(-) diff --git a/crates/markdown_preview/src/markdown_elements.rs b/crates/markdown_preview/src/markdown_elements.rs index 4da6c4ca5a956527ae669707ee683c417f440482..827c11f0453817b00431a1f32db8c645aced4e86 100644 --- a/crates/markdown_preview/src/markdown_elements.rs +++ b/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, /// The text content stripped of any formatting symbols. - pub contents: String, + pub contents: SharedString, /// The list of highlights contained in the Markdown document. pub highlights: Vec<(Range, MarkdownHighlight)>, /// The regions of the various ranges in the Markdown document. diff --git a/crates/markdown_preview/src/markdown_parser.rs b/crates/markdown_preview/src/markdown_parser.rs index ba02a00932320e5ac5bb1ac1cd922e78e1f9c7e5..b7eb73c60a049ca7adb03160d124904bab4cee89 100644 --- a/crates/markdown_preview/src/markdown_parser.rs +++ b/crates/markdown_preview/src/markdown_parser.rs @@ -374,7 +374,7 @@ impl<'a> MarkdownParser<'a> { if !text.is_empty() { let parsed_regions = MarkdownParagraphChunk::Text(ParsedMarkdownText { source_range: source_range.clone(), - contents: text.clone(), + contents: text.into(), highlights: highlights.clone(), region_ranges: region_ranges.clone(), regions: regions.clone(), @@ -429,7 +429,7 @@ impl<'a> MarkdownParser<'a> { if !text.is_empty() { markdown_text_like.push(MarkdownParagraphChunk::Text(ParsedMarkdownText { source_range, - contents: text, + contents: text.into(), highlights, regions, region_ranges, @@ -835,15 +835,14 @@ 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, - contents: contents.borrow().to_string(), - highlights: Vec::default(), - region_ranges: Vec::default(), regions: Vec::default(), + region_ranges: Vec::default(), + highlights: Vec::default(), + contents: contents.borrow().to_string().into(), }), ])); } @@ -853,6 +852,13 @@ 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 { + let mut paragraph = MarkdownParagraph::new(); + self.parse_paragraph(source_range, node, &mut paragraph); + + if !paragraph.is_empty() { + elements.push(ParsedMarkdownElement::Paragraph(paragraph)); + } } else if matches!( name.local, local_name!("h1") @@ -892,7 +898,7 @@ impl<'a> MarkdownParser<'a> { self.consume_children(source_range, node, elements); } } - markup5ever_rcdom::NodeData::ProcessingInstruction { .. } => {} + _ => {} } } @@ -907,13 +913,19 @@ impl<'a> MarkdownParser<'a> { paragraph.push(MarkdownParagraphChunk::Text(ParsedMarkdownText { source_range, regions: Vec::default(), - contents: contents.borrow().to_string(), region_ranges: Vec::default(), highlights: Vec::default(), + contents: contents.borrow().to_string().into(), })); } - markup5ever_rcdom::NodeData::Element { .. } => { - self.consume_paragraph(source_range, node, paragraph); + 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); + } } _ => {} } @@ -991,14 +1003,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_length(&width)) + .and_then(|width| Self::parse_html_element_dimension(&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_length(&height)) + .and_then(|height| Self::parse_html_element_dimension(&height)) { image.set_height(height); } @@ -1006,6 +1018,22 @@ impl<'a> MarkdownParser<'a> { Some(image) } + fn parse_html_element_dimension(value: &str) -> Option { + if value.ends_with("%") { + value + .trim_end_matches("%") + .parse::() + .ok() + .map(|value| relative(value / 100.)) + } else { + value + .trim_end_matches("px") + .parse() + .ok() + .map(|value| px(value).into()) + } + } + fn extract_html_blockquote( &self, node: &Rc, @@ -1072,23 +1100,6 @@ impl<'a> MarkdownParser<'a> { None } } - - /// Parses the width/height attribute value of an html element (e.g. img element) - fn parse_length(value: &str) -> Option { - if value.ends_with("%") { - value - .trim_end_matches("%") - .parse::() - .ok() - .map(|value| relative(value / 100.)) - } else { - value - .trim_end_matches("px") - .parse() - .ok() - .map(|value| px(value).into()) - } - } } #[cfg(test)] @@ -1173,7 +1184,7 @@ mod tests { ParsedMarkdownElement::Paragraph(vec![MarkdownParagraphChunk::Text( ParsedMarkdownText { source_range: 0..35, - contents: "Some bostrikethroughld text".to_string(), + contents: "Some bostrikethroughld text".into(), highlights: Vec::new(), region_ranges: Vec::new(), regions: Vec::new(), @@ -1347,7 +1358,7 @@ mod tests { }), MarkdownParagraphChunk::Text(ParsedMarkdownText { source_range: 0..81, - contents: " Lorem Ipsum ".to_string(), + contents: " Lorem Ipsum ".into(), highlights: Vec::new(), region_ranges: Vec::new(), regions: Vec::new(), @@ -1366,72 +1377,113 @@ mod tests { } #[test] - fn test_parse_length() { + fn test_parse_html_element_dimension() { // Test percentage values assert_eq!( - MarkdownParser::parse_length("50%"), + MarkdownParser::parse_html_element_dimension("50%"), Some(DefiniteLength::Fraction(0.5)) ); assert_eq!( - MarkdownParser::parse_length("100%"), + MarkdownParser::parse_html_element_dimension("100%"), Some(DefiniteLength::Fraction(1.0)) ); assert_eq!( - MarkdownParser::parse_length("25%"), + MarkdownParser::parse_html_element_dimension("25%"), Some(DefiniteLength::Fraction(0.25)) ); assert_eq!( - MarkdownParser::parse_length("0%"), + MarkdownParser::parse_html_element_dimension("0%"), Some(DefiniteLength::Fraction(0.0)) ); // Test pixel values assert_eq!( - MarkdownParser::parse_length("100px"), + MarkdownParser::parse_html_element_dimension("100px"), Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(100.0)))) ); assert_eq!( - MarkdownParser::parse_length("50px"), + MarkdownParser::parse_html_element_dimension("50px"), Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(50.0)))) ); assert_eq!( - MarkdownParser::parse_length("0px"), + MarkdownParser::parse_html_element_dimension("0px"), Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(0.0)))) ); // Test values without units (should be treated as pixels) assert_eq!( - MarkdownParser::parse_length("100"), + MarkdownParser::parse_html_element_dimension("100"), Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(100.0)))) ); assert_eq!( - MarkdownParser::parse_length("42"), + MarkdownParser::parse_html_element_dimension("42"), Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(42.0)))) ); // Test invalid values - 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); + 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); // Test decimal values assert_eq!( - MarkdownParser::parse_length("50.5%"), + MarkdownParser::parse_html_element_dimension("50.5%"), Some(DefiniteLength::Fraction(0.505)) ); assert_eq!( - MarkdownParser::parse_length("100.25px"), + MarkdownParser::parse_html_element_dimension("100.25px"), Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(100.25)))) ); assert_eq!( - MarkdownParser::parse_length("42.0"), + MarkdownParser::parse_html_element_dimension("42.0"), Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(42.0)))) ); } + #[gpui::test] + async fn test_inline_html_image_tag() { + let parsed = + parse("

Some text some more text

") + .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_block_quote() { let parsed = parse( @@ -1668,20 +1720,19 @@ mod tests { async fn test_html_image_tag() { let parsed = parse("").await; - let ParsedMarkdownElement::Image(image) = &parsed.children[0] else { - panic!("Expected a image element"); - }; assert_eq!( - 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, + 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, + })] }, + parsed ); } @@ -1689,20 +1740,19 @@ mod tests { async fn test_html_image_tag_with_alt_text() { let parsed = parse("\"Foo\"").await; - let ParsedMarkdownElement::Image(image) = &parsed.children[0] else { - panic!("Expected a image element"); - }; assert_eq!( - 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, + 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, + })] }, + parsed ); } @@ -1711,20 +1761,19 @@ mod tests { let parsed = parse("").await; - let ParsedMarkdownElement::Image(image) = &parsed.children[0] else { - panic!("Expected a image element"); - }; assert_eq!( - 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.)))), + 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.)))), + })] }, + parsed ); } @@ -1735,20 +1784,19 @@ mod tests { ) .await; - let ParsedMarkdownElement::Image(image) = &parsed.children[0] else { - panic!("Expected a image element"); - }; assert_eq!( - 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.)))), + 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.)))), + })] }, + parsed ); } @@ -2199,7 +2247,7 @@ fn main() { region_ranges: Vec::new(), regions: Vec::new(), source_range, - contents: contents.to_string(), + contents: contents.to_string().into(), })] }