(Un)escape URLs when reading/writing <img src=> attributes

R/check-built.R

@@ -14,7 +14,7 @@ check_missing_images <- function(pkg, src_path, dst_path) {
   img <- xml2::xml_find_all(html, ".//img")
   src <- xml2::xml_attr(img, "src")
 
-  rel_src <- src[xml2::url_parse(src)$scheme == ""]
+  rel_src <- xml2::url_unescape(src[xml2::url_parse(src)$scheme == ""])
   rel_path <- path_norm(path(path_dir(dst_path), rel_src))
   exists <- file_exists(path(pkg$dst_path, rel_path))
 

R/tweak-page.R

@@ -47,10 +47,16 @@ tweak_rmarkdown_html <- function(html, input_path, pkg = list(bs_version = 3)) {
   src <- xml2::xml_attr(img, "src")
   abs_src <- is_absolute_path(src)
   if (any(abs_src)) {
+    img_target_nodes <- img[abs_src]
+    img_src_real <- path_real(xml2::url_unescape(src[abs_src]))
+    input_path_real <- path_real(xml2::url_unescape(input_path))
+    img_rel_paths <- path_rel(path = img_src_real, start = input_path_real)
+    img_rel_paths <- xml2::url_escape(img_rel_paths)
+
     purrr::walk2(
-      img[abs_src],
-      path_rel(path_real(src[abs_src]), path_real(input_path)),
-      xml2::xml_set_attr,
+      .x = img_target_nodes,
+      .y = img_rel_paths,
+      .f = xml2::xml_set_attr,
       attr = "src"
     )
   }

tests/testthat/_snaps/build-article.new.md

@@ -0,0 +1,123 @@
+# bad width gives nice error
+
+    Code
+      rmarkdown_setup_pkgdown(pkg)
+    Condition
+      Error in `rmarkdown_setup_pkgdown()`:
+      ! In _pkgdown.yml, code.width must be a whole number, not the string "abc".
+
+# output is reproducible by default, i.e. 'seed' is respected
+
+    Code
+      cat(output)
+    Output
+      ## [1] 0.080750138 0.834333037 0.600760886 0.157208442 0.007399441
+
+# reports on bad open graph meta-data
+
+    Code
+      build_article("test", pkg)
+    Message
+      Reading vignettes/test.Rmd
+    Condition
+      Error in `build_article()`:
+      ! In vignettes/test.Rmd, opengraph.twitter must be a list, not the number 1.
+
+# build_article styles ANSI escapes
+
+    <span class="co">## <span style="color: #BB0000;">X</span></span>
+
+# build_article yields useful error if pandoc fails
+
+    Code
+      build_article("test", pkg, pandoc_args = "--fail-if-warnings")
+    Message
+      Reading vignettes/test.Rmd
+    Condition
+      Error in `build_article()`:
+      ! Failed to render 'vignettes/test.Rmd'.
+      x [WARNING] This document format requires a nonempty <title> element.
+      x  Defaulting to 'test.knit' as the title.
+      x  To specify a title, use 'title' in metadata or --metadata title="...".
+      x Failing because there were warnings.
+      Caused by error:
+      ! pandoc document conversion failed with error 3
+
+# build_article yields useful error if R fails
+
+    Code
+      build_article("test", pkg)
+    Message
+      Reading vignettes/test.Rmd
+    Condition
+      Error in `build_article()`:
+      ! Failed to render 'vignettes/test.Rmd'.
+      x Quitting from lines 5-9 [unnamed-chunk-1] (test.Rmd)
+      Caused by error:
+      ! Error!
+
+---
+
+    Code
+      summary(expect_error(build_article("test", pkg)))
+    Message
+      Reading vignettes/test.Rmd
+    Output
+      <error/rlang_error>
+      Error in `build_article()`:
+      ! Failed to render 'vignettes/test.Rmd'.
+      x Quitting from lines 5-9 [unnamed-chunk-1] (test.Rmd)
+      Caused by error:
+      ! Error!
+      ---
+      Backtrace:
+           x
+        1. +-evaluate (local) `<fn>`()
+        2. | +-base::withRestarts(...)
+        3. | | \-base (local) withRestartList(expr, restarts)
+        4. | |   +-base (local) withOneRestart(withRestartList(expr, restarts[-nr]), restarts[[nr]])
+        5. | |   | \-base (local) doWithOneRestart(return(expr), restart)
+        6. | |   \-base (local) withRestartList(expr, restarts[-nr])
+        7. | |     +-base (local) withOneRestart(withRestartList(expr, restarts[-nr]), restarts[[nr]])
+        8. | |     | \-base (local) doWithOneRestart(return(expr), restart)
+        9. | |     \-base (local) withRestartList(expr, restarts[-nr])
+       10. | |       \-base (local) withOneRestart(expr, restarts[[1L]])
+       11. | |         \-base (local) doWithOneRestart(return(expr), restart)
+       12. | +-evaluate:::with_handlers(...)
+       13. | | +-base::eval(call)
+       14. | | | \-base::eval(call)
+       15. | | \-base::withCallingHandlers(...)
+       16. | \-base::withVisible(do)
+       17. \-global f()
+       18.   \-global g()
+       19.     \-global h()
+
+# build_article copies image files in subdirectories
+
+    Code
+      build_article("test", pkg)
+    Message
+      Reading vignettes/test.Rmd
+      Writing `articles/test.html`
+
+# warns about missing images
+
+    Code
+      build_article("kitten", pkg)
+    Message
+      Reading vignettes/kitten.Rmd
+      Writing `articles/kitten.html`
+      Missing images in 'vignettes/kitten.Rmd': 'kitten.jpg'
+      i pkgdown can only use images in 'man/figures' and 'vignettes'
+
+# warns about missing alt-text
+
+    Code
+      build_article("kitten", pkg)
+    Message
+      Reading vignettes/kitten.Rmd
+      Writing `articles/kitten.html`
+      x Missing alt-text in 'vignettes/kitten.Rmd'
+      * kitten.jpg
+      i Learn more in `vignette(pkgdown::accessibility)`.
+

tests/testthat/test-build-article.R

@@ -81,7 +81,7 @@ test_that("BS5 article gets correctly activated navbar", {
 
   html <- xml2::read_html(article_path)
   navbar <- xml2::xml_find_first(html, ".//div[contains(@class, 'navbar')]")
-  
+
   expect_equal(
     xpath_text(navbar,".//li[contains(@class, 'active')]//button"),
     "Articles"
@@ -104,7 +104,7 @@ test_that("output is reproducible by default, i.e. 'seed' is respected", {
     r_code_block("runif(5L)")
   ))
   suppressMessages(path <- build_article("test", pkg))
-  
+
   html <- xml2::read_html(path)
   output <- xpath_text(html, "//main//pre")[[2]]
   expect_snapshot(cat(output))
@@ -133,7 +133,7 @@ test_that("can control math mode", {
   suppressMessages(path <- build_article("math", pkg))
   html <- xml2::read_html(path)
   expect_equal(xpath_length(html, ".//span[contains(@class, 'math')]"), 1)
-  
+
   pkg$meta$template$`math-rendering` <- "katex"
   suppressMessages(init_site(pkg))
   suppressMessages(path <- build_article("math", pkg))
@@ -276,6 +276,21 @@ test_that("warns about missing images", {
   expect_snapshot(build_article("kitten", pkg))
 })
 
+test_that("spaces in sorce paths do work", {
+  # create simulated package
+  pkg0 <- local_pkgdown_site()
+  pkg0 <- pkg_add_file(pkg0, "vignettes/kitten.Rmd", "![Kitten](kitten.jpg)")
+  pkg0 <- pkg_add_kitten(pkg0, "vignettes")
+
+  # copy simulated pkg to path that contains spaces
+  pkg1 <- fs::dir_copy(pkg0$src_path, fs::file_temp(pattern = "beware of spaces-"))
+
+  # check that pkgdown site builds anyways
+  expect_no_error(suppressMessages(
+    build_article("kitten", as_pkgdown(pkg1))
+  ))
+})
+
 test_that("warns about missing alt-text", {
   pkg <- local_pkgdown_site()
   pkg <- pkg_add_file(pkg, "vignettes/kitten.Rmd", "![](kitten.jpg)")