Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(116)

Unified Diff: chrome/browser/autocomplete/autocomplete_popup_view_mac.mm

Issue 2127009: Cleanup a bunch of TODO(shess). (Closed) Base URL: git://codf21.jail/chromium.git
Patch Set: Created 10 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/autocomplete/autocomplete_popup_view_mac.mm
diff --git a/chrome/browser/autocomplete/autocomplete_popup_view_mac.mm b/chrome/browser/autocomplete/autocomplete_popup_view_mac.mm
index f0f7a09b0fc43574e6c40a9327df693de42803fd..fd2b20e3f35562abaafebaf587c5a42f864ac826 100644
--- a/chrome/browser/autocomplete/autocomplete_popup_view_mac.mm
+++ b/chrome/browser/autocomplete/autocomplete_popup_view_mac.mm
@@ -74,10 +74,6 @@ NSColor* HoveredBackgroundColor() {
return [[NSColor controlHighlightColor] colorWithAlphaComponent:kPopupAlpha];
}
-// TODO(shess): These are totally unprincipled. I experimented with
-// +controlTextColor and the like, but found myself wondering whether
-// that was really appropriate. Circle back after consulting with
-// someone more knowledgeable about the ins and outs of this.
static const NSColor* ContentTextColor() {
return [NSColor blackColor];
}
@@ -268,9 +264,6 @@ AutocompletePopupViewMac::AutocompletePopupViewMac(
}
AutocompletePopupViewMac::~AutocompletePopupViewMac() {
- // TODO(shess): Having to be aware of destructor ordering in this
- // way seems brittle. There must be a better way.
-
// Destroy the popup model before this object is destroyed, because
// it can call back to us in the destructor.
model_.reset();
@@ -372,9 +365,8 @@ void AutocompletePopupViewMac::UpdatePopupAppearance() {
CreatePopupIfNeeded();
- // The popup's font is a slightly smaller version of what |field_|
- // uses.
- NSFont* fieldFont = [field_ font];
+ // The popup's font is a slightly smaller version of the field's.
+ NSFont* fieldFont = AutocompleteEditViewMac::GetFieldFont();
const CGFloat resultFontSize = [fieldFont pointSize] + kEditFontAdjust;
gfx::Font resultFont = gfx::Font::CreateFont(
base::SysNSStringToWide([fieldFont fontName]), (int)resultFontSize);
@@ -480,18 +472,11 @@ void AutocompletePopupViewMac::OpenURLForRow(int row, bool force_background) {
// The default NSButtonCell drawing leaves the image flush left and
// the title next to the image. This spaces things out to line up
// with the star button and autocomplete field.
-// TODO(shess): Determine if the star button can change size (other
-// than scaling coordinates), in which case this needs to be more
-// dynamic.
- (void)drawInteriorWithFrame:(NSRect)cellFrame inView:(NSView *)controlView {
[[self backgroundColor] set];
NSRectFill(cellFrame);
// Put the image centered vertically but in a fixed column.
- // TODO(shess) Currently, the images are all 16x16 png files, so
- // left-justified works out fine. If that changes, it may be
- // appropriate to align them on their centers instead of their
- // left-hand sides.
NSImage* image = [self image];
if (image) {
NSRect imageRect = cellFrame;

Powered by Google App Engine
This is Rietveld 408576698