Commit 6fee242d authored by Abhijeet Singh's avatar Abhijeet Singh Committed by Commit Bot

Make phonetics optional for Quick Answers

Make displaying phonetics for the dictionary use-case within Quick
Answers optional to handle the scenario when one is unavailable or
multiple are present, parsing failure ends up leading to not displaying
the definition as well.

Bug: b:152110565
Test: Tested on Chrome OS VM.
Change-Id: Ice5a4ed4ee2fd86aaa2c1baeae36c26c2be3262c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2151931Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Abhijeet Singh <siabhijeet@google.com>
Cr-Commit-Position: refs/heads/master@{#759824}
parent aa61e4be
...@@ -22,7 +22,8 @@ constexpr char kSensesKey[] = "senses"; ...@@ -22,7 +22,8 @@ constexpr char kSensesKey[] = "senses";
constexpr char kDefinitionPathUnderSense[] = "definition.text"; constexpr char kDefinitionPathUnderSense[] = "definition.text";
constexpr char kPhoneticsKey[] = "phonetics"; constexpr char kPhoneticsKey[] = "phonetics";
constexpr char kPhoneticsTextKey[] = "text"; constexpr char kPhoneticsTextKey[] = "text";
constexpr char kPhoneticsResultTemplate[] = "%s · /%s/"; constexpr char kDefinitionResultDefaultTemplate[] = "%s · /%s/";
constexpr char kDefinitionResultNoPhoneticsTemplate[] = "%s";
} // namespace } // namespace
...@@ -35,18 +36,13 @@ bool DefinitionResultParser::Parse(const Value* result, ...@@ -35,18 +36,13 @@ bool DefinitionResultParser::Parse(const Value* result,
return false; return false;
} }
// Get Definition. // Get definition and phonetics.
const std::string* definition = ExtractDefinition(first_entry); const std::string* definition = ExtractDefinition(first_entry);
if (!definition) { if (!definition) {
LOG(ERROR) << "Fail in extracting definition"; LOG(ERROR) << "Fail in extracting definition";
return false; return false;
} }
const std::string* phonetics = ExtractPhonetics(first_entry); const std::string* phonetics = ExtractPhonetics(first_entry);
if (!phonetics) {
LOG(ERROR) << "Fail in extracting phonetics";
return false;
}
const std::string* query_term = result->FindStringPath(kQueryTermPath); const std::string* query_term = result->FindStringPath(kQueryTermPath);
if (!query_term) { if (!query_term) {
...@@ -54,8 +50,11 @@ bool DefinitionResultParser::Parse(const Value* result, ...@@ -54,8 +50,11 @@ bool DefinitionResultParser::Parse(const Value* result,
return false; return false;
} }
const std::string& secondary_answer = base::StringPrintf( const std::string& secondary_answer =
kPhoneticsResultTemplate, query_term->c_str(), phonetics->c_str()); phonetics ? base::StringPrintf(kDefinitionResultDefaultTemplate,
query_term->c_str(), phonetics->c_str())
: base::StringPrintf(kDefinitionResultNoPhoneticsTemplate,
query_term->c_str());
quick_answer->result_type = ResultType::kDefinitionResult; quick_answer->result_type = ResultType::kDefinitionResult;
quick_answer->primary_answer = *definition; quick_answer->primary_answer = *definition;
quick_answer->secondary_answer = secondary_answer; quick_answer->secondary_answer = secondary_answer;
...@@ -90,7 +89,7 @@ const std::string* DefinitionResultParser::ExtractPhonetics( ...@@ -90,7 +89,7 @@ const std::string* DefinitionResultParser::ExtractPhonetics(
const Value* first_phonetics = const Value* first_phonetics =
GetFirstListElement(*definition_entry, kPhoneticsKey); GetFirstListElement(*definition_entry, kPhoneticsKey);
if (!first_phonetics) { if (!first_phonetics) {
LOG(ERROR) << "Can't find a phonetics."; LOG(WARNING) << "Can't find a phonetics.";
return nullptr; return nullptr;
} }
......
...@@ -116,7 +116,14 @@ TEST_F(DefinitionResultParserTest, NoPhonetic) { ...@@ -116,7 +116,14 @@ TEST_F(DefinitionResultParserTest, NoPhonetic) {
Value result = BuildDictionaryResult( Value result = BuildDictionaryResult(
"unfathomable", "", "incapable of being fully explored or understood."); "unfathomable", "", "incapable of being fully explored or understood.");
QuickAnswer quick_answer; QuickAnswer quick_answer;
EXPECT_FALSE(parser_->Parse(&result, &quick_answer)); EXPECT_TRUE(parser_->Parse(&result, &quick_answer));
const auto& expected_title = "unfathomable";
const auto& expected_answer =
"incapable of being fully explored or understood.";
EXPECT_EQ(ResultType::kDefinitionResult, quick_answer.result_type);
EXPECT_EQ(expected_answer, quick_answer.primary_answer);
EXPECT_EQ(expected_title, quick_answer.secondary_answer);
} }
TEST_F(DefinitionResultParserTest, NoDefinition) { TEST_F(DefinitionResultParserTest, NoDefinition) {
......
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