DOMUI: Turn the google dashboard link into a real link and fix the behavior of

DOMUI: Turn the google dashboard link into a real link and fix the behavior of
the 'Get Themes' link.

BUG=62887
TEST=none

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

[arv (Not doing code reviews), 2010-11-12 01:17:25]

http://codereview.chromium.org/4821003/diff/1/chrome/browser/dom_ui/options/p...
File chrome/browser/dom_ui/options/personal_options_handler.cc (right):

http://codereview.chromium.org/4821003/diff/1/chrome/browser/dom_ui/options/p...
chrome/browser/dom_ui/options/personal_options_handler.cc:126:
"getThemesGalleryURL",
Is there a reason why this cannot be included in the localized strings and
injected using i18n-values="href:themes_gallery_url"?

http://codereview.chromium.org/4821003/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/personal_options.js (right):

http://codereview.chromium.org/4821003/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/personal_options.js:185:
$('themes-gallery').target = '_blank';  // Open in a new tab.
Can you link to a webkit bug here?

[James Hawkins, 2010-11-12 21:15:29]

http://codereview.chromium.org/4821003/diff/1/chrome/browser/dom_ui/options/p...
File chrome/browser/dom_ui/options/personal_options_handler.cc (right):

http://codereview.chromium.org/4821003/diff/1/chrome/browser/dom_ui/options/p...
chrome/browser/dom_ui/options/personal_options_handler.cc:126:
"getThemesGalleryURL",
On 2010/11/12 01:17:25, arv wrote:
> Is there a reason why this cannot be included in the localized strings and
> injected using i18n-values="href:themes_gallery_url"?

I tried using that construct and it didn't work.  Is it supposed to?

[arv (Not doing code reviews), 2010-11-12 22:12:28]

Yeah, that works in other places.

erik



On Fri, Nov 12, 2010 at 13:15,  <jhawkins@chromium.org> wrote:
>
>
http://codereview.chromium.org/4821003/diff/1/chrome/browser/dom_ui/options/p...
> File chrome/browser/dom_ui/options/personal_options_handler.cc (right):
>
>
http://codereview.chromium.org/4821003/diff/1/chrome/browser/dom_ui/options/p...
> chrome/browser/dom_ui/options/personal_options_handler.cc:126:
> "getThemesGalleryURL",
> On 2010/11/12 01:17:25, arv wrote:
>>
>> Is there a reason why this cannot be included in the localized strings
>
> and
>>
>> injected using i18n-values="href:themes_gallery_url"?
>
> I tried using that construct and it didn't work.  Is it supposed to?
>
> http://codereview.chromium.org/4821003/
>

[James Hawkins, 2010-11-12 23:47:20]

http://codereview.chromium.org/4821003/diff/1/chrome/browser/dom_ui/options/p...
File chrome/browser/dom_ui/options/personal_options_handler.cc (right):

http://codereview.chromium.org/4821003/diff/1/chrome/browser/dom_ui/options/p...
chrome/browser/dom_ui/options/personal_options_handler.cc:126:
"getThemesGalleryURL",
On 2010/11/12 01:17:25, arv wrote:
> Is there a reason why this cannot be included in the localized strings and
> injected using i18n-values="href:themes_gallery_url"?

Done.

http://codereview.chromium.org/4821003/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/personal_options.js (right):

http://codereview.chromium.org/4821003/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/personal_options.js:185:
$('themes-gallery').target = '_blank';  // Open in a new tab.
On 2010/11/12 01:17:25, arv wrote:
> Can you link to a webkit bug here?

I've actually removed this piece of code. Do you want me to file a WK bug?

[James Hawkins, 2010-11-13 03:47:24]

ping

[James Hawkins, 2010-11-15 19:37:14]

+csilv

[arv (Not doing code reviews), 2010-11-15 19:42:22]

LGTM

[csilv, 2010-11-15 19:55:53]

LGTM.  Gotta love code deletion. ;-)