|
|
Chromium Code Reviews|
Created:
3 years, 6 months ago by shivanisha Modified:
3 years, 5 months ago CC:
arv+watch_chromium.org, bettes, chromium-reviews, Charlie Harrison, michaelpg+watch-md-settings_chromium.org, michaelpg+watch-md-ui_chromium.org, stevenjb+watch-md-settings_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionUpdated Ads icon for content settings.
Screenshot attached in comment#56 in the associated bug.
BUG=689992
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2947913003
Cr-Commit-Position: refs/heads/master@{#482873}
Committed: https://chromium.googlesource.com/chromium/src/+/6e26093ba629b7aa843f7284c8751d1f752f4b1e
Patch Set 1 #
Total comments: 2
Patch Set 2 : Removed id and fill attributes #Patch Set 3 : Rebased with refs/heads/master@{#481935} #Patch Set 4 : Updated icon as received from bettes@ #
Total comments: 4
Patch Set 5 : Updated icon definition with <rect> #
Total comments: 2
Patch Set 6 : Indentation #
Messages
Total messages: 57 (28 generated)
Description was changed from ========== Updated Ads icon for content settings. BUG=689992 ========== to ========== Updated Ads icon for content settings. BUG=689992 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
The CQ bit was checked by shivanisha@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Updated Ads icon for content settings. BUG=689992 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Updated Ads icon for content settings. Screenshot attached in comment#44 in the associated bug. BUG=689992 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
shivanisha@chromium.org changed reviewers: + thestig@chromium.org
thestig@, PTAL, Thanks!
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
thestig@chromium.org changed reviewers: + dschuyler@chromium.org
Deferring to someone from chrome/browser/resources/settings/OWNERS.
https://codereview.chromium.org/2947913003/diff/1/chrome/browser/resources/se... File chrome/browser/resources/settings/icons.html (right): https://codereview.chromium.org/2947913003/diff/1/chrome/browser/resources/se... chrome/browser/resources/settings/icons.html:13: <path d="M15,3.5 L15,12.5 C15,13.325 14.3077778,14 13.4444444,14 L2.55555556,14 C1.69222222,14 1,13.325 1,12.5 L1,3.5 C1,2.6675 1.69222222,2 2.55555556,2 L13.4444444,2 C14.3077778,2 15,2.6675 15,3.5 Z M7,5 L3,5 L3,11 L4,11 L4,9 L6,9 L6,11 L7,11 L7,5 Z M6,8 L4,8 L4,6 L6,6 L6,8 Z M13,6.5 C13,5.67 12.33,5 11.5,5 L9,5 L9,11 L11.5,11 C12.33,11 13,10.33 13,9.5 L13,6.5 Z M10,6 L12,6 L12,10 L10,10 L10,6 Z" id="Combined-Shape" fill="#5A5A5A" fill-rule="nonzero"></path> Please see if we can get by without: id="Combined-Shape" fill="#5A5A5A" Having a fill in the icon prevents us from setting it in code. I think it's super likely that this internal id is unused.
The CQ bit was checked by shivanisha@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Feedback addressed, PTAL, Thanks! https://codereview.chromium.org/2947913003/diff/1/chrome/browser/resources/se... File chrome/browser/resources/settings/icons.html (right): https://codereview.chromium.org/2947913003/diff/1/chrome/browser/resources/se... chrome/browser/resources/settings/icons.html:13: <path d="M15,3.5 L15,12.5 C15,13.325 14.3077778,14 13.4444444,14 L2.55555556,14 C1.69222222,14 1,13.325 1,12.5 L1,3.5 C1,2.6675 1.69222222,2 2.55555556,2 L13.4444444,2 C14.3077778,2 15,2.6675 15,3.5 Z M7,5 L3,5 L3,11 L4,11 L4,9 L6,9 L6,11 L7,11 L7,5 Z M6,8 L4,8 L4,6 L6,6 L6,8 Z M13,6.5 C13,5.67 12.33,5 11.5,5 L9,5 L9,11 L11.5,11 C12.33,11 13,10.33 13,9.5 L13,6.5 Z M10,6 L12,6 L12,10 L10,10 L10,6 Z" id="Combined-Shape" fill="#5A5A5A" fill-rule="nonzero"></path> On 2017/06/20 at 21:15:51, dschuyler wrote: > Please see if we can get by without: > id="Combined-Shape" fill="#5A5A5A" > > Having a fill in the icon prevents us from setting it in code. > I think it's super likely that this internal id is unused. Yes, the icon works even without the fill and id attributes. Removing them.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_clang on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)
On 2017/06/21 at 07:11:56, shivanisha wrote: > Feedback addressed, PTAL, Thanks! > > https://codereview.chromium.org/2947913003/diff/1/chrome/browser/resources/se... > File chrome/browser/resources/settings/icons.html (right): > > https://codereview.chromium.org/2947913003/diff/1/chrome/browser/resources/se... > chrome/browser/resources/settings/icons.html:13: <path d="M15,3.5 L15,12.5 C15,13.325 14.3077778,14 13.4444444,14 L2.55555556,14 C1.69222222,14 1,13.325 1,12.5 L1,3.5 C1,2.6675 1.69222222,2 2.55555556,2 L13.4444444,2 C14.3077778,2 15,2.6675 15,3.5 Z M7,5 L3,5 L3,11 L4,11 L4,9 L6,9 L6,11 L7,11 L7,5 Z M6,8 L4,8 L4,6 L6,6 L6,8 Z M13,6.5 C13,5.67 12.33,5 11.5,5 L9,5 L9,11 L11.5,11 C12.33,11 13,10.33 13,9.5 L13,6.5 Z M10,6 L12,6 L12,10 L10,10 L10,6 Z" id="Combined-Shape" fill="#5A5A5A" fill-rule="nonzero"></path> > On 2017/06/20 at 21:15:51, dschuyler wrote: > > Please see if we can get by without: > > id="Combined-Shape" fill="#5A5A5A" > > > > Having a fill in the icon prevents us from setting it in code. > > I think it's super likely that this internal id is unused. > > Yes, the icon works even without the fill and id attributes. Removing them. dschuyler@, friendly ping.
On 2017/06/22 at 15:55:32, shivanisha wrote: > On 2017/06/21 at 07:11:56, shivanisha wrote: > > Feedback addressed, PTAL, Thanks! > > > > https://codereview.chromium.org/2947913003/diff/1/chrome/browser/resources/se... > > File chrome/browser/resources/settings/icons.html (right): > > > > https://codereview.chromium.org/2947913003/diff/1/chrome/browser/resources/se... > > chrome/browser/resources/settings/icons.html:13: <path d="M15,3.5 L15,12.5 C15,13.325 14.3077778,14 13.4444444,14 L2.55555556,14 C1.69222222,14 1,13.325 1,12.5 L1,3.5 C1,2.6675 1.69222222,2 2.55555556,2 L13.4444444,2 C14.3077778,2 15,2.6675 15,3.5 Z M7,5 L3,5 L3,11 L4,11 L4,9 L6,9 L6,11 L7,11 L7,5 Z M6,8 L4,8 L4,6 L6,6 L6,8 Z M13,6.5 C13,5.67 12.33,5 11.5,5 L9,5 L9,11 L11.5,11 C12.33,11 13,10.33 13,9.5 L13,6.5 Z M10,6 L12,6 L12,10 L10,10 L10,6 Z" id="Combined-Shape" fill="#5A5A5A" fill-rule="nonzero"></path> > > On 2017/06/20 at 21:15:51, dschuyler wrote: > > > Please see if we can get by without: > > > id="Combined-Shape" fill="#5A5A5A" > > > > > > Having a fill in the icon prevents us from setting it in code. > > > I think it's super likely that this internal id is unused. > > > > Yes, the icon works even without the fill and id attributes. Removing them. > > dschuyler@, friendly ping. Trying to get it in before feature freeze.
On 2017/06/23 at 01:06:50, shivanisha wrote: > On 2017/06/22 at 15:55:32, shivanisha wrote: > > On 2017/06/21 at 07:11:56, shivanisha wrote: > > > Feedback addressed, PTAL, Thanks! > > > > > > https://codereview.chromium.org/2947913003/diff/1/chrome/browser/resources/se... > > > File chrome/browser/resources/settings/icons.html (right): > > > > > > https://codereview.chromium.org/2947913003/diff/1/chrome/browser/resources/se... > > > chrome/browser/resources/settings/icons.html:13: <path d="M15,3.5 L15,12.5 C15,13.325 14.3077778,14 13.4444444,14 L2.55555556,14 C1.69222222,14 1,13.325 1,12.5 L1,3.5 C1,2.6675 1.69222222,2 2.55555556,2 L13.4444444,2 C14.3077778,2 15,2.6675 15,3.5 Z M7,5 L3,5 L3,11 L4,11 L4,9 L6,9 L6,11 L7,11 L7,5 Z M6,8 L4,8 L4,6 L6,6 L6,8 Z M13,6.5 C13,5.67 12.33,5 11.5,5 L9,5 L9,11 L11.5,11 C12.33,11 13,10.33 13,9.5 L13,6.5 Z M10,6 L12,6 L12,10 L10,10 L10,6 Z" id="Combined-Shape" fill="#5A5A5A" fill-rule="nonzero"></path> > > > On 2017/06/20 at 21:15:51, dschuyler wrote: > > > > Please see if we can get by without: > > > > id="Combined-Shape" fill="#5A5A5A" > > > > > > > > Having a fill in the icon prevents us from setting it in code. > > > > I think it's super likely that this internal id is unused. > > > > > > Yes, the icon works even without the fill and id attributes. Removing them. > > > > dschuyler@, friendly ping. > > Trying to get it in before feature freeze. Friendly ping!
thestig@chromium.org changed reviewers: + dpapad@chromium.org
+dpapad to either review or help find the right reviewer.
On 2017/06/23 at 18:40:42, thestig wrote: > +dpapad to either review or help find the right reviewer. @shivanisha: Can you share before/after screenshot?
On 2017/06/23 18:58:35, dpapad wrote: > On 2017/06/23 at 18:40:42, thestig wrote: > > +dpapad to either review or help find the right reviewer. > > @shivanisha: Can you share before/after screenshot? There are some on the bug, and noted in the CL description.
On 2017/06/23 at 19:08:03, thestig wrote: > On 2017/06/23 18:58:35, dpapad wrote: > > On 2017/06/23 at 18:40:42, thestig wrote: > > > +dpapad to either review or help find the right reviewer. > > > > @shivanisha: Can you share before/after screenshot? > > There are some on the bug, and noted in the CL description. and the earlier icon was just the same as the one in popups. It was just a placeholder before this feature was publicly announced.
On 2017/06/23 at 19:11:31, shivanisha wrote: > On 2017/06/23 at 19:08:03, thestig wrote: > > On 2017/06/23 18:58:35, dpapad wrote: > > > On 2017/06/23 at 18:40:42, thestig wrote: > > > > +dpapad to either review or help find the right reviewer. > > > > > > @shivanisha: Can you share before/after screenshot? > > > > There are some on the bug, and noted in the CL description. > > and the earlier icon was just the same as the one in popups. It was just a placeholder before this feature was publicly announced. the earlier placeholder was the same as open-in-new (not popups as mentioned in last comment)
On 2017/06/23 at 19:08:03, thestig wrote: > On 2017/06/23 18:58:35, dpapad wrote: > > On 2017/06/23 at 18:40:42, thestig wrote: > > > +dpapad to either review or help find the right reviewer. > > > > @shivanisha: Can you share before/after screenshot? > > There are some on the bug, and noted in the CL description. Thanks, I see it now at https://bugs.chromium.org/p/chromium/issues/detail?id=689992&desc=2#c44, although there is only the "after". @shivanisha: The icon seems to have the wrong size size and positioning compared to other icons on the page and, see http://imgur.com/a/nJJb3. It seems smaller and squizzed to the top left. I am suspecting that there is unnecessary empty space within the SVG itself that is causing the positioning/size problems.
On 2017/06/23 at 19:24:53, dpapad wrote: > On 2017/06/23 at 19:08:03, thestig wrote: > > On 2017/06/23 18:58:35, dpapad wrote: > > > On 2017/06/23 at 18:40:42, thestig wrote: > > > > +dpapad to either review or help find the right reviewer. > > > > > > @shivanisha: Can you share before/after screenshot? > > > > There are some on the bug, and noted in the CL description. > > Thanks, I see it now at https://bugs.chromium.org/p/chromium/issues/detail?id=689992&desc=2#c44, although there is only the "after". > > @shivanisha: The icon seems to have the wrong size size and positioning compared to other icons on the page and, see http://imgur.com/a/nJJb3. It seems smaller and squizzed to the top left. I am suspecting that there is unnecessary empty space within the SVG itself that is causing the positioning/size problems. dpapad@, This is the SVG file that I got from the UI resources. Do you have a pointer as to how to change it?
On 2017/06/23 at 19:50:33, shivanisha wrote: > On 2017/06/23 at 19:24:53, dpapad wrote: > > On 2017/06/23 at 19:08:03, thestig wrote: > > > On 2017/06/23 18:58:35, dpapad wrote: > > > > On 2017/06/23 at 18:40:42, thestig wrote: > > > > > +dpapad to either review or help find the right reviewer. > > > > > > > > @shivanisha: Can you share before/after screenshot? > > > > > > There are some on the bug, and noted in the CL description. > > > > Thanks, I see it now at https://bugs.chromium.org/p/chromium/issues/detail?id=689992&desc=2#c44, although there is only the "after". > > > > @shivanisha: The icon seems to have the wrong size size and positioning compared to other icons on the page and, see http://imgur.com/a/nJJb3. It seems smaller and squizzed to the top left. I am suspecting that there is unnecessary empty space within the SVG itself that is causing the positioning/size problems. > > dpapad@, This is the SVG file that I got from the UI resources. Do you have a pointer as to how to change it? bettes@chromium.org is MD Settings UX designer. They should be able to provide a better icon, or point you to the right direction. Also looking at the screenshot at https://bugs.chromium.org/p/chromium/issues/detail?id=689992&desc=2#c36, that icon seems to be correctly sized, so maybe figuring out where that icon came from could be helpful.
On 2017/06/23 at 20:28:56, dpapad wrote: > On 2017/06/23 at 19:50:33, shivanisha wrote: > > On 2017/06/23 at 19:24:53, dpapad wrote: > > > On 2017/06/23 at 19:08:03, thestig wrote: > > > > On 2017/06/23 18:58:35, dpapad wrote: > > > > > On 2017/06/23 at 18:40:42, thestig wrote: > > > > > > +dpapad to either review or help find the right reviewer. > > > > > > > > > > @shivanisha: Can you share before/after screenshot? > > > > > > > > There are some on the bug, and noted in the CL description. > > > > > > Thanks, I see it now at https://bugs.chromium.org/p/chromium/issues/detail?id=689992&desc=2#c44, although there is only the "after". > > > > > > @shivanisha: The icon seems to have the wrong size size and positioning compared to other icons on the page and, see http://imgur.com/a/nJJb3. It seems smaller and squizzed to the top left. I am suspecting that there is unnecessary empty space within the SVG itself that is causing the positioning/size problems. > > > > dpapad@, This is the SVG file that I got from the UI resources. Do you have a pointer as to how to change it? > > bettes@chromium.org is MD Settings UX designer. They should be able to provide a better icon, or point you to the right direction. Also looking at the screenshot at https://bugs.chromium.org/p/chromium/issues/detail?id=689992&desc=2#c36, that icon seems to be correctly sized, so maybe figuring out where that icon came from could be helpful. Thanks, mailed bettes@ for help on this.
The CQ bit was checked by shivanisha@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
dpapad@, PTAL , Thanks! Added the icon as received from bettes@ Screenshots attached in comment#55 in the bug.
On 2017/06/27 at 19:17:43, shivanisha wrote: > dpapad@, PTAL , Thanks! > Added the icon as received from bettes@ > > Screenshots attached in comment#55 in the bug. dpapad@, I had used the SVG and taken the path d attribute from there. Is there something else that I missed since the letters don't seem to be rendering correctly?
On 2017/06/27 at 19:59:34, shivanisha wrote: > On 2017/06/27 at 19:17:43, shivanisha wrote: > > dpapad@, PTAL , Thanks! > > Added the icon as received from bettes@ > > > > Screenshots attached in comment#55 in the bug. > > dpapad@, I had used the SVG and taken the path d attribute from there. Is there something else that I missed since the letters don't seem to be rendering correctly? @dschuyler: Does this sound familiar? Could it be related to downsizing the image in a way that distorts it?
On 2017/06/27 at 20:25:10, dpapad wrote: > On 2017/06/27 at 19:59:34, shivanisha wrote: > > On 2017/06/27 at 19:17:43, shivanisha wrote: > > > dpapad@, PTAL , Thanks! > > > Added the icon as received from bettes@ > > > > > > Screenshots attached in comment#55 in the bug. > > > > dpapad@, I had used the SVG and taken the path d attribute from there. Is there something else that I missed since the letters don't seem to be rendering correctly? > > @dschuyler: Does this sound familiar? Could it be related to downsizing the image in a way that distorts it? Is there any other attribute apart form path that we need to add in the code.? I tried adding both path attributes and its still the same issue. The svg I am using is here: https://drive.google.com/corp/drive/u/1/folders/0B8_p1x-BVGf0UHA1WHlTRVRXNlk
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2947913003/diff/60001/chrome/browser/resource... File chrome/browser/resources/settings/icons.html (right): https://codereview.chromium.org/2947913003/diff/60001/chrome/browser/resource... chrome/browser/resources/settings/icons.html:13: <path d="M19,3H5C3.9,3,3,3.9,3,5v14c0,1.1,0.9,2,2,2h14c1.1,0,2-0.9,2-2V5C21,3.9,20.1,3,19,3z M11,15H9.5v-1.5h-2V15H6v-4.5V9h1.5h2H10h1V15z M18,14c0,0.5-0.5,1-1,1h-4V9h4c0.5,0,1,0.5,1,1V14z"</path> Please end the <path ... with a '>' before the </path>
https://codereview.chromium.org/2947913003/diff/60001/chrome/browser/resource... File chrome/browser/resources/settings/icons.html (right): https://codereview.chromium.org/2947913003/diff/60001/chrome/browser/resource... chrome/browser/resources/settings/icons.html:12: <g id="ads"> It looks like the middles of the letters are missing, they would be drawn by: <rect x="7.5" y="10.5" width="2" height="1.5"/> <rect x="14.5" y="10.5" width="2" height="3"/>
On 2017/06/27 20:25:10, dpapad wrote: > On 2017/06/27 at 19:59:34, shivanisha wrote: > > On 2017/06/27 at 19:17:43, shivanisha wrote: > > > dpapad@, PTAL , Thanks! > > > Added the icon as received from bettes@ > > > > > > Screenshots attached in comment#55 in the bug. > > > > dpapad@, I had used the SVG and taken the path d attribute from there. Is > there something else that I missed since the letters don't seem to be rendering > correctly? > > @dschuyler: Does this sound familiar? Could it be related to downsizing the > image in a way that distorts it? The icon does have some damage which I made some review notes about. Also the letters are not true letters, they are shapes - so they do suffer from the same scaling issues. IMO, the A would be more recognizable with sloped sides (rather than the vertical sides that make it look more boxy than triangular).
The CQ bit was checked by shivanisha@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
dschuyler@, As discussed on email, PTAL, Thanks! comment#56 in the bug has the updated screenshots. https://codereview.chromium.org/2947913003/diff/60001/chrome/browser/resource... File chrome/browser/resources/settings/icons.html (right): https://codereview.chromium.org/2947913003/diff/60001/chrome/browser/resource... chrome/browser/resources/settings/icons.html:12: <g id="ads"> On 2017/06/27 at 20:57:39, dschuyler wrote: > It looks like the middles of the letters are missing, they would be drawn by: > <rect x="7.5" y="10.5" width="2" height="1.5"/> > <rect x="14.5" y="10.5" width="2" height="3"/> done https://codereview.chromium.org/2947913003/diff/60001/chrome/browser/resource... chrome/browser/resources/settings/icons.html:13: <path d="M19,3H5C3.9,3,3,3.9,3,5v14c0,1.1,0.9,2,2,2h14c1.1,0,2-0.9,2-2V5C21,3.9,20.1,3,19,3z M11,15H9.5v-1.5h-2V15H6v-4.5V9h1.5h2H10h1V15z M18,14c0,0.5-0.5,1-1,1h-4V9h4c0.5,0,1,0.5,1,1V14z"</path> On 2017/06/27 at 20:48:17, dschuyler wrote: > Please end the <path ... with a '>' before the </path> done
dpapad@chromium.org changed reviewers: - dpapad@chromium.org
Deferring to @dschuyler who has more experienced with SVG icons in Settings.
dschuyler@chromium.org changed reviewers: + dpapad@chromium.org
Code in CL LGTM. Opinion on the icon: the icon itself is not great. A box with two letters doesn't read well or translate well. In this case the first letter is very hard to discern (does it look like an 'A', it could be an 'R'?). https://codereview.chromium.org/2947913003/diff/80001/chrome/browser/resource... File chrome/browser/resources/settings/icons.html (right): https://codereview.chromium.org/2947913003/diff/80001/chrome/browser/resource... chrome/browser/resources/settings/icons.html:15: <rect x="14.5" y="10.5" width="2" height="3"></rect> nit: indent lines 12 to 15 one more level (the are all within the <g>).
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Description was changed from ========== Updated Ads icon for content settings. Screenshot attached in comment#44 in the associated bug. BUG=689992 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Updated Ads icon for content settings. Screenshot attached in comment#56 in the associated bug. BUG=689992 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
dschuyler@, Thanks! https://codereview.chromium.org/2947913003/diff/80001/chrome/browser/resource... File chrome/browser/resources/settings/icons.html (right): https://codereview.chromium.org/2947913003/diff/80001/chrome/browser/resource... chrome/browser/resources/settings/icons.html:15: <rect x="14.5" y="10.5" width="2" height="3"></rect> On 2017/06/28 at 00:11:02, dschuyler wrote: > nit: indent lines 12 to 15 one more level (the are all within the <g>). done
The CQ bit was checked by shivanisha@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dschuyler@chromium.org Link to the patchset: https://codereview.chromium.org/2947913003/#ps100001 (title: "Indentation")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 100001, "attempt_start_ts": 1498614954586670,
"parent_rev": "edd3a026b008f41b7981b5b9786905d5d1b31b41", "commit_rev":
"6e26093ba629b7aa843f7284c8751d1f752f4b1e"}
Message was sent while issue was closed.
Description was changed from ========== Updated Ads icon for content settings. Screenshot attached in comment#56 in the associated bug. BUG=689992 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Updated Ads icon for content settings. Screenshot attached in comment#56 in the associated bug. BUG=689992 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2947913003 Cr-Commit-Position: refs/heads/master@{#482873} Committed: https://chromium.googlesource.com/chromium/src/+/6e26093ba629b7aa843f7284c875... ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as https://chromium.googlesource.com/chromium/src/+/6e26093ba629b7aa843f7284c875... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
