Issue 12079033: mac: Move bookmark hierarchy arrow from a pdf to a png.

Issue 12079033: mac: Move bookmark hierarchy arrow from a pdf to a png. (Closed)

Created:
7 years, 10 months ago by Nico

Modified:
7 years, 10 months ago

Reviewers:
Robert Sesek, sail, sky

CC:
chromium-reviews, tfarina, oshima+watch_chromium.org, sail+watch_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

mac: Move bookmark hierarchy arrow from a pdf to a png. Also use a slightly different asset on views. BUG=111101 TEST=click a bookmark folder containing a bookmark folder. Hierarchy arrow still looks good. TBR=sail Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=179408

Patch Set 1 #

Patch Set 2 : . #

Patch Set 3 : margin #

Patch Set 4 : remove pdf #

Total comments: 2

Patch Set 5 : rsesek #

Created: 7 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+22 lines, -1523 lines)			Patch
D	chrome/app/theme/menu_hierarchy_arrow.pdf	View	1 2 3 4	1 chunk	+2 lines, -1515 lines	0 comments	Download
M	chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm	View	1 2 3 4	4 chunks	+8 lines, -3 lines	0 comments	Download
M	chrome/chrome_dll_bundle.gypi	View		1 chunk	+0 lines, -1 line	0 comments	Download
D	ui/resources/default_100_percent/cros/menu_arrow.png	View		0 chunks	+-1 lines, --1 lines	0 comments	Download
D	ui/resources/default_200_percent/cros/menu_arrow.png	View		0 chunks	+-1 lines, --1 lines	0 comments	Download
M	ui/resources/ui_resources.grd	View		1 chunk	+6 lines, -1 line	0 comments	Download
M	ui/views/controls/menu/menu_config_views.cc	View		1 chunk	+2 lines, -1 line	0 comments	Download
M	ui/views/controls/menu/menu_image_util.cc	View	1 2 3 4	3 chunks	+6 lines, -4 lines	0 comments	Download

Messages

Total messages: 10 (0 generated)

Expand Messages | Collapse Messages

Nico

Trung did an over-the-shoulder review of the mac bits (and noticed I wasn't deleting the ...

7 years, 10 months ago (2013-01-29 17:39:50 UTC) #4

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/thakis@chromium.org/12079033/6004

7 years, 10 months ago (2013-01-29 17:40:23 UTC) #5

Robert Sesek

https://codereview.chromium.org/12079033/diff/6004/chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm File chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm (right): https://codereview.chromium.org/12079033/diff/6004/chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm#newcode231 chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm:231: [rb.GetImageNamed(IDR_MENU_HIERARCHY_ARROW).ToNSImage() retain]); drive-by: GetNativeImageNamed

7 years, 10 months ago (2013-01-29 17:55:17 UTC) #6

commit-bot: I haz the power

Sorry for I got bad news for ya. Compile failed with a clobber build on ...

7 years, 10 months ago (2013-01-29 18:00:22 UTC) #7

Nico

7 years, 10 months ago (2013-01-29 18:05:02 UTC) #8

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/thakis@chromium.org/12079033/6007

7 years, 10 months ago (2013-01-29 18:07:56 UTC) #9

sail

7 years, 10 months ago (2013-01-29 18:13:02 UTC) #10

LGTM too!

On a related note, I dislike how this code caches the image. At some point we
should go through the code base and remove all unneeded caches.

Expand Messages | Collapse Messages