[Mac] Create HyperlinkButtonCell and make the "Get themes" button an instance

[Mac] Create HyperlinkButtonCell and make the "Get themes" button an instance

* Creates a HyperlinkButtonCell that can be used in conjuction with a NSButton
  to display the control as a hyperlink (blue underliend text), rather than
  a normal Cocoa control.
* Switch the "Get themes..." button in Preferences to be one.

BUG=17989, 21414
TEST=Preferences-->Personal Stuff, "Get themes..." is a hyperlink.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=27479

[Nico, 2009-09-28 17:39:46]

Looks pretty good, some minor comments & a few questions.

http://codereview.chromium.org/248019/diff/1/3
File chrome/browser/cocoa/hyperlink_button_cell.h (right):

http://codereview.chromium.org/248019/diff/1/3#newcode15
Line 15: @property (retain) NSURL* url;
Is this used at all?

http://codereview.chromium.org/248019/diff/1/3#newcode16
Line 16: @property (retain) NSColor* color;
"textColor" or "linkColor". "color" is too generic.

http://codereview.chromium.org/248019/diff/1/4
File chrome/browser/cocoa/hyperlink_button_cell.mm (right):

http://codereview.chromium.org/248019/diff/1/4#newcode29
Line 29: - (NSDictionary*)linkAttributes {
Maybe declare the private functions in an @implementation HyperlinkButtonCell
(Private) block at the top of this file?

http://codereview.chromium.org/248019/diff/1/4#newcode33
Line 33: [paragraphStyle setAlignment:NSCenterTextAlignment];
If you use `- (NSTextAlignment)alignment` here, would it then be possible to
configure the text alignment in IB?

http://codereview.chromium.org/248019/diff/1/4#newcode36
Line 36: self.color,                             NSForegroundColorAttributeName,
Do not align all the NS* stuff

http://codereview.chromium.org/248019/diff/1/4#newcode41
Line 41: self.url,                               NSLinkAttributeName,
Should be enough to put @"" here instead of self.url.

http://codereview.chromium.org/248019/diff/1/4#newcode50
Line 50: self.color = [NSColor blueColor];
I haven't seen us use the . syntax anywhere. Go with [self setColor:...] for
consistency?

http://codereview.chromium.org/248019/diff/1/4#newcode66
Line 66: - (id)init {
The init functions in the other files seem to be the first thing in the
@implementation block, maybe move your inits up too?

[TVL, 2009-09-28 18:17:10]

why are we making the button in prefs a hyperlink?  did windows make this
switch?  it seems like a button makes more sense then a link for this sorta
thing.

[Nico, 2009-09-28 18:20:48]

Yes, see one of the two bugs in the BUGS line: http://crbug.com/21414
. The ui guidance from some discussion on chromium-dev was "if it
opens a web page, it should be a link" iirc.

On Mon, Sep 28, 2009 at 11:17 AM,  <thomasvl@chromium.org> wrote:
> why are we making the button in prefs a hyperlink? =A0did windows make th=
is
> switch? =A0it seems like a button makes more sense then a link for this s=
orta
> thing.
>
> http://codereview.chromium.org/248019
>

[Robert Sesek, 2009-09-28 21:09:10]

http://codereview.chromium.org/248019/diff/1/3
File chrome/browser/cocoa/hyperlink_button_cell.h (right):

http://codereview.chromium.org/248019/diff/1/3#newcode15
Line 15: @property (retain) NSURL* url;
On 2009/09/28 17:39:46, Nico wrote:
> Is this used at all?

I thought it may be useful in target-action handlers, but removed.

http://codereview.chromium.org/248019/diff/1/3#newcode16
Line 16: @property (retain) NSColor* color;
On 2009/09/28 17:39:46, Nico wrote:
> "textColor" or "linkColor". "color" is too generic.

Done.

http://codereview.chromium.org/248019/diff/1/4
File chrome/browser/cocoa/hyperlink_button_cell.mm (right):

http://codereview.chromium.org/248019/diff/1/4#newcode29
Line 29: - (NSDictionary*)linkAttributes {
On 2009/09/28 17:39:46, Nico wrote:
> Maybe declare the private functions in an @implementation HyperlinkButtonCell
> (Private) block at the top of this file?

Done.

http://codereview.chromium.org/248019/diff/1/4#newcode33
Line 33: [paragraphStyle setAlignment:NSCenterTextAlignment];
On 2009/09/28 17:39:46, Nico wrote:
> If you use `- (NSTextAlignment)alignment` here, would it then be possible to
> configure the text alignment in IB?

Done.

http://codereview.chromium.org/248019/diff/1/4#newcode36
Line 36: self.color,                             NSForegroundColorAttributeName,
On 2009/09/28 17:39:46, Nico wrote:
> Do not align all the NS* stuff

Done.

http://codereview.chromium.org/248019/diff/1/4#newcode50
Line 50: self.color = [NSColor blueColor];
On 2009/09/28 17:39:46, Nico wrote:
> I haven't seen us use the . syntax anywhere. Go with [self setColor:...] for
> consistency?

We use it elsewhere occasionally, but done.

http://codereview.chromium.org/248019/diff/1/4#newcode66
Line 66: - (id)init {
On 2009/09/28 17:39:46, Nico wrote:
> The init functions in the other files seem to be the first thing in the
> @implementation block, maybe move your inits up too?

Done.

[Nico, 2009-09-29 02:03:23]

LGTM assuming you add the usual AddRemove and Display tests for cells.