Commit 3695ffef authored by kuan's avatar kuan Committed by Commit bot

display error message for empty distilled content

also:
- ensure that error message is displayed for empty title
- move dom_distiller_viewer.html/js from content to core dir
- added tests.

BUG=403066

Review URL: https://codereview.chromium.org/563923002

Cr-Commit-Position: refs/heads/master@{#296719}
parent a82c0a5f
......@@ -860,8 +860,6 @@
'defines!': ['CONTENT_IMPLEMENTATION'],
'dependencies': [
'components.gyp:autofill_content_browser',
'components.gyp:dom_distiller_content',
'components.gyp:dom_distiller_core',
'components.gyp:password_manager_content_renderer',
'components.gyp:pref_registry_test_support',
'components_resources.gyp:components_resources',
......@@ -876,6 +874,11 @@
'../skia/skia.gyp:skia',
'../testing/gmock.gyp:gmock',
'../testing/gtest.gyp:gtest',
# Dependencies of dom_distiller
'components.gyp:dom_distiller_content',
'components.gyp:dom_distiller_core',
'components_strings.gyp:components_strings',
],
'include_dirs': [
'..',
......
......@@ -9,15 +9,20 @@
#include "components/dom_distiller/content/distiller_page_web_contents.h"
#include "components/dom_distiller/content/web_contents_main_frame_observer.h"
#include "components/dom_distiller/core/distiller_page.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/viewer.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/navigation_controller.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/web_contents_observer.h"
#include "content/public/test/content_browser_test.h"
#include "content/shell/browser/shell.h"
#include "grit/components_strings.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "third_party/dom_distiller_js/dom_distiller.pb.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/resource/resource_bundle.h"
using content::ContentBrowserTest;
......@@ -407,4 +412,110 @@ IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, MarkupInfo) {
EXPECT_EQ(600, markup_image2.height());
}
IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest,
TestTitleAndContentAreNeverEmpty) {
const std::string some_title = "some title";
const std::string some_content = "some content";
const std::string no_title =
l10n_util::GetStringUTF8(IDS_DOM_DISTILLER_VIEWER_NO_DATA_TITLE);
const std::string no_content =
l10n_util::GetStringUTF8(IDS_DOM_DISTILLER_VIEWER_NO_DATA_CONTENT);
{ // Test non-empty title and content for article.
scoped_ptr<DistilledArticleProto> article_proto(
new DistilledArticleProto());
article_proto->set_title(some_title);
(*(article_proto->add_pages())).set_html(some_content);
std::string html = viewer::GetUnsafeArticleHtml(article_proto.get(),
DistilledPagePrefs::LIGHT, DistilledPagePrefs::SERIF);
EXPECT_THAT(html, HasSubstr(some_title));
EXPECT_THAT(html, HasSubstr(some_content));
EXPECT_THAT(html, Not(HasSubstr(no_title)));
EXPECT_THAT(html, Not(HasSubstr(no_content)));
}
{ // Test empty title and content for article.
scoped_ptr<DistilledArticleProto> article_proto(
new DistilledArticleProto());
article_proto->set_title("");
(*(article_proto->add_pages())).set_html("");
std::string html = viewer::GetUnsafeArticleHtml(article_proto.get(),
DistilledPagePrefs::LIGHT, DistilledPagePrefs::SERIF);
EXPECT_THAT(html, HasSubstr(no_title));
EXPECT_THAT(html, HasSubstr(no_content));
EXPECT_THAT(html, Not(HasSubstr(some_title)));
EXPECT_THAT(html, Not(HasSubstr(some_content)));
}
{ // Test missing title and non-empty content for article.
scoped_ptr<DistilledArticleProto> article_proto(
new DistilledArticleProto());
(*(article_proto->add_pages())).set_html(some_content);
std::string html = viewer::GetUnsafeArticleHtml(article_proto.get(),
DistilledPagePrefs::LIGHT, DistilledPagePrefs::SERIF);
EXPECT_THAT(html, HasSubstr(no_title));
EXPECT_THAT(html, HasSubstr(no_content));
EXPECT_THAT(html, Not(HasSubstr(some_title)));
EXPECT_THAT(html, Not(HasSubstr(some_content)));
}
{ // Test non-empty title and missing content for article.
scoped_ptr<DistilledArticleProto> article_proto(
new DistilledArticleProto());
article_proto->set_title(some_title);
std::string html = viewer::GetUnsafeArticleHtml(article_proto.get(),
DistilledPagePrefs::LIGHT, DistilledPagePrefs::SERIF);
EXPECT_THAT(html, HasSubstr(no_title));
EXPECT_THAT(html, HasSubstr(no_content));
EXPECT_THAT(html, Not(HasSubstr(some_title)));
EXPECT_THAT(html, Not(HasSubstr(some_content)));
}
{ // Test non-empty title and content for page.
scoped_ptr<DistilledPageProto> page_proto(new DistilledPageProto());
page_proto->set_title(some_title);
page_proto->set_html(some_content);
std::string html = viewer::GetUnsafePartialArticleHtml(page_proto.get(),
DistilledPagePrefs::LIGHT, DistilledPagePrefs::SERIF);
EXPECT_THAT(html, HasSubstr(some_title));
EXPECT_THAT(html, HasSubstr(some_content));
EXPECT_THAT(html, Not(HasSubstr(no_title)));
EXPECT_THAT(html, Not(HasSubstr(no_content)));
}
{ // Test empty title and content for page.
scoped_ptr<DistilledPageProto> page_proto(new DistilledPageProto());
page_proto->set_title("");
page_proto->set_html("");
std::string html = viewer::GetUnsafePartialArticleHtml(page_proto.get(),
DistilledPagePrefs::LIGHT, DistilledPagePrefs::SERIF);
EXPECT_THAT(html, HasSubstr(no_title));
EXPECT_THAT(html, HasSubstr(no_content));
EXPECT_THAT(html, Not(HasSubstr(some_title)));
EXPECT_THAT(html, Not(HasSubstr(some_content)));
}
{ // Test missing title and non-empty content for page.
scoped_ptr<DistilledPageProto> page_proto(new DistilledPageProto());
page_proto->set_html(some_content);
std::string html = viewer::GetUnsafePartialArticleHtml(page_proto.get(),
DistilledPagePrefs::LIGHT, DistilledPagePrefs::SERIF);
EXPECT_THAT(html, HasSubstr(no_title));
EXPECT_THAT(html, HasSubstr(some_content));
EXPECT_THAT(html, Not(HasSubstr(some_title)));
EXPECT_THAT(html, Not(HasSubstr(no_content)));
}
{ // Test non-empty title and missing content for page.
scoped_ptr<DistilledPageProto> page_proto(new DistilledPageProto());
page_proto->set_title(some_title);
std::string html = viewer::GetUnsafePartialArticleHtml(page_proto.get(),
DistilledPagePrefs::LIGHT, DistilledPagePrefs::SERIF);
EXPECT_THAT(html, HasSubstr(some_title));
EXPECT_THAT(html, HasSubstr(no_content));
EXPECT_THAT(html, Not(HasSubstr(no_title)));
EXPECT_THAT(html, Not(HasSubstr(some_content)));
}
}
} // namespace dom_distiller
......@@ -89,6 +89,15 @@ const std::string GetFontCssClass(DistilledPagePrefs::FontFamily font_family) {
return kSansSerifCssClass;
}
void EnsureNonEmptyTitleAndContent(std::string* title, std::string* content) {
if (title->empty())
*title = l10n_util::GetStringUTF8(IDS_DOM_DISTILLER_VIEWER_NO_DATA_TITLE);
if (content->empty()) {
*content = l10n_util::GetStringUTF8(
IDS_DOM_DISTILLER_VIEWER_NO_DATA_CONTENT);
}
}
std::string ReplaceHtmlTemplateValues(
const std::string& title,
const std::string& content,
......@@ -146,6 +155,7 @@ const std::string GetUnsafePartialArticleHtml(
std::ostringstream unsafe_output_stream;
unsafe_output_stream << page_proto->html();
std::string unsafe_article_html = unsafe_output_stream.str();
EnsureNonEmptyTitleAndContent(&title, &unsafe_article_html);
std::string original_url = page_proto->url();
return ReplaceHtmlTemplateValues(
title, unsafe_article_html, "visible", original_url, theme, font_family);
......@@ -166,12 +176,10 @@ const std::string GetUnsafeArticleHtml(
unsafe_output_stream << article_proto->pages(page_num).html();
}
unsafe_article_html = unsafe_output_stream.str();
} else {
title = l10n_util::GetStringUTF8(IDS_DOM_DISTILLER_VIEWER_NO_DATA_TITLE);
unsafe_article_html =
l10n_util::GetStringUTF8(IDS_DOM_DISTILLER_VIEWER_NO_DATA_CONTENT);
}
EnsureNonEmptyTitleAndContent(&title, &unsafe_article_html);
std::string original_url;
if (article_proto->pages_size() > 0 && article_proto->pages(0).has_url()) {
original_url = article_proto->pages(0).url();
......
......@@ -3,8 +3,8 @@
<include name="IDR_ABOUT_DOM_DISTILLER_HTML" file="../dom_distiller/webui/resources/about_dom_distiller.html" flattenhtml="true" allowexternalscript="true" type="BINDATA" />
<include name="IDR_ABOUT_DOM_DISTILLER_CSS" file="../dom_distiller/webui/resources/about_dom_distiller.css" type="BINDATA" />
<include name="IDR_ABOUT_DOM_DISTILLER_JS" file="../dom_distiller/webui/resources/about_dom_distiller.js" type="BINDATA" />
<include name="IDR_DOM_DISTILLER_VIEWER_HTML" file="../dom_distiller/content/resources/dom_distiller_viewer.html" type="BINDATA" />
<include name="IDR_DOM_DISTILLER_VIEWER_JS" file="../dom_distiller/content/resources/dom_distiller_viewer.js" type="BINDATA" />
<include name="IDR_DOM_DISTILLER_VIEWER_HTML" file="../dom_distiller/core/html/dom_distiller_viewer.html" type="BINDATA" />
<include name="IDR_DOM_DISTILLER_VIEWER_JS" file="../dom_distiller/core/javascript/dom_distiller_viewer.js" type="BINDATA" />
<include name="IDR_DISTILLER_JS" file="../dom_distiller/core/javascript/domdistiller.js" flattenhtml="true" type="BINDATA" />
<include name="IDR_DISTILLER_CSS" file="../dom_distiller/core/css/distilledpage.css" type="BINDATA" />
<include name="IDR_IS_DISTILLABLE_JS" file="../dom_distiller/core/javascript/is_distillable_trigger.js" type="BINDATA" />
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment