Gt Word escaping potentially invalid chars

[thebioengineer]

Summary

Thank you for contributing to gt! To make this process easier for everyone, please explain the context and purpose of your contribution. Also, list the changes made to the existing code or documentation.

I found that if there were carrots (>,<) in the text of cells, it would cause the document not to render correctly. Adding this in escapes this and other potentially impactful symbols using htmltools::htmlEscape. If you can come up with something else I am all ears, because this will prevent people from passing their own valid ooxml code through gt to word.

Also realized the code to add a summary rows added the bottom row lines after the first row.

Related GitHub Issues and PRs

• Ref: # N/A

Checklist

• I understand and agree to the Code of Conduct.
• I have listed any major changes in the NEWS.
• I have added testthat unit tests to tests/testthat for any new functionality.

[thebioengineer]

Hey @rich-iannone , I'd appreciate your thoughts on using htmlEscape vs a more targeted solution

[rich-iannone]

@thebioengineer I think using htmlEscape() here is the right choice.

I've lightly re-styled the .R file and merged in the dev branch. However, from that, a test is failing. On my machine, anyway. Could you pull from the changes here, make some manual tests, and fix if necessary? Seems fine on CI so I wonder if one of my packages is out of date. (I'll keep trying to figure this out in the meantime.)

[thebioengineer]

I think you are getting a failing test locally but not on CI because there are skips of tests on CI and CRAN, but not locally.

There is now a bug in that because cells have footnote tags added before they are processed by xml_t(). Since the footnote tags are written in as a string representing xml code, the string is escaped as text. I need to think a moment on how to fix this

[thebioengineer]

I had an idea...there is the process_text() function that blindly returns text (or processes the markdown text to then normal text). I can apply the htmlEscape there and that is honestly probably a better location than in xml_t() - which makes more sense to not modify text

[rich-iannone]

Nice! I'll check out the branch and test locally here as well.

[rich-iannone]

Perhaps it hasn't been addressed yet (or maybe it's different on your system) but I get a test error with this statement (line 749 of test-as_word.R):

  expect_equal(
    docx_table_body_header %>%
      xml2::xml_find_all(".//w:p") %>%
      xml2::xml_text(),
    c(
      "numtrue1false", "chartrue2false", "fctr", "date", "time",
      "datetime", "currency", "row", "group"
    )
  )

Error output is:

Failure (test-as_word.R:749:3): tables with footnotes can be added to a word doc
docx_table_body_header %>% xml2::xml_find_all(".//w:p") %>% xml2::xml_text() (`actual`) not equal to c(...) (`expected`).

actual[1:5] vs expected[1:5]
- "num<w:rPr><w:vertAlign w:val=\"superscript\"></w:vertAlign><w:i>true</w:i><w:t xml:space=\"default\">1</w:t><w:i>false</w:i><w:vertAlign w:val=\"baseline\"></w:vertAlign></w:rPr>"
+ "numtrue1false"
- "char<w:rPr><w:vertAlign w:val=\"superscript\"></w:vertAlign><w:i>true</w:i><w:t xml:space=\"default\">2</w:t><w:i>false</w:i><w:vertAlign w:val=\"baseline\"></w:vertAlign></w:rPr>"
+ "chartrue2false"
  "fctr"
  "date"
  "time"

[thebioengineer]

Perhaps it hasn't been addressed yet (or maybe it's different on your system) but I get a test error with this statement (line 749 of test-as_word.R):

  expect_equal(
    docx_table_body_header %>%
      xml2::xml_find_all(".//w:p") %>%
      xml2::xml_text(),
    c(
      "numtrue1false", "chartrue2false", "fctr", "date", "time",
      "datetime", "currency", "row", "group"
    )
  )

Error output is:

Failure (test-as_word.R:749:3): tables with footnotes can be added to a word doc
docx_table_body_header %>% xml2::xml_find_all(".//w:p") %>% xml2::xml_text() (`actual`) not equal to c(...) (`expected`).

actual[1:5] vs expected[1:5]
- "num<w:rPr><w:vertAlign w:val=\"superscript\"></w:vertAlign><w:i>true</w:i><w:t xml:space=\"default\">1</w:t><w:i>false</w:i><w:vertAlign w:val=\"baseline\"></w:vertAlign></w:rPr>"
+ "numtrue1false"
- "char<w:rPr><w:vertAlign w:val=\"superscript\"></w:vertAlign><w:i>true</w:i><w:t xml:space=\"default\">2</w:t><w:i>false</w:i><w:vertAlign w:val=\"baseline\"></w:vertAlign></w:rPr>"
+ "chartrue2false"
  "fctr"
  "date"
  "time"

Yes, this should be addressed now...it was because of htmlEscape being applied at the xml_t() location, instead of escaping the text earlier.

[rich-iannone]

Looks great!