yihui / knitr

A general-purpose tool for dynamic report generation in R
https://yihui.org/knitr/
2.36k stars 873 forks source link

Automatic figure numbering knitr_1.44 fails #2290

Closed jay-sf closed 9 months ago

jay-sf commented 9 months ago

After updating knitr_1.43 to knitr_1.44 captions are no longer numbered and floating right.

Example:

---
title: "My Title"
subtitle: "Subtitle"
author: "Me"
date:  "1/1/2016"
output:
   bookdown::html_document2: 
     toc: true
     toc_float: true
     toc_depth: 4
     toc_collapsed: true
     number_sections: true
     keep_md: yes
     theme: flatly
     citation_package: biblatex
header-includes: 
- \usepackage{amsmath}
- \usepackage{extarrows}
---
<style type="text/css">
  body{
    font-size: 11pt;
  }
  .tblCaptionBlue > caption{
    color: blue;
  }
  .tblCaptionRed > caption{
    color: red;
  }
</style>

`` `{r cars, fig.cap = "<b>An amazing plot</b>"}``
plot(cars)
`` `

Should be:

Also see https://stackoverflow.com/q/77148220/6574038

Thanks J

## R version 4.3.1 (2023-06-16)
## Platform: x86_64-pc-linux-gnu (64-bit)
## Running under: Ubuntu 22.04.3 LTS
## 
## Matrix products: default
## BLAS:   /usr/lib/x86_64-linux-gnu/blas/libblas.so.3.10.0 
## LAPACK: /usr/lib/x86_64-linux-gnu/lapack/liblapack.so.3.10.0
## 
## locale:
##  [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C              
##  [3] LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8    
##  [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8   
##  [7] LC_PAPER=en_US.UTF-8       LC_NAME=C                 
##  [9] LC_ADDRESS=C               LC_TELEPHONE=C            
## [11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       
## 
## time zone: Europe/Zurich
## tzcode source: system (glibc)
## 
## attached base packages:
## [1] stats     graphics  grDevices utils     datasets  methods   base     
## 
## loaded via a namespace (and not attached):
##  [1] digest_0.6.33     R6_2.5.1          bookdown_0.34     fastmap_1.1.1    
##  [5] xfun_0.40         cachem_1.0.8      knitr_1.44        htmltools_0.5.6  
##  [9] rmarkdown_2.24    cli_3.6.1         sass_0.4.7        jquerylib_0.1.4  
## [13] compiler_4.3.1    rstudioapi_0.15.0 tools_4.3.1       evaluate_0.21    
## [17] bslib_0.5.1       yaml_2.3.7        jsonlite_1.8.7    rlang_1.1.1

By filing an issue to this repo, I promise that

I understand that my issue may be closed if I don't fulfill my promises.

cderv commented 9 months ago

It seems to be a bookdown issue. Minimal reprex

---
title: "My Title"
subtitle: "Subtitle"
author: "Me"
date:  "1/1/2016"
output: bookdown::html_document2
---

```{r cars, fig.cap = "<b>An amazing plot</b>"}
plot(cars)

Note that removing `<b>` around the caption takes solves it. 

Using Markdown to put bold works too: `fig.cap = "**An amazing plot**"`
jay-sf commented 9 months ago

Thanks for prompt answer. Right, there are tags used and wanted. The problem appeared after updating all packages. Tried both, downgrading to rmarkdown_2.24 and bookdown_0.34, but didn't help. Only downgrading to knitr_1.43 solved it, i.e. brought the old behavior shown in second figure back.

cderv commented 9 months ago

The problem comes from c7a423c02246ecea48a24197126d9343e126b9a6 in #2269 to fix #2023

We produce

<div class="figure">
<img src="test_files/figure-html/cars-1.png" alt="<b>An amazing plot</b>"  />
<p class="caption">(\#fig:cars)<b>An amazing plot</b></p>
</div>

and before

<div class="figure">
<img src="test_files/figure-html/cars-1.png" alt="&lt;b&gt;An amazing plot&lt;/b&gt;"  />
<p class="caption">(\#fig:cars)<b>An amazing plot</b></p>
</div>

It seems this change has broken the refs resolving of bookdown

I believe the alt text should contains escaped HTML balise

Right, there are tags used and wanted

An really about this, fig.alt should probably be specified in that case as by default fig.alt will be set to fig.cap, and you don't want HTML tag in fig.alt

We could probably run xfun::strip_html() on fig.alt to insure that...

etiennebacher commented 9 months ago

In the meantime, it seems one can use fig.cap = "**An amazing plot**" instead

cderv commented 9 months ago

Thanks @etiennebacher ! That is what I mentioned in https://github.com/yihui/knitr/issues/2290#issuecomment-1729480818 above.

One could also provide fig.alt also

---
title: "My Title"
subtitle: "Subtitle"
author: "Me"
date:  "1/1/2016"
output: bookdown::html_document2
---

```{r cars, fig.cap = "<b>An amazing plot</b>", fig.alt="An amazing plot"}
plot(cars)


A `alt` text should have no HTML tag, so if some are passed for caption, they should be removed. 

In **knitr** 

* `fig.alt` is set to `fig.cap` is no alt text is provided. 
* `fig.alt` is HTML escaped - which is #2291 fix as this was a regression in 1.44

HTML escape means that `<b>` and `</b>` will still be in the alt text, that is why I think if one need HTML tags in caption, then `fig.alt` should be set manually. 
jay-sf commented 9 months ago

If I understand correctly, fig.alt= is created automatically if not provided and escapes any HTML but this doesn't work well.

We could probably run xfun::strip_html() on fig.alt to insure that...

So I guess quoted suggestion would remove any HTML before creating automated alt text which I think would be desirable. Probably nobody needs tags in alt text, they are designed for screen readers or if images won't load, and both don't need HTML tags.

Just noticed that fig.alt="" also works.

---
title: "My Title"
subtitle: "Subtitle"
author: "Me"
date:  "1/1/2016"
output: bookdown::html_document2
---

```{r cars, fig.cap = "<b>An amazing plot</b>", fig.alt=""}
plot(cars)
```

But this probably isn't W3C conform.

If you could implement a fix, that would be great. I'm sure I'm not the only lazy person who doesn't specify a specific alt text, and you could keep tons of .rmds with gzillions of figures running without having to change their code.

yihui commented 9 months ago

With the PR #2291, special HTML characters will be escaped. If you want to strip HTML tags instead, you can use an option hook:

knitr::opts_hooks$set(fig.cap = function(options) {
  if (is.null(options$fig.alt)) options$fig.alt = options$fig.cap
  options$fig.alt = xfun::strip_html(options$fig.alt)
  options
})
jay-sf commented 9 months ago

Too bad, that does not work. Floating seems fixed, but no figure numbering.

---
title: "My Title"
subtitle: "Subtitle"
author: "Me"
date:  "1/1/2016"
output: bookdown::html_document2
---

```{r, include=FALSE}
knitr::opts_hooks$set(fig.alt = function(options) {
  if (is.null(options$fig.alt)) options$fig.alt = options$fig.cap
  options$fig.alt = xfun::strip_html(options$fig.alt)
  options
})
```

```{r cars, fig.cap = "<b>An amazing plot</b>"}
plot(cars)
```

cderv commented 9 months ago

I think the options hook should be on fig.cap

knitr::opts_hooks$set(fig.cap = function(options) {
  if (is.null(options$fig.alt)) options$fig.alt = options$fig.cap
  options$fig.alt = xfun::strip_html(options$fig.alt)
  options
})

And you can install dev version of knitr which have the escape fix now since #2291 is merged

jay-sf commented 9 months ago

Works, thanks very much.

github-actions[bot] commented 3 months ago

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue by following the issue guide (https://yihui.org/issue/), and link to this old issue if necessary.