From 06e9a40564dd400bf9ae83675530ea6a0d348fa8 Mon Sep 17 00:00:00 2001 From: sinofp Date: Thu, 4 May 2023 22:04:39 +0100 Subject: [PATCH 1/6] Add optional decoding="async" loading="lazy" for img MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In theory, they can make the page load faster and show content faster. There’s one problem: CommonMark allows arbitrary inline elements in alt text. If I want to get the correct alt text, I need to match every inline event. I think most people will only use plain text, so I only match Event::Text. --- components/config/src/config/markup.rs | 3 ++ components/markdown/src/markdown.rs | 44 +++++++++++++++++++++++--- 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/components/config/src/config/markup.rs b/components/config/src/config/markup.rs index 7cd03520c8..b1ac7fa22f 100644 --- a/components/config/src/config/markup.rs +++ b/components/config/src/config/markup.rs @@ -51,6 +51,8 @@ pub struct Markdown { /// The compiled extra themes into a theme set #[serde(skip_serializing, skip_deserializing)] // not a typo, 2 are need pub extra_theme_set: Arc>, + /// Add loading="lazy" decoding="async" to img tags. When turned on, the alt text must be plain text. Defaults to false + pub lazy_async_image: bool, } impl Markdown { @@ -204,6 +206,7 @@ impl Default for Markdown { extra_syntaxes_and_themes: vec![], extra_syntax_set: None, extra_theme_set: Arc::new(None), + lazy_async_image: false, } } } diff --git a/components/markdown/src/markdown.rs b/components/markdown/src/markdown.rs index d773a21f1f..88c4f6983c 100644 --- a/components/markdown/src/markdown.rs +++ b/components/markdown/src/markdown.rs @@ -252,6 +252,9 @@ pub fn markdown_to_html( let mut stop_next_end_p = false; + let lazy_async_image = context.config.markdown.lazy_async_image; + let mut is_image_alt = false; // For alt text between Event::Start(Tag::Image(..)) and Event::End(Tag::Image(..)) + let mut opts = Options::empty(); let mut has_summary = false; opts.insert(Options::ENABLE_TABLES); @@ -326,7 +329,13 @@ pub fn markdown_to_html( for (event, mut range) in Parser::new_ext(content, opts).into_offset_iter() { match event { Event::Text(text) => { - if let Some(ref mut _code_block) = code_block { + if lazy_async_image && is_image_alt { + let mut alt = String::new(); + cmark::escape::escape_html(&mut alt, &text) + .expect("Could not write to buffer"); + + events.push(Event::Html(alt.into())); + } else if let Some(ref mut _code_block) = code_block { if contains_shortcode(text.as_ref()) { // mark the start of the code block events let stack_start = events.len(); @@ -387,13 +396,38 @@ pub fn markdown_to_html( events.push(Event::Html("\n".into())); } Event::Start(Tag::Image(link_type, src, title)) => { - if is_colocated_asset_link(&src) { + let link = if is_colocated_asset_link(&src) { let link = format!("{}{}", context.current_page_permalink, &*src); - events.push(Event::Start(Tag::Image(link_type, link.into(), title))); + link.into() } else { - events.push(Event::Start(Tag::Image(link_type, src, title))); - } + src + }; + + events.push(if lazy_async_image { + is_image_alt = true; + + let mut img_before_alt: String = "\"").expect("Could events.push(if lazy_async_image { + is_image_alt = false; + Event::Html("\" loading=\"lazy\" decoding=\"async\" />".into()) + } else { + event + }), Event::Start(Tag::Link(link_type, link, title)) if link.is_empty() => { error = Some(Error::msg("There is a link that is missing a URL")); events.push(Event::Start(Tag::Link(link_type, "#".into(), title))); From 6127764ceecee5af87f3b1c4bd1ede22f1029e54 Mon Sep 17 00:00:00 2001 From: sinofp Date: Thu, 4 May 2023 22:19:03 +0100 Subject: [PATCH 2/6] Add very basic test for img This is the reason why we should use plain text when lazy_async_image is enabled. --- components/markdown/tests/img.rs | 48 ++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 components/markdown/tests/img.rs diff --git a/components/markdown/tests/img.rs b/components/markdown/tests/img.rs new file mode 100644 index 0000000000..41ba9feadd --- /dev/null +++ b/components/markdown/tests/img.rs @@ -0,0 +1,48 @@ +mod common; +use config::Config; + +#[test] +fn can_transform_image() { + // normal image + let rendered = common::render("![haha](https://example.com/abc.jpg)").unwrap().body; + assert_eq!(rendered, "

\"haha\"

\n"); + + // alt text with style + let rendered = common::render("![__ha__*ha*](https://example.com/abc.jpg)").unwrap().body; + assert_eq!(rendered, "

\"haha\"

\n"); + + // alt text with link + let rendered = + common::render("![ha[ha](https://example.com)](https://example.com/abc.jpg)").unwrap().body; + assert_eq!(rendered, "

\"haha\"

\n"); +} + +#[test] +fn can_add_lazy_loading_and_async_decoding() { + // normal alt text + let mut config = Config::default_for_test(); + config.markdown.lazy_async_image = true; + let rendered = + common::render_with_config("![haha](https://example.com/abc.jpg)", config.clone()) + .unwrap() + .body; + assert_eq!(rendered, "

\"haha\"

\n"); + + // Below is acceptable, but not recommended by CommonMark + + // alt text with style + let rendered = + common::render_with_config("![__ha__*ha*](https://example.com/abc.jpg)", config.clone()) + .unwrap() + .body; + assert_eq!(rendered, "

\"<stronghaha\" loading=\"lazy\" decoding=\"async\" />

\n"); + + // alt text with link + let rendered = common::render_with_config( + "![ha[ha](https://example.com)](https://example.com/abc.jpg)", + config.clone(), + ) + .unwrap() + .body; + assert_eq!(rendered, "

\"ha<aha\" loading=\"lazy\" decoding=\"async\" />

\n"); +} From dd45fb389e558ef7e93332c09fe6c36b07efc8e2 Mon Sep 17 00:00:00 2001 From: sinofp Date: Thu, 4 May 2023 22:25:14 +0100 Subject: [PATCH 3/6] Explain lazy_async_image in documentation --- docs/content/documentation/getting-started/configuration.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/content/documentation/getting-started/configuration.md b/docs/content/documentation/getting-started/configuration.md index b2ff781f6a..ac85b72426 100644 --- a/docs/content/documentation/getting-started/configuration.md +++ b/docs/content/documentation/getting-started/configuration.md @@ -122,6 +122,11 @@ external_links_no_referrer = false # For example, `...` into `…`, `"quote"` into `“curly”` etc smart_punctuation = false +# Whether to set decoding="async" and loading="lazy" for all images +# When turned on, the alt text must be plain text. +# For example, `![xx](...)` is ok but `![*x*x](...)` isn’t ok +lazy_async_image = false + # Configuration of the link checker. [link_checker] # Skip link checking for external URLs that start with these prefixes From bd85ca41e525601b44ce47af7b4a8ef5a7ed18a5 Mon Sep 17 00:00:00 2001 From: sinofp Date: Sat, 6 May 2023 13:20:47 +0100 Subject: [PATCH 4/6] Add test with empty alt and special characters I totaly forgot one can leave the alt text empty. I thought I need to eliminate the alt attribute in that case, but actually empty alt text is better than not having an alt attribute at all: https://www.w3.org/TR/WCAG20-TECHS/H67.html https://www.boia.org/blog/images-that-dont-need-alternative-text-still-need-alt-attributes Thus I will leave the empty alt text. Another test is added to ensure alt text is properly escaped. I will remove the redundant escaping code after this commit. --- components/markdown/tests/img.rs | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/components/markdown/tests/img.rs b/components/markdown/tests/img.rs index 41ba9feadd..68f5107438 100644 --- a/components/markdown/tests/img.rs +++ b/components/markdown/tests/img.rs @@ -7,6 +7,14 @@ fn can_transform_image() { let rendered = common::render("![haha](https://example.com/abc.jpg)").unwrap().body; assert_eq!(rendered, "

\"haha\"

\n"); + // empty alt text + let rendered = common::render("![](https://example.com/abc.jpg)").unwrap().body; + assert_eq!(rendered, "

\"\"

\n"); + + // alt text needs to be escaped + let rendered = common::render("![ha\"h>a](https://example.com/abc.jpg)").unwrap().body; + assert_eq!(rendered, "

\"ha"h>a\"

\n"); + // alt text with style let rendered = common::render("![__ha__*ha*](https://example.com/abc.jpg)").unwrap().body; assert_eq!(rendered, "

\"haha\"

\n"); @@ -19,15 +27,29 @@ fn can_transform_image() { #[test] fn can_add_lazy_loading_and_async_decoding() { - // normal alt text let mut config = Config::default_for_test(); config.markdown.lazy_async_image = true; + + // normal alt text let rendered = common::render_with_config("![haha](https://example.com/abc.jpg)", config.clone()) .unwrap() .body; assert_eq!(rendered, "

\"haha\"

\n"); + // empty alt text + let rendered = common::render_with_config("![](https://example.com/abc.jpg)", config.clone()) + .unwrap() + .body; + assert_eq!(rendered, "

\"\"

\n"); + + // alt text needs to be escaped + let rendered = + common::render_with_config("![ha\"h>a](https://example.com/abc.jpg)", config.clone()) + .unwrap() + .body; + assert_eq!(rendered, "

\"ha"h>a\"

\n"); + // Below is acceptable, but not recommended by CommonMark // alt text with style From e7ae995e656d8ba412bc1dcb57974ca2e791bffc Mon Sep 17 00:00:00 2001 From: sinofp Date: Sat, 6 May 2023 13:22:47 +0100 Subject: [PATCH 5/6] Remove manually escaping alt text After removing the if-else inside the arm of Event::Text(text), the alt text is still escaped. Indeed they are redundant. --- components/markdown/src/markdown.rs | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/components/markdown/src/markdown.rs b/components/markdown/src/markdown.rs index 88c4f6983c..a6cb3beca6 100644 --- a/components/markdown/src/markdown.rs +++ b/components/markdown/src/markdown.rs @@ -253,7 +253,6 @@ pub fn markdown_to_html( let mut stop_next_end_p = false; let lazy_async_image = context.config.markdown.lazy_async_image; - let mut is_image_alt = false; // For alt text between Event::Start(Tag::Image(..)) and Event::End(Tag::Image(..)) let mut opts = Options::empty(); let mut has_summary = false; @@ -329,13 +328,7 @@ pub fn markdown_to_html( for (event, mut range) in Parser::new_ext(content, opts).into_offset_iter() { match event { Event::Text(text) => { - if lazy_async_image && is_image_alt { - let mut alt = String::new(); - cmark::escape::escape_html(&mut alt, &text) - .expect("Could not write to buffer"); - - events.push(Event::Html(alt.into())); - } else if let Some(ref mut _code_block) = code_block { + if let Some(ref mut _code_block) = code_block { if contains_shortcode(text.as_ref()) { // mark the start of the code block events let stack_start = events.len(); @@ -404,8 +397,6 @@ pub fn markdown_to_html( }; events.push(if lazy_async_image { - is_image_alt = true; - let mut img_before_alt: String = " events.push(if lazy_async_image { - is_image_alt = false; Event::Html("\" loading=\"lazy\" decoding=\"async\" />".into()) } else { event From b351471fc1375a06a57c2b36be890ed51f37a5af Mon Sep 17 00:00:00 2001 From: sinofp Date: Sat, 6 May 2023 20:49:17 +0100 Subject: [PATCH 6/6] Use insta for snapshot testing `cargo insta review` looks cool! I wanted to dedup the cases variable, but my Rust skill is not good enough to declare a global vector. --- components/markdown/tests/img.rs | 75 +++++-------------- ...n_add_lazy_loading_and_async_decoding.snap | 10 +++ .../snapshots/img__can_transform_image.snap | 10 +++ 3 files changed, 39 insertions(+), 56 deletions(-) create mode 100644 components/markdown/tests/snapshots/img__can_add_lazy_loading_and_async_decoding.snap create mode 100644 components/markdown/tests/snapshots/img__can_transform_image.snap diff --git a/components/markdown/tests/img.rs b/components/markdown/tests/img.rs index 68f5107438..c34713a9d7 100644 --- a/components/markdown/tests/img.rs +++ b/components/markdown/tests/img.rs @@ -3,68 +3,31 @@ use config::Config; #[test] fn can_transform_image() { - // normal image - let rendered = common::render("![haha](https://example.com/abc.jpg)").unwrap().body; - assert_eq!(rendered, "

\"haha\"

\n"); - - // empty alt text - let rendered = common::render("![](https://example.com/abc.jpg)").unwrap().body; - assert_eq!(rendered, "

\"\"

\n"); - - // alt text needs to be escaped - let rendered = common::render("![ha\"h>a](https://example.com/abc.jpg)").unwrap().body; - assert_eq!(rendered, "

\"ha"h>a\"

\n"); - - // alt text with style - let rendered = common::render("![__ha__*ha*](https://example.com/abc.jpg)").unwrap().body; - assert_eq!(rendered, "

\"haha\"

\n"); + let cases = vec![ + "![haha](https://example.com/abc.jpg)", + "![](https://example.com/abc.jpg)", + "![ha\"h>a](https://example.com/abc.jpg)", + "![__ha__*ha*](https://example.com/abc.jpg)", + "![ha[ha](https://example.com)](https://example.com/abc.jpg)", + ]; - // alt text with link - let rendered = - common::render("![ha[ha](https://example.com)](https://example.com/abc.jpg)").unwrap().body; - assert_eq!(rendered, "

\"haha\"

\n"); + let body = common::render(&cases.join("\n")).unwrap().body; + insta::assert_snapshot!(body); } #[test] fn can_add_lazy_loading_and_async_decoding() { + let cases = vec![ + "![haha](https://example.com/abc.jpg)", + "![](https://example.com/abc.jpg)", + "![ha\"h>a](https://example.com/abc.jpg)", + "![__ha__*ha*](https://example.com/abc.jpg)", + "![ha[ha](https://example.com)](https://example.com/abc.jpg)", + ]; + let mut config = Config::default_for_test(); config.markdown.lazy_async_image = true; - // normal alt text - let rendered = - common::render_with_config("![haha](https://example.com/abc.jpg)", config.clone()) - .unwrap() - .body; - assert_eq!(rendered, "

\"haha\"

\n"); - - // empty alt text - let rendered = common::render_with_config("![](https://example.com/abc.jpg)", config.clone()) - .unwrap() - .body; - assert_eq!(rendered, "

\"\"

\n"); - - // alt text needs to be escaped - let rendered = - common::render_with_config("![ha\"h>a](https://example.com/abc.jpg)", config.clone()) - .unwrap() - .body; - assert_eq!(rendered, "

\"ha"h>a\"

\n"); - - // Below is acceptable, but not recommended by CommonMark - - // alt text with style - let rendered = - common::render_with_config("![__ha__*ha*](https://example.com/abc.jpg)", config.clone()) - .unwrap() - .body; - assert_eq!(rendered, "

\"<stronghaha\" loading=\"lazy\" decoding=\"async\" />

\n"); - - // alt text with link - let rendered = common::render_with_config( - "![ha[ha](https://example.com)](https://example.com/abc.jpg)", - config.clone(), - ) - .unwrap() - .body; - assert_eq!(rendered, "

\"ha<aha\" loading=\"lazy\" decoding=\"async\" />

\n"); + let body = common::render_with_config(&cases.join("\n"), config).unwrap().body; + insta::assert_snapshot!(body); } diff --git a/components/markdown/tests/snapshots/img__can_add_lazy_loading_and_async_decoding.snap b/components/markdown/tests/snapshots/img__can_add_lazy_loading_and_async_decoding.snap new file mode 100644 index 0000000000..ed179b4043 --- /dev/null +++ b/components/markdown/tests/snapshots/img__can_add_lazy_loading_and_async_decoding.snap @@ -0,0 +1,10 @@ +--- +source: components/markdown/tests/img.rs +expression: body +--- +

haha + +ha"h>a +<strong>ha</strong><em>ha</em> +ha<a href=ha" loading="lazy" decoding="async" />

+ diff --git a/components/markdown/tests/snapshots/img__can_transform_image.snap b/components/markdown/tests/snapshots/img__can_transform_image.snap new file mode 100644 index 0000000000..5ad51f586a --- /dev/null +++ b/components/markdown/tests/snapshots/img__can_transform_image.snap @@ -0,0 +1,10 @@ +--- +source: components/markdown/tests/img.rs +expression: body +--- +

haha + +ha"h>a +haha +haha

+