Fix for extra linefeeds in CSS/Script includes #122

dcomtois · 2019-01-07T00:07:33Z

I changed the collapse= value from "\r\n" to "\n" to prevent having empty lines created in the source HTML.

For instance, before this fix, notice the extra empty lines (which do not reflect the true content of bootstrap.min.css)

<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8"/>

  <title>Some Title</title>
  <style type="text/css">/*!

 * Bootstrap v4.1.2 (https://getbootstrap.com/)

 * Copyright 2011-2018 The Bootstrap Authors

 * Copyright 2011-2018 Twitter, Inc.

 * Licensed under MIT (https://github.com/twbs/bootstrap/blob/master/LICENSE)

After the fix, all is normal. Tested on both Windows and Linux,

<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8"/>

  <title>Some Title</title>
  <style type="text/css">/*!
 * Bootstrap v4.1.2 (https://getbootstrap.com/)
 * Copyright 2011-2018 The Bootstrap Authors
 * Copyright 2011-2018 Twitter, Inc.
 * Licensed under MIT (https://github.com/twbs/bootstrap/blob/master/LICENSE)

… in source html

wch · 2019-01-07T15:42:40Z

The newlines do have \r\n, but there are not extra empty lines in the output. For example:

library(htmltools)
s <- includeCSS(
  system.file(package="shiny", "www/shared/bootstrap/css/bootstrap.css")
)

s1 <- substr(s, 1, 200)
print(s1)
#> [1] "<style type=\"text/css\">/*!\r\n * Bootstrap v3.3.7 (http://getbootstrap.com)\r\n * Copyright 2011-2016 Twitter, Inc.\r\n * Licensed under MIT (https://github.com/twbs/bootstrap/blob/master/LICENSE)\r\n */\r\n/*!"
cat(s1)
#> <style type="text/css">/*!
#>  * Bootstrap v3.3.7 (http://getbootstrap.com)
#>  * Copyright 2011-2016 Twitter, Inc.
#>  * Licensed under MIT (https://github.com/twbs/bootstrap/blob/master/LICENSE)
#>  */
#> /*!

What program are you using to view the result?

dcomtois · 2019-01-07T17:13:26Z

I am viewing it either in a browser ("show source"), or in np++, where it shows doubled CR-LF... I use htmltools in the following way:

    html_content <-
      tags$div(
        class="container st-container",
        tags$head(
          tags$title(HTML(conv_non_ascii(report.title))),
          if (isTRUE(bootstrap.css))
            includeCSS(
              path = paste(stpath, "includes/stylesheets/bootstrap.min.css",
                           sep="/")),
          includeCSS(
            path = paste(stpath, "includes/stylesheets/summarytools.css",
                         sep="/")),
          if (!is.na(custom.css)) 
            includeCSS(path = custom.css)
        ),
        res)
    # and then a bit later:
     save_html(html = html_content, file = outfile_path)

I use this in summarytools, and the rest of HTML code is formatted correctly... only the CSS bits have blank lines inserted. For instance, if you try:

install.packages("summarytools")
view(dfSummary(iris), method = "browser")

and show source, you'll see it.

EDIT
I noticed that in shiny's css directory, the files have LF line endings, while in summarytools, the endings are CRLF. I thought it might be related, but even after converting to LF only, I still get the same results :\

wch · 2019-01-07T21:59:11Z

I just tested this on Mac and Windows. I see the problem in Windows (in Edge, but not in Chrome), but not on my Mac (on Chrome, Firefox, or Safari).
When I run:

library(summarytools)
view(dfSummary(iris), method = "browser")

This is what it looks like in Chrome and Edge on Windows:

And on Chrome, Firefox, and Safari on Mac:

On Windows, when I open the output file in a text editor, I see 0d0d0a, which is \r\r\n, or CR-CR-LF.

However, when I use the example from my previous comment, but this time take bootstrap.css from summarytools and run it in Windows, I see the same output, with just \r\n:

library(htmltools)
s <- includeCSS(
  system.file(package="summarytools", "includes/stylesheets/bootstrap.css")
)
s1 <- substr(s, 1, 200)
print(s1)
#> [1] "<style type=\"text/css\">/*!\r\n * Bootstrap v4.1.2 (https://getbootstrap.com/)\r\n * Copyright 2011-2018 The Bootstrap Authors\r\n * Copyright 2011-2018 Twitter, Inc.\r\n * Licensed under MIT (https://github.c"
cat(s1)
#> <style type="text/css">/*!
#>  * Bootstrap v4.1.2 (https://getbootstrap.com/)
#>  * Copyright 2011-2018 The Bootstrap Authors
#>  * Copyright 2011-2018 Twitter, Inc.
#>  * Licensed under MIT (https://github.c

So, I agree that there is a problem in Windows when I run your example. However, the proposed fix, which involves changing includeCSS, doesn't seem to get at the source of the problem. Somewhere, there's an extra \r getting in there, but it's not from includeCSS.

If you can find a minimal example that involves only the htmltools package, that would help.

dcomtois · 2019-01-07T22:27:12Z

Thanks for putting in the the time and effort to try and get to the bottom of this. I understand your position; I'll try to isolate the problem and keep you posted.

kevinushey · 2019-01-07T22:38:41Z

If the file is being written to a text connection, R will automatically translate \n to \r\n linefeeds on Windows, which could explain why \r\n is translated to \r\r\n. For example:

library(htmltools)
s <- includeCSS(
  system.file(package="shiny", "www/shared/bootstrap/css/bootstrap.css")
)

file <- tempfile()
text <- as.character(s)
writeLines(text, con = file, useBytes = TRUE)
readChar(file, file.info(file)$size, TRUE)

You'll see the duplicated \r\r\n linefeeds in the generated file. If you want to write literal \r\n separators I think you need to open the file with a binary connection.

dcomtois · 2019-01-07T23:17:11Z

You'll see the duplicated \r\r\n linefeeds in the generated file. If you want to write literal \r\n separators I think you need to open the file with a binary connection.

Thanks for the suggestion, certainly worth a try... I'd rather have only \n's but it might not be possible.

dcomtois · 2019-01-09T13:04:57Z

@kevinushey you were spot on.

I took only the first few lines of the bootstrap css and called it "bogus.css", with CRLF endings.

bogus_CRLF <- readChar("bogus_CRLF.css", nchars = 300)
capture.output(cat(bogu_sCRLF), file = "bogus_out.css")

Reading in HEX mode, we see CR's (Hex 0D) are doubled indeed

2F2A210D0D0A202A...

When writing in binary mode, all is fine (i.e. no doubling of CR's).

This is a rather annoying flaw in R's core... I didn't see it when googling the issue... Only thing I found has to do with C:
https://stackoverflow.com/questions/33415598/0a-converting-to-0d0a-in-c

Filing a bug report might be the appropriate step to take? What do you guys think?

In the meantime I plan on redefining save_html() in my package, in order to write in binary mode.

kevinushey · 2019-01-09T15:22:05Z

I don't think this is a bug; it's the expected and documented behavior when writing \n newlines to a text connection. It's just perhaps a bit surprising when you first see it.

That said, I don't understand why htmltools wants to use \r\n line endings in these cases; I think just using \n would be fine? If not, then I think these components should still be pasted with \n but translation to \r\n should then happen in save_html().

wch · 2019-01-09T21:10:43Z

I agree, it's not clear to me why \r\n is desirable.

@jcheng5 do you know why it's used there? Here's where it came from:
7ae40c6#diff-72abdc1c2c5a6d65974d57ef9bd3666cR827

jcheng5 · 2019-01-09T21:35:44Z

Nope. Let's get rid of it

…tmltools#122

dcomtois · 2019-01-10T18:28:49Z

Thx for this, it should make things much cleaner on Windows.

@yihui Pls note that in my pull request, the \r\n I was trying to address was in includeCSS() (and also in includeScript(), not in paste2()... Or is the plan is to commit this PR as well?

@kevinushey Can we really say it's "intended" though? I mean who would want to intentionally produce \r\r\n's? Seems to me the logical way would check if \n is preceded by \r, and if not, then insert the \r!

yihui · 2019-01-10T18:42:29Z

@dcomtois Please feel free to ignore the change in my servr package. It doesn't have anything to do with htmltools. I blindly copied \r\n from somewhere (perhaps httpuv or Rook) and always thought it was weird.

jcheng5 · 2019-08-26T18:58:22Z

We'll need to re-investigate this to make sure the whitespace eating changes didn't change the behavior in an undesirable way. (Because I think we might be writing directly to a binary connection now, I'm not sure.)

Also, we'll need to add unit tests.

jcheng5 · 2019-09-04T16:00:50Z

I've been trying for 30 minutes now but for the life of me I can't reproduce this issue. I'm using both summarytools and htmltools from CRAN, and I also tried summarytools 0.9.3.

I can reproduce something similar enough though:

tf <- tempfile(fileext = ".html")
htmltools::save_html(htmltools::HTML("\r\n"), tf)
grepl("\r\r\n", readChar(tf, file.info(tf)$size, TRUE))

Rather than (or in addition to) eliminating \r\n from being introduced by includeCSS, I'd like to prevent \r\n from being converted to \r\r\n, period.

wch · 2019-09-04T16:32:46Z

Update: the reason it won't repro is because summarytools made this change:
dcomtois/summarytools@a885413

jcheng5 · 2019-09-05T17:12:13Z

Thanks for this @dcomtois. I ended up doing a different fix in #137.

dcomtois added 2 commits January 6, 2019 16:33

Changed collapse value from "\r\n" to "\n" to avoid double line feeds…

09439f0

… in source html

Changed collapse value from "\r\n" to "\n" to avoid double line feeds…

8662d00

… in source html

yihui added a commit to yihui/servr that referenced this pull request Jan 10, 2019

nobody knows why \r\n is necessary, so let me get rid of it rstudio/h…

d86b803

…tmltools#122

wch added this to the v0.4.0 milestone Aug 26, 2019

jcheng5 mentioned this pull request Sep 4, 2019

Stop extra carriage returns from being inserted on windows #137

Merged

jcheng5 closed this Sep 5, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix for extra linefeeds in CSS/Script includes #122

Fix for extra linefeeds in CSS/Script includes #122

dcomtois commented Jan 7, 2019 •

edited

Loading

wch commented Jan 7, 2019

dcomtois commented Jan 7, 2019 •

edited

Loading

wch commented Jan 7, 2019 •

edited

Loading

dcomtois commented Jan 7, 2019

kevinushey commented Jan 7, 2019

dcomtois commented Jan 7, 2019 •

edited

Loading

dcomtois commented Jan 9, 2019 •

edited

Loading

kevinushey commented Jan 9, 2019

wch commented Jan 9, 2019

jcheng5 commented Jan 9, 2019

dcomtois commented Jan 10, 2019 •

edited

Loading

yihui commented Jan 10, 2019

jcheng5 commented Aug 26, 2019 •

edited

Loading

jcheng5 commented Sep 4, 2019

wch commented Sep 4, 2019

jcheng5 commented Sep 5, 2019

Fix for extra linefeeds in CSS/Script includes #122

Fix for extra linefeeds in CSS/Script includes #122

Conversation

dcomtois commented Jan 7, 2019 • edited Loading

wch commented Jan 7, 2019

dcomtois commented Jan 7, 2019 • edited Loading

wch commented Jan 7, 2019 • edited Loading

dcomtois commented Jan 7, 2019

kevinushey commented Jan 7, 2019

dcomtois commented Jan 7, 2019 • edited Loading

dcomtois commented Jan 9, 2019 • edited Loading

kevinushey commented Jan 9, 2019

wch commented Jan 9, 2019

jcheng5 commented Jan 9, 2019

dcomtois commented Jan 10, 2019 • edited Loading

yihui commented Jan 10, 2019

jcheng5 commented Aug 26, 2019 • edited Loading

jcheng5 commented Sep 4, 2019

wch commented Sep 4, 2019

jcheng5 commented Sep 5, 2019

dcomtois commented Jan 7, 2019 •

edited

Loading

dcomtois commented Jan 7, 2019 •

edited

Loading

wch commented Jan 7, 2019 •

edited

Loading

dcomtois commented Jan 7, 2019 •

edited

Loading

dcomtois commented Jan 9, 2019 •

edited

Loading

dcomtois commented Jan 10, 2019 •

edited

Loading

jcheng5 commented Aug 26, 2019 •

edited

Loading