Skip to content

GitLab

T
tangled
Commit 28e39abf authored by dbeam's avatar dbeam Committed by Commit bot
Browse files
Options

Revert of [Template Expressions] changing ${ leader to $i18n{ (patchset #4... 

Revert of [Template Expressions] changing ${ leader to $i18n{ (patchset #4 id:60001 of https://codereview.chromium.org/1585923011/ )

Reason for revert:
broke incognito NTP and chrome://apps
https://code.google.com/p/chromium/issues/detail?id=581621

Original issue's description:
> [Template Expressions] changing ${ leader to $i18n{
>
> This CL changes the Template Expressions from using
> a ${ to identify the start of an expression, to using
> $i18n{.  The intention is to reduce (eliminate) false
> positives when searching for expressions that need
> replacement.  This CL also prepares for the replacement
> work to be done in a background thread and used on a
> broader set of files.
>
> BUG=506009
>
> Committed: https://crrev.com/5aa7fd5f14e49c65ebba87e4248424d622b92930
> Cr-Commit-Position: refs/heads/master@{#371537}

TBR=thakis@chromium.org,dschuyler@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=506009

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

Cr-Commit-Position: refs/heads/master@{#371888}
parent d680edc4
Hide whitespace changes
Inline Side-by-side
Showing with 63 additions and 72 deletions
chrome/browser/resources/ntp4/new_incognito_tab_theme.css
View file @ 28e39abf
......@@ -5,21 +5,21 @@
html {
background-attachment: fixed;
background-color: $i18n{colorBackground};
background-position: $i18n{backgroundBarDetached};
background-repeat: $i18n{backgroundTiling};
background-color: ${colorBackground};
background-position: ${backgroundBarDetached};
background-repeat: ${backgroundTiling};
height: 100%;
overflow: auto;
}
html[hascustombackground='true'] {
background-image: url(chrome://theme/IDR_THEME_NTP_BACKGROUND?$i18n{themeId});
background-image: url(chrome://theme/IDR_THEME_NTP_BACKGROUND?${themeId});
}
html[bookmarkbarattached='true'] {
background-position: $i18n{backgroundBarAttached};
background-position: ${backgroundBarAttached};
}
#attribution-img {
content: url(chrome://theme/IDR_THEME_NTP_ATTRIBUTION?$i18n{themeId});
content: url(chrome://theme/IDR_THEME_NTP_ATTRIBUTION?${themeId});
}
chrome/browser/resources/ntp4/new_tab_theme.css
View file @ 28e39abf
/* Copyright 2012 The Chromium Authors. All rights reserved.
/* Copyright (c) 2012 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. */
html {
background-attachment: fixed;
background-color: $i18n{colorBackground};
background-image: url(chrome://theme/IDR_THEME_NTP_BACKGROUND?$i18n{themeId});
background-position: $i18n{backgroundBarDetached};
background-repeat: $i18n{backgroundTiling};
background-color: ${colorBackground};
background-image: url(chrome://theme/IDR_THEME_NTP_BACKGROUND?${themeId});
background-position: ${backgroundBarDetached};
background-repeat: ${backgroundTiling};
}
#attribution {
left: $i18n{leftAlignAttribution};
right: $i18n{rightAlignAttribution};
text-align: $i18n{textAlignAttribution};
display: $i18n{displayAttribution};
left: ${leftAlignAttribution};
right: ${rightAlignAttribution};
text-align: ${textAlignAttribution};
display: ${displayAttribution};
}
#attribution-img {
content: url(chrome://theme/IDR_THEME_NTP_ATTRIBUTION?$i18n{themeId});
content: url(chrome://theme/IDR_THEME_NTP_ATTRIBUTION?${themeId});
}
html[bookmarkbarattached='true'] {
background-position: $i18n{backgroundBarAttached};
background-position: ${backgroundBarAttached};
}
body {
color: $i18n{colorTextRgba};
color: ${colorTextRgba};
height: 100%;
overflow: auto;
}
#attribution,
[is='action-link'] {
color: $i18n{colorTextLight};
color: ${colorTextLight};
}
[is='action-link']:active {
color: $i18n{colorTextRgba};
color: ${colorTextRgba};
}
.page-switcher {
color: rgba($i18n{colorText}, 0.5);
color: rgba(${colorText}, 0.5);
}
.page-switcher:hover,
.page-switcher:focus,
.page-switcher.drag-target {
background-color: rgba($i18n{colorText}, 0.06);
background-color: rgba(${colorText}, 0.06);
}
/* Only change the background to a gradient when a promo is showing. */
......@@ -55,34 +55,34 @@ body {
.showing-login-area #page-switcher-end:focus,
.showing-login-area #page-switcher-end.drag-target {
background: linear-gradient(top,
rgba($i18n{colorText}, 0) 0,
rgba($i18n{colorText}, .01) 60px,
rgba($i18n{colorText}, .06) 183px);
rgba(${colorText}, 0) 0,
rgba(${colorText}, .01) 60px,
rgba(${colorText}, .06) 183px);
}
.tile-page-scrollbar {
background-color: $i18n{colorTextLight};
background-color: ${colorTextLight};
}
/* Footer *********************************************************************/
#footer-border {
background: linear-gradient(left,
rgba($i18n{colorSectionBorder}, 0.2),
rgba($i18n{colorSectionBorder}, 0.3) 20%,
rgba($i18n{colorSectionBorder}, 0.3) 80%,
rgba($i18n{colorSectionBorder}, 0.2));
rgba(${colorSectionBorder}, 0.2),
rgba(${colorSectionBorder}, 0.3) 20%,
rgba(${colorSectionBorder}, 0.3) 80%,
rgba(${colorSectionBorder}, 0.2));
}
.dot input:focus {
background-color: $i18n{colorBackground};
background-color: ${colorBackground};
}
.thumbnail-wrapper {
/* This shows through at the (rounded) thumbnail's corners. */
background-color: $i18n{colorSectionBorder};
background-color: ${colorSectionBorder};
}
.filler .thumbnail {
border-color: $i18n{colorBackground};
border-color: ${colorBackground};
}
chrome/browser/ui/webui/ntp/ntp_resource_cache.cc
View file @ 28e39abf
// Copyright 2012 The Chromium Authors. All rights reserved.
// Copyright (c) 2012 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.
......@@ -553,7 +553,7 @@ void NTPResourceCache::CreateNewTabIncognitoCSS() {
: SkColorSetRGB(0x32, 0x32, 0x32);
// Generate the replacements.
ui::TemplateReplacements substitutions;
std::map<base::StringPiece, std::string> substitutions;
// Cache-buster for background.
substitutions["themeId"] =
......@@ -605,7 +605,7 @@ void NTPResourceCache::CreateNewTabCSS() {
SkColorGetB(color_header));
// Generate the replacements.
ui::TemplateReplacements substitutions;
std::map<base::StringPiece, std::string> substitutions;
// Cache-buster for background.
substitutions["themeId"] =
......
chrome/browser/web_dev_style/css_checker.py
View file @ 28e39abf
# Copyright 2012 The Chromium Authors. All rights reserved.
# Copyright (c) 2012 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.
......@@ -55,7 +55,7 @@ class CSSChecker(object):
return re.sub(re.compile(r'--[\d\w-]+: {.*?};', re.DOTALL), '', s)
def _remove_template_expressions(s):
return re.sub(re.compile(r'\$i18n{[^}]*}', re.DOTALL), '', s)
return re.sub(re.compile(r'\${[^}]*}', re.DOTALL), '', s)
def _remove_grit(s):
grit_reg = re.compile(r"""
......
ui/base/template_expressions.cc
View file @ 28e39abf
......@@ -8,36 +8,29 @@
#include "base/logging.h"
namespace {
const char kLeader[] = "$i18n{";
const size_t kLeaderSize = arraysize(kLeader) - 1;
const char kTail[] = "}";
const size_t kTailSize = arraysize(kTail) - 1;
} // namespace
namespace ui {
std::string ReplaceTemplateExpressions(
base::StringPiece format_string,
const TemplateReplacements& substitutions) {
const std::map<base::StringPiece, std::string>& substitutions) {
std::string formatted;
const size_t kValueLengthGuess = 16;
formatted.reserve(format_string.length() +
substitutions.size() * kValueLengthGuess);
base::StringPiece::const_iterator i = format_string.begin();
while (i < format_string.end()) {
if (base::StringPiece(i).starts_with(kLeader)) {
size_t key_start = i + kLeaderSize - format_string.begin();
size_t key_length = format_string.find(kTail, key_start);
if (*i == '$' && i + 2 < format_string.end() && i[1] == '{' &&
i[2] != '}') {
size_t key_start = i + strlen("${") - format_string.begin();
size_t key_length = format_string.find('}', key_start);
if (key_length == base::StringPiece::npos)
NOTREACHED() << "TemplateExpression missing ending tag";
NOTREACHED() << "TemplateExpression missing ending brace '}'";
key_length -= key_start;
std::string key(format_string.begin() + key_start, key_length);
DCHECK(!key.empty());
base::StringPiece key(format_string.begin() + key_start, key_length);
const auto& replacement = substitutions.find(key);
if (replacement != substitutions.end()) {
formatted.append(replacement->second);
i += kLeaderSize + key_length + kTailSize;
i += strlen("${") + key_length + strlen("}");
continue;
} else {
NOTREACHED() << "TemplateExpression key not found: " << key;
......
ui/base/template_expressions.h
View file @ 28e39abf
......@@ -16,15 +16,12 @@
namespace ui {
// Map of strings for template replacement in |ReplaceTemplateExpressions|.
typedef std::map<const std::string, std::string> TemplateReplacements;
// Replace ${foo} in the format string with the value for the foo key in
// |subst|. If the key is not found in the |substitutions| that item will
// be unaltered.
UI_BASE_EXPORT std::string ReplaceTemplateExpressions(
base::StringPiece format_string,
const TemplateReplacements& substitutions);
const std::map<base::StringPiece, std::string>& substitutions);
} // namespace ui
......
ui/base/template_expressions_unittest.cc
View file @ 28e39abf
......@@ -9,26 +9,27 @@
namespace ui {
TEST(TemplateExpressionsTest, ReplaceTemplateExpressionsPieces) {
TemplateReplacements substitutions;
std::map<base::StringPiece, std::string> substitutions;
substitutions["test"] = "word";
substitutions["5"] = "number";
EXPECT_EQ("${}", ReplaceTemplateExpressions("${}", substitutions));
EXPECT_EQ("", ReplaceTemplateExpressions("", substitutions));
EXPECT_EQ("word", ReplaceTemplateExpressions("$i18n{test}", substitutions));
EXPECT_EQ("number ", ReplaceTemplateExpressions("$i18n{5} ", substitutions));
EXPECT_EQ("multiple: word, number.",
ReplaceTemplateExpressions("multiple: $i18n{test}, $i18n{5}.",
substitutions));
EXPECT_EQ("word", ReplaceTemplateExpressions("${test}", substitutions));
EXPECT_EQ("number ", ReplaceTemplateExpressions("${5} ", substitutions));
EXPECT_EQ(
"multiple: word, number.",
ReplaceTemplateExpressions("multiple: ${test}, ${5}.", substitutions));
}
TEST(TemplateExpressionsTest,
ReplaceTemplateExpressionsConsecutiveDollarSignsPieces) {
TemplateReplacements substitutions;
std::map<base::StringPiece, std::string> substitutions;
substitutions["a"] = "x";
EXPECT_EQ("$ $$ $$$", ReplaceTemplateExpressions("$ $$ $$$", substitutions));
EXPECT_EQ("$x", ReplaceTemplateExpressions("$$i18n{a}", substitutions));
EXPECT_EQ("$$x", ReplaceTemplateExpressions("$$$i18n{a}", substitutions));
EXPECT_EQ("$i18n12", ReplaceTemplateExpressions("$i18n12", substitutions));
EXPECT_EQ("$x", ReplaceTemplateExpressions("$${a}", substitutions));
EXPECT_EQ("$$x", ReplaceTemplateExpressions("$$${a}", substitutions));
EXPECT_EQ("$12", ReplaceTemplateExpressions("$12", substitutions));
}
} // namespace ui
ui/base/webui/web_ui_util.cc
View file @ 28e39abf
......@@ -123,7 +123,7 @@ void SetLoadTimeDataDefaults(const std::string& app_locale,
}
std::string GetWebUiCssTextDefaults(const std::string& css_template) {
ui::TemplateReplacements placeholders;
std::map<base::StringPiece, std::string> placeholders;
placeholders["textDirection"] = GetTextDirection();
placeholders["fontFamily"] = GetFontFamily();
placeholders["fontSize"] = GetFontSize();
......
ui/webui/resources/css/text_defaults.css
View file @ 28e39abf
......@@ -16,14 +16,14 @@
* Otherwise its $placeholders won't be expanded. */
html {
direction: $i18n{textDirection};
direction: ${textDirection};
}
body {
font-family: $i18n{fontFamily};
font-size: $i18n{fontSize};
font-family: ${fontFamily};
font-size: ${fontSize};
}
button {
font-family: $i18n{fontFamily};
font-family: ${fontFamily};
}
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