Skip to content

GitLab

T
tangled
Commit e486ef03 authored by Peter Laurens's avatar Peter Laurens Committed by Commit Bot
Browse files
Options

Revert "Unescape fragment for display in Omnibox" 

This reverts commit f8625da2.

Reason for revert: This change breaks EGTests for iOS Simulator and Device, see:

https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.mac%2Fios-simulator-full-configs%2F1434%2F%2B%2Frecipes%2Fsteps%2Fios_chrome_web_egtests__iPhone_6s_Plus_iOS_10.0_%2F0%2Fstdout

Original change's description:
> Unescape fragment for display in Omnibox
> 
> A change in Chrome 63 causes Chrome to urlencode the fragment component
> of URLs. The Chrome omnibox should decode the fragment component as it
> does for other URL components and matching Firefox.
> 
> Bug: 789163
> Change-Id: If82d72445d904495c8d3e4129ab63be141745328
> Reviewed-on: https://chromium-review.googlesource.com/822819
> Commit-Queue: Eric Lawrence <elawrence@chromium.org>
> Reviewed-by: Peter Kasting <pkasting@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#524123}

TBR=pkasting@chromium.org,elawrence@chromium.org

Change-Id: I9572e8a5d66381478c1faca0f78a66b878a4b059
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 789163
Reviewed-on: https://chromium-review.googlesource.com/828080Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Commit-Queue: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524221}
parent 95ef9cc2
Hide whitespace changes
Inline Side-by-side
Showing with 13 additions and 26 deletions
chrome/browser/ui/views/omnibox/omnibox_view_views_browsertest.cc
View file @ e486ef03
......@@ -462,16 +462,6 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewViewsTest, TextElideStatus) {
gfx::NO_ELIDE);
}
IN_PROC_BROWSER_TEST_F(OmniboxViewViewsTest, FragmentUnescapedForDisplay) {
OmniboxView* view = nullptr;
ASSERT_NO_FATAL_FAILURE(GetOmniboxViewForBrowser(browser(), &view));
ui_test_utils::NavigateToURL(browser(),
GURL("https://www.google.com/#%E2%98%83"));
EXPECT_EQ(view->GetText(),
base::UTF8ToUTF16("https://www.google.com/#\u2603"));
}
IN_PROC_BROWSER_TEST_F(OmniboxViewViewsTest, FriendlyAccessibleLabel) {
OmniboxView* omnibox_view = nullptr;
ASSERT_NO_FATAL_FAILURE(GetOmniboxViewForBrowser(browser(), &omnibox_view));
......
components/url_formatter/elide_url_unittest.cc
View file @ e486ef03
......@@ -347,16 +347,15 @@ TEST(TextEliderTest, TestElisionSpecialCases) {
kEllipsisStr + "berkeley.edu:4430/" + kEllipsisStr + "/arbitfilename"},
// Unescaping.
{"http://www/%E4%BD%A0%E5%A5%BD?"
"q=%E4%BD%A0%E5%A5%BD#\xe4\xbd\xa0\xe4\xbd\xa0\xe4\xbd\xa0",
"www/\xe4\xbd\xa0\xe5\xa5\xbd?q=\xe4\xbd\xa0\xe5\xa5\xbd#\xe4\xbd\xa0" +
{"http://www/%E4%BD%A0%E5%A5%BD?q=%E4%BD%A0%E5%A5%BD#\xe4\xbd\xa0",
"www/\xe4\xbd\xa0\xe5\xa5\xbd?q=\xe4\xbd\xa0\xe5\xa5\xbd#" +
kEllipsisStr},
// Invalid unescaping for path. The ref will always be valid UTF-8. We
// don't bother to do too many edge cases, since these are handled by the
// escaper unittest.
{"http://www/%E4%A0%E5%A5%BD?q=%E4%BD%A0%E5%A5%BD#\xe4\xbd\xa0",
"www/%E4%A0%E5%A5%BD?q=\xe4\xbd\xa0\xe5\xa5\xbd#\xe4\xbd\xa0"},
"www/%E4%A0%E5%A5%BD?q=\xe4\xbd\xa0\xe5\xa5\xbd#" + kEllipsisStr},
};
RunElisionTest(testcases);
......
components/url_formatter/url_formatter.cc
View file @ e486ef03
......@@ -586,10 +586,11 @@ base::string16 FormatUrlWithAdjustments(
NonHostComponentTransform(unescape_rules),
&url_string, &new_parsed->query, adjustments);
// Ref. This is valid, unescaped UTF-8, so we can just convert.
if (parsed.ref.is_valid())
url_string.push_back('#');
AppendFormattedComponent(spec, parsed.ref,
NonHostComponentTransform(unescape_rules),
NonHostComponentTransform(net::UnescapeRule::NONE),
&url_string, &new_parsed->ref, adjustments);
}
......
components/url_formatter/url_formatter_unittest.cc
View file @ e486ef03
......@@ -828,7 +828,7 @@ TEST(UrlFormatterTest, FormatUrl) {
{"With a port number and a reference",
"http://www.google.com:8080/#\xE3\x82\xB0", default_format_type,
net::UnescapeRule::NORMAL, L"http://www.google.com:8080/#\x30B0", 7},
net::UnescapeRule::NORMAL, L"http://www.google.com:8080/#%E3%82%B0", 7},
// -------- IDN tests --------
{"Japanese IDN with ja", "http://xn--l8jvb1ey91xtjb.jp",
......@@ -1119,7 +1119,7 @@ TEST(UrlFormatterTest, FormatUrlParsed) {
kFormatUrlOmitNothing, net::UnescapeRule::NORMAL, &parsed,
nullptr, nullptr);
EXPECT_EQ(WideToUTF16(L"http://\x30B0:\x30FC@\x30B0\x30FC\x30B0\x30EB.jp:8080"
L"/\x30B0/?q=\x30B0#\x30B0"),
L"/\x30B0/?q=\x30B0#%E3%82%B0"),
formatted);
EXPECT_EQ(WideToUTF16(L"\x30B0"),
formatted.substr(parsed.username.begin, parsed.username.len));
......@@ -1133,7 +1133,7 @@ TEST(UrlFormatterTest, FormatUrlParsed) {
formatted.substr(parsed.path.begin, parsed.path.len));
EXPECT_EQ(WideToUTF16(L"q=\x30B0"),
formatted.substr(parsed.query.begin, parsed.query.len));
EXPECT_EQ(WideToUTF16(L"\x30B0"),
EXPECT_EQ(WideToUTF16(L"%E3%82%B0"),
formatted.substr(parsed.ref.begin, parsed.ref.len));
// Omit_username_password + unescape case.
......@@ -1143,7 +1143,7 @@ TEST(UrlFormatterTest, FormatUrlParsed) {
kFormatUrlOmitUsernamePassword, net::UnescapeRule::NORMAL,
&parsed, nullptr, nullptr);
EXPECT_EQ(WideToUTF16(L"http://\x30B0\x30FC\x30B0\x30EB.jp:8080"
L"/\x30B0/?q=\x30B0#\x30B0"),
L"/\x30B0/?q=\x30B0#%E3%82%B0"),
formatted);
EXPECT_FALSE(parsed.username.is_valid());
EXPECT_FALSE(parsed.password.is_valid());
......@@ -1155,7 +1155,7 @@ TEST(UrlFormatterTest, FormatUrlParsed) {
formatted.substr(parsed.path.begin, parsed.path.len));
EXPECT_EQ(WideToUTF16(L"q=\x30B0"),
formatted.substr(parsed.query.begin, parsed.query.len));
EXPECT_EQ(WideToUTF16(L"\x30B0"),
EXPECT_EQ(WideToUTF16(L"%E3%82%B0"),
formatted.substr(parsed.ref.begin, parsed.ref.len));
// View-source case.
......@@ -1389,12 +1389,9 @@ TEST(UrlFormatterTest, FormatUrlWithOffsets) {
kFormatUrlOmitNothing, net::UnescapeRule::SPACES, unescape_offsets);
const size_t ref_offsets[] = {
0, 1, 2, 3, 4, 5, 6, 7, 8, 9,
10, 11, 12, 13, 14, 15, 16, 17, 18, 19,
20, 21, 22, 23, 24, 25, 26, 27, 28, 29,
30, 31, kNpos, kNpos, kNpos, kNpos, kNpos, kNpos, kNpos, kNpos,
32, kNpos, kNpos, kNpos, kNpos, kNpos, kNpos, kNpos, kNpos, 33};
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33,
34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49};
// Unescape to "http://www.google.com/foo.html#\x30B0\x30B0z".
CheckAdjustedOffsets("http://www.google.com/foo.html#%E3%82%B0%E3%82%B0z",
kFormatUrlOmitNothing, net::UnescapeRule::NORMAL,
......
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