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

Using url in CSS argument throws an error now #2163

Closed
cderv opened this issue Jun 7, 2021 · 12 comments · Fixed by #2164
Closed

Using url in CSS argument throws an error now #2163

cderv opened this issue Jun 7, 2021 · 12 comments · Fixed by #2164
Labels
bug an unexpected problem or unintended behavior

Comments

@cderv
Copy link
Collaborator

cderv commented Jun 7, 2021

This is a regression we have

Reproducible example :

dir.create(tmp_dir <- tempfile())
owd <- setwd(tmp_dir)
xfun::write_utf8(c(
  "---", 
  "title: test",
  "---", 
  "",
  "# test"), "test.Rmd")

rmarkdown::render("test.Rmd", 
                  rmarkdown::html_document(
                    css = "http://netdna.bootstrapcdn.com/font-awesome/4.0.3/css/font-awesome.css"
                  )
)

setwd(owd)
unlink(tmp_dir, TRUE)
Could not fetch http:/netdna.bootstrapcdn.com/font-awesome/4.0.3/css/font-awesome.css
InvalidUrlException "http:/netdna.bootstrapcdn.com/font-awesome/4.0.3/css/font-awesome.css" "URL must be absolute"
Error: pandoc document conversion failed with error 61
Run `rlang::last_error()` to see where the error occurred.

It must come from the new sass support in css arg I believe.

This was first reported in yihui/knitr#1864 (comment)

@cderv cderv added the bug an unexpected problem or unintended behavior label Jun 7, 2021
@cderv
Copy link
Collaborator Author

cderv commented Jun 7, 2021

A quick git bisect shows that it happens in 98ba0e2 (#2095)

@cderv
Copy link
Collaborator Author

cderv commented Jun 7, 2021

Workaround for those stuck by this in rmarkdown 2.8 : pass the CSS arg asis to Pandoc

output:
    html_document:
        pandoc_args: ["--css", "http://netdna.bootstrapcdn.com/font-awesome/4.0.3/css/font-awesome.css"]

@vnijs @yihui this could solve the issue in yihui/knitr#1864 (comment) without waiting for a new CRAN release.

Sorry for the trouble.

@yihui
Copy link
Member

yihui commented Jun 7, 2021

No worries! Thanks for the investigation and fix! Sounds like we'd better make a CRAN release relatively soon.

@cderv
Copy link
Collaborator Author

cderv commented Jun 7, 2021

Yes sounds like it as this is a regression. This is the type of issue that makes it hard to plan releases 😅

However, I am surprised there was no issue open about this earlier. Using url in css argument does not seem to be used that often - I would have thought otherwise. So maybe this is not so urgent.

@yihui
Copy link
Member

yihui commented Jun 7, 2021

Without our CRAN release, it would hold @vnijs's release of radiant. It's been a month since the last release. If we were to use my previous release schedule, it's time to consider a release now, even if nothing is urgent.

@cderv
Copy link
Collaborator Author

cderv commented Jun 7, 2021

Without our CRAN release, it would hold @vnijs's release of radiant

That was why I mentioned the workaround above #2163 (comment) that would work on radiant's vignette I think. Aim is to not block radiant.

But for sure we can do another release - no problem with that. as you said it is time and we have important fix in the current 2.9 version that would be good to get on CRAN 👍

I let you handle that and ping me if I can help with anything. Thanks !

@yihui
Copy link
Member

yihui commented Jun 7, 2021

@vnijs Would you like to use the workaround or prefer us updating rmarkdown on CRAN?

@cderv I just looked at NEWS.md, and personally I'm particularly excited about the fact that HTML widgets could use href components instead of being limited to file. IMO, that fix could justify a release. Usually I tend to make releases on a monthly basis if there are substantial bug fixes, so that no more users will be bitten by the same bugs that have already been fixed. This CSS bug is relatively new, and probably not many people would run into it, I'm still a little bit concerned if there really is only one package affected on CRAN. It would be too late and scary if I get a CRAN email informing me that the previous version broken other packages.

vnijs added a commit to radiant-rstats/radiant that referenced this issue Jun 8, 2021
* (R)markdown in Suggests
* vignette workaround for rstudio/rmarkdown#2163
@vnijs
Copy link

vnijs commented Jun 8, 2021

Thanks @cderv. Just got confirmation that radiant is on its way to CRAN with the workaround!

@cderv
Copy link
Collaborator Author

cderv commented Jun 8, 2021

I just looked at NEWS.md, and personally I'm particularly excited about the fact that HTML widgets could use href components instead of being limited to file. IMO, that fix could justify a release.

Exactly, that it is what I had in mind by "we have important fix in the current 2.9 version that would be good to get on CRAN" 😄 This one is important.

Just got confirmation that radiant is on its way to CRAN with the workaround!

@vnijs glad it help you in the short term. We'll get back to you when the fix is on CRAN so that you can change back to a more traditional YAML syntax.

@pwildenhain
Copy link

pwildenhain commented Jun 15, 2021

Just to chime in, we noticed this at our company where we take advantage of using a url for the css arg in the yaml header, to get our company themes/colors in all of our Rmarkdown reporting.

A release would be greatly appreciated 🙇🏻 👍🏻

@cderv
Copy link
Collaborator Author

cderv commented Jun 15, 2021

Hi @pwildenhain !

You're right in time! We already pushed to CRAN (today!) the release with this fix : https://github.com/rstudio/rmarkdown/releases/tag/v2.9

On CRAN already: https://cran.r-project.org/web/packages/rmarkdown/index.html

Should be available as binaries in a few days.

jonathan-g added a commit to jonathan-g/rmarkdown that referenced this issue Jun 18, 2021
* rstudio_origin/master: (90 commits)
  start the next version
  CRAN release v2.9
  fix rstudio#2163: do not normalize web paths in the css argument of html output formats (rstudio#2164)
  Add a css file for future tests
  Restore previous shiny theme when run() exits (rstudio#2160)
  Correct name in workflow file
  Add a workflow to test Pandoc nighly (rstudio#2153)
  bslib 0.2.5.1 is now on CRAN
  use `stop2()` from knitr internal instead of `stop(..., call. = FALSE)` (rstudio#2152)
  74e2f2f changed the error message, and a test relies on the content of this message (it probably shouldn't be so specific)
  tweak wording
  Support 'href' dependencies (rstudio#2151)
  Loop correctly on splitted file by file_scope function (rstudio#2150)
  update snapshot file following change in test name in a8aad75
  amend 84ff469: clarify the meaning of fig_crop = 'auto' in the doc
  Add the position of the last character (rstudio#2146)
  don't put the checklist in the comment since it is required
  Fix for lost encoding in shiny_prerendered_html. (rstudio#2140)
  Insure fontawesome V4 compatibility in navbar (rstudio#1994)
  fix rstudio#2043: replace parse(text) with xfun::parse_only() to avoid hanging the R session when the input is empty
  ...
jonathan-g added a commit to jonathan-g/rmarkdown that referenced this issue Jun 18, 2021
Merge remote-tracking branch 'rstudio_origin/master' into jg-devel

* rstudio_origin/master: (64 commits)
  start the next version
  CRAN release v2.9
  fix rstudio#2163: do not normalize web paths in the css argument of html output formats (rstudio#2164)
  Add a css file for future tests
  Restore previous shiny theme when run() exits (rstudio#2160)
  Correct name in workflow file
  Add a workflow to test Pandoc nighly (rstudio#2153)
  bslib 0.2.5.1 is now on CRAN
  use `stop2()` from knitr internal instead of `stop(..., call. = FALSE)` (rstudio#2152)
  74e2f2f changed the error message, and a test relies on the content of this message (it probably shouldn't be so specific)
  tweak wording
  Support 'href' dependencies (rstudio#2151)
  Loop correctly on splitted file by file_scope function (rstudio#2150)
  update snapshot file following change in test name in a8aad75
  amend 84ff469: clarify the meaning of fig_crop = 'auto' in the doc
  Add the position of the last character (rstudio#2146)
  don't put the checklist in the comment since it is required
  Fix for lost encoding in shiny_prerendered_html. (rstudio#2140)
  Insure fontawesome V4 compatibility in navbar (rstudio#1994)
  fix rstudio#2043: replace parse(text) with xfun::parse_only() to avoid hanging the R session when the input is empty
  ...

# Conflicts:
#	DESCRIPTION
#	R/html_dependencies.R
jonathan-g added a commit to jonathan-g/rmarkdown that referenced this issue Jun 18, 2021
Merge remote-tracking branch 'rstudio_origin/master' into minimal-tree-fix

* rstudio_origin/master: (223 commits)
  start the next version
  CRAN release v2.9
  fix rstudio#2163: do not normalize web paths in the css argument of html output formats (rstudio#2164)
  Add a css file for future tests
  Restore previous shiny theme when run() exits (rstudio#2160)
  Correct name in workflow file
  Add a workflow to test Pandoc nighly (rstudio#2153)
  bslib 0.2.5.1 is now on CRAN
  use `stop2()` from knitr internal instead of `stop(..., call. = FALSE)` (rstudio#2152)
  74e2f2f changed the error message, and a test relies on the content of this message (it probably shouldn't be so specific)
  tweak wording
  Support 'href' dependencies (rstudio#2151)
  Loop correctly on splitted file by file_scope function (rstudio#2150)
  update snapshot file following change in test name in a8aad75
  amend 84ff469: clarify the meaning of fig_crop = 'auto' in the doc
  Add the position of the last character (rstudio#2146)
  don't put the checklist in the comment since it is required
  Fix for lost encoding in shiny_prerendered_html. (rstudio#2140)
  Insure fontawesome V4 compatibility in navbar (rstudio#1994)
  fix rstudio#2043: replace parse(text) with xfun::parse_only() to avoid hanging the R session when the input is empty
  ...

# Conflicts:
#	DESCRIPTION
#	NEWS.md
#	R/html_dependencies.R
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants