Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for lazy loading images #2211

Merged
merged 6 commits into from
May 6, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions components/config/src/config/markup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Option<ThemeSet>>,
/// 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 {
Expand Down Expand Up @@ -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,
}
}
}
32 changes: 28 additions & 4 deletions components/markdown/src/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,8 @@ pub fn markdown_to_html(

let mut stop_next_end_p = false;

let lazy_async_image = context.config.markdown.lazy_async_image;

let mut opts = Options::empty();
let mut has_summary = false;
opts.insert(Options::ENABLE_TABLES);
Expand Down Expand Up @@ -387,13 +389,35 @@ pub fn markdown_to_html(
events.push(Event::Html("</code></pre>\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 {
let mut img_before_alt: String = "<img src=\"".to_string();
cmark::escape::escape_href(&mut img_before_alt, &link)
.expect("Could not write to buffer");
if !title.is_empty() {
img_before_alt
.write_str("\" title=\"")
.expect("Could not write to buffer");
cmark::escape::escape_href(&mut img_before_alt, &title)
.expect("Could not write to buffer");
}
img_before_alt.write_str("\" alt=\"").expect("Could not write to buffer");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not certain there will be an alt right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the user might not input any alt text, but we still need to emit an empty alt attribute for the screen reader to ignore the picture: https://www.boia.org/blog/images-that-dont-need-alternative-text-still-need-alt-attributes

Event::Html(img_before_alt.into())
} else {
Event::Start(Tag::Image(link_type, link, title))
});
}
Event::End(Tag::Image(..)) => events.push(if lazy_async_image {
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)));
Expand Down
70 changes: 70 additions & 0 deletions components/markdown/tests/img.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
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, "<p><img src=\"https://example.com/abc.jpg\" alt=\"haha\" /></p>\n");

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

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

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

// alt text with link
let rendered =
common::render("![ha[ha](https://example.com)](https://example.com/abc.jpg)").unwrap().body;
assert_eq!(rendered, "<p><img src=\"https://example.com/abc.jpg\" alt=\"haha\" /></p>\n");
}

#[test]
fn can_add_lazy_loading_and_async_decoding() {
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, "<p><img src=\"https://example.com/abc.jpg\" alt=\"haha\" loading=\"lazy\" decoding=\"async\" /></p>\n");

// empty alt text
let rendered = common::render_with_config("![](https://example.com/abc.jpg)", config.clone())
.unwrap()
.body;
assert_eq!(rendered, "<p><img src=\"https://example.com/abc.jpg\" alt=\"\" loading=\"lazy\" decoding=\"async\" /></p>\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, "<p><img src=\"https://example.com/abc.jpg\" alt=\"ha&quot;h&gt;a\" loading=\"lazy\" decoding=\"async\" /></p>\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, "<p><img src=\"https://example.com/abc.jpg\" alt=\"<strong>ha</strong><em>ha</em>\" loading=\"lazy\" decoding=\"async\" /></p>\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, "<p><img src=\"https://example.com/abc.jpg\" alt=\"ha<a href=\"https://example.com\">ha</a>\" loading=\"lazy\" decoding=\"async\" /></p>\n");
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test with an empty alt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! I added it for both pulldown-cmark and mine.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I forgot zola was already using insta for snapshot testing. Can you check eg can_render_basic_markdown to see how it works? Otherwise I'll do it myself later

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries, I want to try it, so I’ve converted the test.

5 changes: 5 additions & 0 deletions docs/content/documentation/getting-started/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down