Skip to content

Commit

Permalink
fix display of filenames in file:/// URLs
Browse files Browse the repository at this point in the history
The code attempted to shorten file:/// URLs to just the filename
when displayed as the title of a page.  But that appears to have
regressed sometime in the past.  This shortening is consistent with
how we display the title of images (which are like "foo.png (123x456)".)
Chrome does a poor job of displaying longer titles (most of the
tab title ends up being "file:///C:/" anyway).

In any case, using a FilePath to get the filename from a URL may not
have even worked on Windows, where the path separator is a backslash.

It appears Glen wrote the original code, Brett may be the one to have
regressed it in a refactor, and I probably broke it worse in a FilePath
refactor.

BUG=69467
TEST=load a text file via a file: URL; tab title is just the file name

Review URL: http://codereview.chromium.org/6591127

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@76792 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
evan@chromium.org committed Mar 3, 2011
1 parent 9861ae9 commit 3ce9568
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 11 deletions.
11 changes: 0 additions & 11 deletions content/browser/tab_contents/navigation_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -243,17 +243,6 @@ NavigationEntry* NavigationController::CreateNavigationEntry(
entry->set_virtual_url(url);
entry->set_user_typed_url(url);
entry->set_update_virtual_url_with_url(reverse_on_redirect);
if (url.SchemeIsFile()) {
// Use the filename as the title, not the full path.
// We need to call FormatUrl() to perform URL de-escaping;
// it's a bit ugly to grab the filename out of the resulting string.
std::string languages =
profile->GetPrefs()->GetString(prefs::kAcceptLanguages);
std::wstring formatted = UTF16ToWideHack(net::FormatUrl(url, languages));
std::wstring filename =
FilePath::FromWStringHack(formatted).BaseName().ToWStringHack();
entry->set_title(WideToUTF16Hack(filename));
}
return entry;
}

Expand Down
8 changes: 8 additions & 0 deletions content/browser/tab_contents/navigation_entry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,14 @@ const string16& NavigationEntry::GetTitleForDisplay(
} else if (!url_.is_empty()) {
title = net::FormatUrl(url_, languages);
}

// For file:// URLs use the filename as the title, not the full path.
if (url_.SchemeIsFile()) {
string16::size_type slashpos = title.rfind('/');
if (slashpos != string16::npos)
title = title.substr(slashpos + 1);
}

ui::ElideString(title, chrome::kMaxTitleChars, &cached_display_title_);
return cached_display_title_;
}
Expand Down
5 changes: 5 additions & 0 deletions content/browser/tab_contents/navigation_entry_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ TEST_F(NavigationEntryTest, NavigationEntryURLs) {
EXPECT_EQ(ASCIIToUTF16("www.google.com"),
entry1_.get()->GetTitleForDisplay(""));

// file:/// URLs should only show the filename.
entry1_.get()->set_url(GURL("file:///foo/bar baz.txt"));
EXPECT_EQ(ASCIIToUTF16("bar baz.txt"),
entry1_.get()->GetTitleForDisplay(""));

// Title affects GetTitleForDisplay
entry1_.get()->set_title(ASCIIToUTF16("Google"));
EXPECT_EQ(ASCIIToUTF16("Google"), entry1_.get()->GetTitleForDisplay(""));
Expand Down

0 comments on commit 3ce9568

Please sign in to comment.