From adb0beddcfde5844a5b160ab5f27b9f2f98a5dd9 Mon Sep 17 00:00:00 2001
From: Carson <cpsievert1@gmail.com>
Date: Wed, 2 Dec 2020 10:55:02 -0600
Subject: [PATCH 1/4] parseCssColors(): treat incoming NAs as an invalid color
 specification (instead of throwing with an unintelligible error. Closes #161

---
 R/colors.R                   |  7 +++++++
 tests/testthat/test-colors.R | 16 ++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/R/colors.R b/R/colors.R
index 4db11d03..8397073d 100644
--- a/R/colors.R
+++ b/R/colors.R
@@ -30,6 +30,13 @@
 #' @md
 #' @export
 parseCssColors <- function(str, mustWork = TRUE) {
+  # Logic below assumes a character string with non-missing values
+  # Note that an empty string is not a valid color, so parsing fails
+  # on NA input values, and thus, will be converted back to NA
+  # when `mustWork = FALSE`
+  stopifnot(is.character(str))
+  str[is.na(str)] <- ""
+
   # Strip insignificant whitespace
   str <- color_strip_ws(str)
 
diff --git a/tests/testthat/test-colors.R b/tests/testthat/test-colors.R
index 7188d6b9..f7036ce7 100644
--- a/tests/testthat/test-colors.R
+++ b/tests/testthat/test-colors.R
@@ -119,3 +119,19 @@ test_that("decode_color_keyword", {
   expect_error(decode_color_keyword(" orange "))
   expect_error(decode_color_keyword(NA))
 })
+
+test_that("parseCssColors() handles incoming NA values sensibly", {
+  expect_error(parseCssColors(NA))
+  expect_equal(
+    parseCssColors(NA, mustWork = FALSE),
+    NA_character_
+  )
+  expect_identical(
+    parseCssColors(c(NA, "red"), mustWork = FALSE),
+    c(NA, "#FF0000")
+  )
+  expect_identical(
+    parseCssColors(c(NA, "blue", "notacolor"), mustWork = FALSE),
+    c(NA, "#0000FF", NA)
+  )
+})

From 7501ae390a6859747185c11f960348ae0d038fc5 Mon Sep 17 00:00:00 2001
From: Carson <cpsievert1@gmail.com>
Date: Wed, 2 Dec 2020 11:39:07 -0600
Subject: [PATCH 2/4] fix unit test and update news

---
 NEWS                         | 2 ++
 tests/testthat/test-colors.R | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 05fb1dec..14f84072 100644
--- a/NEWS
+++ b/NEWS
@@ -13,6 +13,8 @@ htmltools 0.5.0.9002
 
 * Closed #189: `validateCssUnit()` now accepts `fit-content`. (#190)
 
+* Closed #161: `parseCssColors(x, mustWork = FALSE)` now returns NA for NA input values instead of throwing an error. (#194)
+
 * `htmlPreserve()` can now optionally use the Pandoc `raw_attribute` extension to enclose HTML.
 
 htmltools 0.5.0
diff --git a/tests/testthat/test-colors.R b/tests/testthat/test-colors.R
index f7036ce7..2cad926a 100644
--- a/tests/testthat/test-colors.R
+++ b/tests/testthat/test-colors.R
@@ -122,8 +122,8 @@ test_that("decode_color_keyword", {
 
 test_that("parseCssColors() handles incoming NA values sensibly", {
   expect_error(parseCssColors(NA))
-  expect_equal(
-    parseCssColors(NA, mustWork = FALSE),
+  expect_identical(
+    parseCssColors(NA_character_, mustWork = FALSE),
     NA_character_
   )
   expect_identical(

From 75b8c3cbebc91292b2241a0cbf8b23eb8fba7691 Mon Sep 17 00:00:00 2001
From: Carson <cpsievert1@gmail.com>
Date: Wed, 2 Dec 2020 11:58:05 -0600
Subject: [PATCH 3/4] also allow for singular generic NA value

---
 R/colors.R                   | 4 +++-
 tests/testthat/test-colors.R | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/R/colors.R b/R/colors.R
index 8397073d..409d228a 100644
--- a/R/colors.R
+++ b/R/colors.R
@@ -34,7 +34,9 @@ parseCssColors <- function(str, mustWork = TRUE) {
   # Note that an empty string is not a valid color, so parsing fails
   # on NA input values, and thus, will be converted back to NA
   # when `mustWork = FALSE`
-  stopifnot(is.character(str))
+  if (!(is.character(str) || rlang::is_na(str))) {
+    stop("`str` must be a character vector (or NA).")
+  }
   str[is.na(str)] <- ""
 
   # Strip insignificant whitespace
diff --git a/tests/testthat/test-colors.R b/tests/testthat/test-colors.R
index 2cad926a..01a9bb50 100644
--- a/tests/testthat/test-colors.R
+++ b/tests/testthat/test-colors.R
@@ -123,7 +123,7 @@ test_that("decode_color_keyword", {
 test_that("parseCssColors() handles incoming NA values sensibly", {
   expect_error(parseCssColors(NA))
   expect_identical(
-    parseCssColors(NA_character_, mustWork = FALSE),
+    parseCssColors(NA, mustWork = FALSE),
     NA_character_
   )
   expect_identical(

From 5e64737e2cec84c8752ad31c951f969e94ac9d16 Mon Sep 17 00:00:00 2001
From: Carson <cpsievert1@gmail.com>
Date: Wed, 2 Dec 2020 12:10:49 -0600
Subject: [PATCH 4/4] handle a vector of NAs as well

---
 R/colors.R                   | 5 +++--
 tests/testthat/test-colors.R | 4 ++++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/R/colors.R b/R/colors.R
index 409d228a..47776687 100644
--- a/R/colors.R
+++ b/R/colors.R
@@ -34,10 +34,11 @@ parseCssColors <- function(str, mustWork = TRUE) {
   # Note that an empty string is not a valid color, so parsing fails
   # on NA input values, and thus, will be converted back to NA
   # when `mustWork = FALSE`
-  if (!(is.character(str) || rlang::is_na(str))) {
+  isNA <- is.na(str)
+  if (!(is.character(str) || all(isNA))) {
     stop("`str` must be a character vector (or NA).")
   }
-  str[is.na(str)] <- ""
+  str[isNA] <- ""
 
   # Strip insignificant whitespace
   str <- color_strip_ws(str)
diff --git a/tests/testthat/test-colors.R b/tests/testthat/test-colors.R
index 01a9bb50..d765e67e 100644
--- a/tests/testthat/test-colors.R
+++ b/tests/testthat/test-colors.R
@@ -126,6 +126,10 @@ test_that("parseCssColors() handles incoming NA values sensibly", {
     parseCssColors(NA, mustWork = FALSE),
     NA_character_
   )
+  expect_identical(
+    parseCssColors(rep(NA, 2), mustWork = FALSE),
+    rep(NA_character_, 2)
+  )
   expect_identical(
     parseCssColors(c(NA, "red"), mustWork = FALSE),
     c(NA, "#FF0000")