Theme Preferences for Distilled Pages

components/dom_distiller/core/viewer.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/dom_distiller/core/viewer.h"
 
 #include <string>
 #include <vector>
 
 #include "base/json/json_writer.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/string_util.h"
 #include "components/dom_distiller/core/dom_distiller_service.h"
 #include "components/dom_distiller/core/proto/distilled_article.pb.h"
 #include "components/dom_distiller/core/proto/distilled_page.pb.h"
+#include "components/dom_distiller/core/reader_mode_preferences.h"
 #include "components/dom_distiller/core/task_tracker.h"
 #include "components/dom_distiller/core/url_constants.h"
 #include "components/dom_distiller/core/url_utils.h"
 #include "grit/component_resources.h"
 #include "grit/components_strings.h"
 #include "net/base/escape.h"
 #include "net/url_request/url_request.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "url/gurl.h"
 
 namespace dom_distiller {
 
 namespace {
 
+const char kWhite[] = "#FFF";

[nyquist, 2014/06/23 16:03:06]

Nit: I think these two can go away if you follow battre's comment.

[smaslo, 2014/06/23 22:14:33]

Done.

+const char kBlack[] = "#000";
+
+std::string GetBodyInsertionText(ReaderModePrefs* reader_mode_prefs) {

[nyquist, 2014/06/23 16:03:06]

Is there a way of writing a test to ensure we use the correct background-color
and color for the CSS?

[smaslo, 2014/06/23 22:14:33]

The only way I've heard so far is doing a string test to see if the CSS file
contains the correct string.  For example, we could test if the CSS file
contains "background-color: #000;". However, this method seems rather fragile.

On 2014/06/23 16:03:06, nyquist wrote:

+  if (reader_mode_prefs->GetHighContrastPref()) {
+    std::string body_insertion_text = "background-color: ";
+    body_insertion_text += kBlack;
+    body_insertion_text += ";\ncolor: ";
+    body_insertion_text += kWhite;
+    body_insertion_text += ";";
+    return body_insertion_text;
+  }
+  else {
+    std::string body_insertion_text = "background-color: ";
+    body_insertion_text += kWhite;
+    body_insertion_text += ";\ncolor: ";
+    body_insertion_text += kBlack;
+    body_insertion_text += ";";
+    return body_insertion_text;
+   }

[battre, 2014/06/23 08:37:23]

Opt:

Wouldn't this be more readable if you phrase it as
if (reader_mode_prefs->GetHighContrastPref())
  return "background-color: #000;\ncolor: #FFF;";  // White on black.
else
  return "background-color: #FFF;\ncolor: #000;";  // Black on white.

Note that you don't add a \n after the color.

[smaslo, 2014/06/23 22:14:33]

Done.

+}
+
 std::string ReplaceHtmlTemplateValues(
     const std::string& title,
     const std::string& content,
     const std::string& loading_indicator_class,
     const std::string& original_url) {
   base::StringPiece html_template =
       ResourceBundle::GetSharedInstance().GetRawDataResource(
           IDR_DOM_DISTILLER_VIEWER_HTML);
   std::vector<std::string> substitutions;
   substitutions.push_back(title);                                         // $1
   substitutions.push_back(kViewerCssPath);                                // $2
   substitutions.push_back(kViewerJsPath);                                 // $3
   substitutions.push_back(title);                                         // $4
   substitutions.push_back(content);                                       // $5
   substitutions.push_back(loading_indicator_class);                       // $6
   substitutions.push_back(
       l10n_util::GetStringUTF8(IDS_DOM_DISTILLER_VIEWER_LOADING_STRING)); // $7
   substitutions.push_back(original_url);                                  // $8
   substitutions.push_back(
       l10n_util::GetStringUTF8(IDS_DOM_DISTILLER_VIEWER_VIEW_ORIGINAL));  // $9
   return ReplaceStringPlaceholders(html_template, substitutions, NULL);
 }
 
+std::string ReplaceCSSTemplateValues(ReaderModePrefs* reader_mode_prefs) {
+  std::vector<std::string> substitutions;
+  substitutions.push_back(GetBodyInsertionText(reader_mode_prefs));
+  base::StringPiece css_template =
+      ResourceBundle::GetSharedInstance().GetRawDataResource(
+          IDR_DISTILLER_CSS);
+  return ReplaceStringPlaceholders(css_template, substitutions, NULL);
+}
+
 }  // namespace
 
 namespace viewer {
 
 const std::string GetUnsafeIncrementalDistilledPageJs(
     const DistilledPageProto* page_proto,
     const bool is_last_page) {
   std::string output;
   base::StringValue value(page_proto->html());
   base::JSONWriter::Write(&value, &output);
@@ skipping 56 matching lines @@
 }
 
 const std::string GetErrorPageHtml() {
   std::string title = l10n_util::GetStringUTF8(
       IDS_DOM_DISTILLER_VIEWER_FAILED_TO_FIND_ARTICLE_TITLE);
   std::string content = l10n_util::GetStringUTF8(
       IDS_DOM_DISTILLER_VIEWER_FAILED_TO_FIND_ARTICLE_CONTENT);
   return ReplaceHtmlTemplateValues(title, content, "hidden", "");
 }
 
-const std::string GetCss() {
-  return ResourceBundle::GetSharedInstance()
-      .GetRawDataResource(IDR_DISTILLER_CSS)
-      .as_string();
+const std::string GetCss(ReaderModePrefs* reader_mode_prefs) {
+  return ReplaceCSSTemplateValues(reader_mode_prefs);

[nyquist, 2014/06/23 16:03:06]

This is only used once, so maybe you could inline this?

[smaslo, 2014/06/23 22:14:34]

Done.

 }
 
 const std::string GetJavaScript() {
   return ResourceBundle::GetSharedInstance()
       .GetRawDataResource(IDR_DOM_DISTILLER_VIEWER_JS)
       .as_string();
 }
 
 scoped_ptr<ViewerHandle> CreateViewRequest(
     DomDistillerServiceInterface* dom_distiller_service,
@@ skipping 27 matching lines @@
                                           requested_url).Pass();
   }
 
   // It is invalid to not specify a query param for |kEntryIdKey| or |kUrlKey|.
   return scoped_ptr<ViewerHandle>();
 }
 
 }  // namespace viewer
 
 }  // namespace dom_distiller