New issue
Advanced search Search tips

Issue 840509 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

need svg replacement for display_notification_icon.png

Project Member Reported by est...@chromium.org, May 7 2018

Issue description

here[0] is an image we're currently using when we display a notification because the resolution's been changed[1]. We should replace it with a .icon.

This will also allow us to remove ash/resources/ash_resources.grd

[0] https://cs.chromium.org/chromium/src/ash/resources/default_100_percent/cros/notification/display_notification_icon.png?q=cros/notification/display_notification_icon.png&sq=package:chromium&dr
[1] https://cs.chromium.org/chromium/src/ash/display/resolution_notification_controller.cc?rcl=4bc42f6223d52c7e885761bd0b93f26763eb38b5&l=227
 
I'm not sure if we need this icon at all anymore as we're not displaying "icons" (right-hand graphic) in the new notification template, we keep that for social notifications.

Didn't we already revamp resolution notifications with a one similar to the one attached? In which case this would enable us to remove this resource.
Screen Shot 2018-05-07 at 1.22.18 PM.png
58.7 KB View Download
it's quite possible we can just remove it. Let me look into it.
Owner: est...@chromium.org
here is what the notification looks like (ignore the text --- that's an artifact of the hacks I used to display the notification on my local build). It's not one of the display notifications that has been updated to look like the above, but I gather it should be. I'll update to use the above template.
upload try 2
resolutionchangenotif.png
22.9 KB View Download
#3, correct it should just use the same template. Thanks for catching that!
Status: Started (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, May 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f57e1c35076dedd3a90ab55adae20772404b6702

commit f57e1c35076dedd3a90ab55adae20772404b6702
Author: Evan Stade <estade@chromium.org>
Date: Wed May 09 20:10:13 2018

cros - Update appearance of resolution change notification.

This removes the last raster asset in ash_resources.grd, so remove that
file and the associated build target.

Bug:  840509 ,505953
Change-Id: I43ec69b5b8b9fc34aa71a1b65f204447e73b133c
Reviewed-on: https://chromium-review.googlesource.com/1050523
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557292}
[modify] https://crrev.com/f57e1c35076dedd3a90ab55adae20772404b6702/ash/BUILD.gn
[modify] https://crrev.com/f57e1c35076dedd3a90ab55adae20772404b6702/ash/display/resolution_notification_controller.cc
[modify] https://crrev.com/f57e1c35076dedd3a90ab55adae20772404b6702/ash/resources/BUILD.gn
[delete] https://crrev.com/a70afb735edb30430772ae2e5a41e75babbc3667/ash/resources/ash_resources.grd
[delete] https://crrev.com/a70afb735edb30430772ae2e5a41e75babbc3667/ash/resources/default_100_percent/cros/notification/display_notification_icon.png
[delete] https://crrev.com/a70afb735edb30430772ae2e5a41e75babbc3667/ash/resources/default_200_percent/cros/notification/display_notification_icon.png
[modify] https://crrev.com/f57e1c35076dedd3a90ab55adae20772404b6702/ash/system/bluetooth/bluetooth_notification_controller.cc
[modify] https://crrev.com/f57e1c35076dedd3a90ab55adae20772404b6702/ash/system/locale/locale_notification_controller.cc
[modify] https://crrev.com/f57e1c35076dedd3a90ab55adae20772404b6702/ash/system/power/battery_notification.cc
[modify] https://crrev.com/f57e1c35076dedd3a90ab55adae20772404b6702/ash/system/power/dual_role_notification.cc
[modify] https://crrev.com/f57e1c35076dedd3a90ab55adae20772404b6702/ash/system/power/peripheral_battery_notifier.cc
[modify] https://crrev.com/f57e1c35076dedd3a90ab55adae20772404b6702/ash/system/power/tray_power.cc
[modify] https://crrev.com/f57e1c35076dedd3a90ab55adae20772404b6702/ash/system/screen_layout_observer.cc
[modify] https://crrev.com/f57e1c35076dedd3a90ab55adae20772404b6702/ash/system/screen_security/screen_capture_tray_item.cc
[modify] https://crrev.com/f57e1c35076dedd3a90ab55adae20772404b6702/ash/system/screen_security/screen_share_tray_item.cc
[modify] https://crrev.com/f57e1c35076dedd3a90ab55adae20772404b6702/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/f57e1c35076dedd3a90ab55adae20772404b6702/chrome/browser/ui/app_list/DEPS
[modify] https://crrev.com/f57e1c35076dedd3a90ab55adae20772404b6702/chrome/browser/ui/app_list/crostini/crostini_app_model_builder.cc
[modify] https://crrev.com/f57e1c35076dedd3a90ab55adae20772404b6702/chrome/chrome_paks.gni
[modify] https://crrev.com/f57e1c35076dedd3a90ab55adae20772404b6702/chrome/test/BUILD.gn
[modify] https://crrev.com/f57e1c35076dedd3a90ab55adae20772404b6702/tools/check_grd_for_unused_strings.py
[modify] https://crrev.com/f57e1c35076dedd3a90ab55adae20772404b6702/tools/gritsettings/resource_ids
[modify] https://crrev.com/f57e1c35076dedd3a90ab55adae20772404b6702/tools/resources/find_unused_resources.py

Project Member

Comment 8 by bugdroid1@chromium.org, May 10 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b6370aecafa241206aa031d26344effbb2d8b50f

commit b6370aecafa241206aa031d26344effbb2d8b50f
Author: Daniel Murphy <dmurph@chromium.org>
Date: Thu May 10 18:37:37 2018

Revert "cros - Update appearance of resolution change notification."

This reverts commit f57e1c35076dedd3a90ab55adae20772404b6702.

Reason for revert: There are ash and mash_ash failures, reverting because it's ash-related. See  crbug.com/841889 


Original change's description:
> cros - Update appearance of resolution change notification.
> 
> This removes the last raster asset in ash_resources.grd, so remove that
> file and the associated build target.
> 
> Bug:  840509 ,505953
> Change-Id: I43ec69b5b8b9fc34aa71a1b65f204447e73b133c
> Reviewed-on: https://chromium-review.googlesource.com/1050523
> Commit-Queue: Evan Stade <estade@chromium.org>
> Reviewed-by: James Cook <jamescook@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#557292}

TBR=jamescook@chromium.org,sky@chromium.org,estade@chromium.org

Change-Id: I4b0ce3e1d01b5b8e390216b379f4158380f542b2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  840509 , 505953,  841889 
Reviewed-on: https://chromium-review.googlesource.com/1054118
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557598}
[modify] https://crrev.com/b6370aecafa241206aa031d26344effbb2d8b50f/ash/BUILD.gn
[modify] https://crrev.com/b6370aecafa241206aa031d26344effbb2d8b50f/ash/display/resolution_notification_controller.cc
[modify] https://crrev.com/b6370aecafa241206aa031d26344effbb2d8b50f/ash/resources/BUILD.gn
[add] https://crrev.com/b6370aecafa241206aa031d26344effbb2d8b50f/ash/resources/ash_resources.grd
[add] https://crrev.com/b6370aecafa241206aa031d26344effbb2d8b50f/ash/resources/default_100_percent/cros/notification/display_notification_icon.png
[add] https://crrev.com/b6370aecafa241206aa031d26344effbb2d8b50f/ash/resources/default_200_percent/cros/notification/display_notification_icon.png
[modify] https://crrev.com/b6370aecafa241206aa031d26344effbb2d8b50f/ash/system/bluetooth/bluetooth_notification_controller.cc
[modify] https://crrev.com/b6370aecafa241206aa031d26344effbb2d8b50f/ash/system/locale/locale_notification_controller.cc
[modify] https://crrev.com/b6370aecafa241206aa031d26344effbb2d8b50f/ash/system/power/battery_notification.cc
[modify] https://crrev.com/b6370aecafa241206aa031d26344effbb2d8b50f/ash/system/power/dual_role_notification.cc
[modify] https://crrev.com/b6370aecafa241206aa031d26344effbb2d8b50f/ash/system/power/peripheral_battery_notifier.cc
[modify] https://crrev.com/b6370aecafa241206aa031d26344effbb2d8b50f/ash/system/power/tray_power.cc
[modify] https://crrev.com/b6370aecafa241206aa031d26344effbb2d8b50f/ash/system/screen_layout_observer.cc
[modify] https://crrev.com/b6370aecafa241206aa031d26344effbb2d8b50f/ash/system/screen_security/screen_capture_tray_item.cc
[modify] https://crrev.com/b6370aecafa241206aa031d26344effbb2d8b50f/ash/system/screen_security/screen_share_tray_item.cc
[modify] https://crrev.com/b6370aecafa241206aa031d26344effbb2d8b50f/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/b6370aecafa241206aa031d26344effbb2d8b50f/chrome/browser/ui/app_list/DEPS
[modify] https://crrev.com/b6370aecafa241206aa031d26344effbb2d8b50f/chrome/browser/ui/app_list/crostini/crostini_app_model_builder.cc
[modify] https://crrev.com/b6370aecafa241206aa031d26344effbb2d8b50f/chrome/chrome_paks.gni
[modify] https://crrev.com/b6370aecafa241206aa031d26344effbb2d8b50f/chrome/test/BUILD.gn
[modify] https://crrev.com/b6370aecafa241206aa031d26344effbb2d8b50f/tools/check_grd_for_unused_strings.py
[modify] https://crrev.com/b6370aecafa241206aa031d26344effbb2d8b50f/tools/gritsettings/resource_ids
[modify] https://crrev.com/b6370aecafa241206aa031d26344effbb2d8b50f/tools/resources/find_unused_resources.py

Project Member

Comment 9 by bugdroid1@chromium.org, May 10 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/13e8011c812d7b42b072039b0b859ee82aaae055

commit 13e8011c812d7b42b072039b0b859ee82aaae055
Author: Evan Stade <estade@chromium.org>
Date: Thu May 10 21:21:01 2018

Reland "cros - Update appearance of resolution change notification."

This is a reland of f57e1c35076dedd3a90ab55adae20772404b6702
which was reverted due to being blamed for the msan failures which
were fixed by d8f72fd2090cb340d5151f985955c73734353ba3

Original change's description:
> cros - Update appearance of resolution change notification.
>
> This removes the last raster asset in ash_resources.grd, so remove that
> file and the associated build target.
>
> Bug:  840509 ,505953
> Change-Id: I43ec69b5b8b9fc34aa71a1b65f204447e73b133c
> Reviewed-on: https://chromium-review.googlesource.com/1050523
> Commit-Queue: Evan Stade <estade@chromium.org>
> Reviewed-by: James Cook <jamescook@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#557292}

TBR=jamescook@chromium.org,sky@chromium.org

Bug:  840509 , 505953
Change-Id: Iae0b07cbf2e14e307b74e11d07d21290ee72291b
Reviewed-on: https://chromium-review.googlesource.com/1054222
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557679}
[modify] https://crrev.com/13e8011c812d7b42b072039b0b859ee82aaae055/ash/BUILD.gn
[modify] https://crrev.com/13e8011c812d7b42b072039b0b859ee82aaae055/ash/display/resolution_notification_controller.cc
[modify] https://crrev.com/13e8011c812d7b42b072039b0b859ee82aaae055/ash/resources/BUILD.gn
[delete] https://crrev.com/f7e9d43def9ef8ebc79dc975f57e3faca7b288fc/ash/resources/ash_resources.grd
[delete] https://crrev.com/f7e9d43def9ef8ebc79dc975f57e3faca7b288fc/ash/resources/default_100_percent/cros/notification/display_notification_icon.png
[delete] https://crrev.com/f7e9d43def9ef8ebc79dc975f57e3faca7b288fc/ash/resources/default_200_percent/cros/notification/display_notification_icon.png
[modify] https://crrev.com/13e8011c812d7b42b072039b0b859ee82aaae055/ash/system/bluetooth/bluetooth_notification_controller.cc
[modify] https://crrev.com/13e8011c812d7b42b072039b0b859ee82aaae055/ash/system/locale/locale_notification_controller.cc
[modify] https://crrev.com/13e8011c812d7b42b072039b0b859ee82aaae055/ash/system/power/battery_notification.cc
[modify] https://crrev.com/13e8011c812d7b42b072039b0b859ee82aaae055/ash/system/power/dual_role_notification.cc
[modify] https://crrev.com/13e8011c812d7b42b072039b0b859ee82aaae055/ash/system/power/peripheral_battery_notifier.cc
[modify] https://crrev.com/13e8011c812d7b42b072039b0b859ee82aaae055/ash/system/power/tray_power.cc
[modify] https://crrev.com/13e8011c812d7b42b072039b0b859ee82aaae055/ash/system/screen_layout_observer.cc
[modify] https://crrev.com/13e8011c812d7b42b072039b0b859ee82aaae055/ash/system/screen_security/screen_capture_tray_item.cc
[modify] https://crrev.com/13e8011c812d7b42b072039b0b859ee82aaae055/ash/system/screen_security/screen_share_tray_item.cc
[modify] https://crrev.com/13e8011c812d7b42b072039b0b859ee82aaae055/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/13e8011c812d7b42b072039b0b859ee82aaae055/chrome/browser/ui/app_list/DEPS
[modify] https://crrev.com/13e8011c812d7b42b072039b0b859ee82aaae055/chrome/browser/ui/app_list/crostini/crostini_app_model_builder.cc
[modify] https://crrev.com/13e8011c812d7b42b072039b0b859ee82aaae055/chrome/chrome_paks.gni
[modify] https://crrev.com/13e8011c812d7b42b072039b0b859ee82aaae055/chrome/test/BUILD.gn
[modify] https://crrev.com/13e8011c812d7b42b072039b0b859ee82aaae055/tools/check_grd_for_unused_strings.py
[modify] https://crrev.com/13e8011c812d7b42b072039b0b859ee82aaae055/tools/gritsettings/resource_ids
[modify] https://crrev.com/13e8011c812d7b42b072039b0b859ee82aaae055/tools/resources/find_unused_resources.py

Status: Fixed (was: Started)

Sign in to add a comment