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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 10692069: HiDPI assets for product_logo_16 / _32 (Closed)

Created:
8 years, 5 months ago by Nico
Modified:
8 years, 5 months ago
Reviewers:
flackr
CC:
chromium-reviews, Avi (use Gerrit), creis+watch_chromium.org, brettw-cc_chromium.org, arv (Not doing code reviews), oshima+watch_chromium.org, ajwong+watch_chromium.org
Visibility:
Public.

Description

HiDPI assets for product_logo_16 / _32 Use the new logo on about:chrome BUG=134372, 135179 TEST=none Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=145475

Patch Set 1 #

Patch Set 2 : git diff #

Patch Set 3 : rebase #

Total comments: 6

Patch Set 4 : rebase #

Patch Set 5 : add todo #

Unified diffs Side-by-side diffs Delta from patch set Stats (+26 lines, -16 lines) Patch
D chrome/app/theme/chromium/product_logo_16.png View 0 chunks +-1 lines, --1 lines 0 comments Download
D chrome/app/theme/chromium/product_logo_26.png View 0 chunks +-1 lines, --1 lines 0 comments Download
D chrome/app/theme/chromium/product_logo_32.png View 0 chunks +-1 lines, --1 lines 0 comments Download
M chrome/app/theme/theme_resources.grd View 1 1 chunk +0 lines, -6 lines 0 comments Download
M chrome/app/theme/theme_resources_standard.grd View 1 chunk +6 lines, -0 lines 0 comments Download
M chrome/browser/browser_resources.grd View 1 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/first_run/try_chrome_dialog_view.cc View 1 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/resources/chromeos/login/oobe.css View 1 2 3 4 1 chunk +3 lines, -1 line 0 comments Download
M chrome/browser/resources/help/help.css View 1 2 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/browser/resources/help/help.html View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/tab_contents/spelling_bubble_model.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/cocoa/confirm_bubble_controller_unittest.mm View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/cocoa/keystone_infobar_delegate.mm View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/gtk/confirm_bubble_gtk_browsertest.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/ash/launcher/chrome_launcher_controller.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/tab_icon_view.cc View 1 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/web_applications/web_app_mac_unittest.mm View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/chrome_dll.gypi View 1 chunk +1 line, -1 line 0 comments Download
M chrome/chrome_installer.gypi View 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
Nico
8 years, 5 months ago (2012-07-04 11:34:17 UTC) #1
flackr
http://codereview.chromium.org/10692069/diff/5001/chrome/app/theme/theme_resources.grd File chrome/app/theme/theme_resources.grd (left): http://codereview.chromium.org/10692069/diff/5001/chrome/app/theme/theme_resources.grd#oldcode137 chrome/app/theme/theme_resources.grd:137: <include name="IDR_PRODUCT_LOGO_128" file="google_chrome/product_logo_128.png" type="BINDATA" /> I think we should ...
8 years, 5 months ago (2012-07-04 12:09:46 UTC) #2
Nico
Thanks! http://codereview.chromium.org/10692069/diff/5001/chrome/app/theme/theme_resources.grd File chrome/app/theme/theme_resources.grd (left): http://codereview.chromium.org/10692069/diff/5001/chrome/app/theme/theme_resources.grd#oldcode137 chrome/app/theme/theme_resources.grd:137: <include name="IDR_PRODUCT_LOGO_128" file="google_chrome/product_logo_128.png" type="BINDATA" /> On 2012/07/04 12:09:46, ...
8 years, 5 months ago (2012-07-04 16:12:42 UTC) #3
flackr
LGTM http://codereview.chromium.org/10692069/diff/5001/chrome/browser/resources/chromeos/login/oobe.css File chrome/browser/resources/chromeos/login/oobe.css (right): http://codereview.chromium.org/10692069/diff/5001/chrome/browser/resources/chromeos/login/oobe.css#newcode135 chrome/browser/resources/chromeos/login/oobe.css:135: url('chrome://theme/IDR_PRODUCT_LOGO_32@2x') 2x); On 2012/07/04 16:12:42, Nico wrote: > ...
8 years, 5 months ago (2012-07-04 16:36:21 UTC) #4
Nico
8 years, 5 months ago (2012-07-04 16:41:14 UTC) #5 
Thanks!

http://codereview.chromium.org/10692069/diff/5001/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe.css (right):

http://codereview.chromium.org/10692069/diff/5001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe.css:135:
url('chrome://theme/IDR_PRODUCT_LOGO_32@2x') 2x);
On 2012/07/04 16:36:21, flackr wrote:
> On 2012/07/04 16:12:42, Nico wrote:
> > On 2012/07/04 12:09:46, flackr wrote:
> > > This conversion should be automatic, does it not work if you just specify
> > > url('chrome://theme/IDR_PRODUCT_LOGO_32')?
> > 
> > It would probably work, but for that I'd have to move oobe.html from an
> include
> > to a structure, and I don't have a CrOs build to test this change at hand.
So
> > I'm trying to keep the diff simple here too.
> 
> Can you add a TODO for that? Thanks!

Done.

Powered by Google App Engine
This is Rietveld 408576698