New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature

Blocked on:
issue 585840
issue 586082



Sign in to add a comment
link

Issue 581336: Icons for buttons in web notifications

Reported by mvanouwe...@chromium.org, Jan 26 2016 Project Member

Issue description

On some platforms the button rendered for a NotificationAction can display an icon. This example shows it on an Android phone and watch: http://arstechnica.com/gadgets/2014/06/android-wear-review/3/

The spec proposal for this feature is tracked here:
https://github.com/whatwg/notifications/issues/59
 

Comment 1 by mvanouwe...@chromium.org, Jan 26 2016

Prototype of a full implementation for Android:
https://codereview.chromium.org/1620203004/

Comment 2 by peter@chromium.org, Jan 26 2016

Cc: peter@chromium.org

Comment 3 by bugdroid1@chromium.org, Jan 27 2016

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9a2f11e52fc01897767d686f3829cb4ced01bf47

commit 9a2f11e52fc01897767d686f3829cb4ced01bf47
Author: mvanouwerkerk <mvanouwerkerk@chromium.org>
Date: Wed Jan 27 10:15:23 2016

Use NotificationResources instead of a bare SkBitmap.

This prepares the notification feature plumbing for using multiple
resources when buttons can have icons. IPC messages cannot take more
than 5 arguments, so the idea is that all icon resources will be wrapped
in this struct.

BUG= 581336 

Review URL: https://codereview.chromium.org/1634933006

Cr-Commit-Position: refs/heads/master@{#371765}

[modify] http://crrev.com/9a2f11e52fc01897767d686f3829cb4ced01bf47/chrome/browser/notifications/platform_notification_service_impl.cc
[modify] http://crrev.com/9a2f11e52fc01897767d686f3829cb4ced01bf47/chrome/browser/notifications/platform_notification_service_impl.h
[modify] http://crrev.com/9a2f11e52fc01897767d686f3829cb4ced01bf47/chrome/browser/notifications/platform_notification_service_unittest.cc
[modify] http://crrev.com/9a2f11e52fc01897767d686f3829cb4ced01bf47/chrome/browser/push_messaging/push_messaging_notification_manager.cc
[modify] http://crrev.com/9a2f11e52fc01897767d686f3829cb4ced01bf47/content/browser/notifications/notification_message_filter.cc
[modify] http://crrev.com/9a2f11e52fc01897767d686f3829cb4ced01bf47/content/browser/notifications/notification_message_filter.h
[modify] http://crrev.com/9a2f11e52fc01897767d686f3829cb4ced01bf47/content/child/notifications/notification_manager.cc
[modify] http://crrev.com/9a2f11e52fc01897767d686f3829cb4ced01bf47/content/child/notifications/notification_manager.h
[modify] http://crrev.com/9a2f11e52fc01897767d686f3829cb4ced01bf47/content/child/notifications/pending_notifications_tracker.cc
[modify] http://crrev.com/9a2f11e52fc01897767d686f3829cb4ced01bf47/content/child/notifications/pending_notifications_tracker.h
[modify] http://crrev.com/9a2f11e52fc01897767d686f3829cb4ced01bf47/content/common/platform_notification_messages.h
[modify] http://crrev.com/9a2f11e52fc01897767d686f3829cb4ced01bf47/content/content_common.gypi
[modify] http://crrev.com/9a2f11e52fc01897767d686f3829cb4ced01bf47/content/public/browser/platform_notification_service.h
[add] http://crrev.com/9a2f11e52fc01897767d686f3829cb4ced01bf47/content/public/common/notification_resources.h
[modify] http://crrev.com/9a2f11e52fc01897767d686f3829cb4ced01bf47/content/shell/browser/layout_test/layout_test_notification_manager.cc
[modify] http://crrev.com/9a2f11e52fc01897767d686f3829cb4ced01bf47/content/shell/browser/layout_test/layout_test_notification_manager.h

Comment 4 by bugdroid1@chromium.org, Jan 27 2016

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0049a9be71b7cf6c218c0f9ce71068950d776dee

commit 0049a9be71b7cf6c218c0f9ce71068950d776dee
Author: mvanouwerkerk <mvanouwerkerk@chromium.org>
Date: Wed Jan 27 17:19:09 2016

Extract NotificationBuilderBase for holding the arguments.

This enables making smarter decisions in StandardNotificationBuilder#build
in future.

BUG= 581336 

Review URL: https://codereview.chromium.org/1639163003

Cr-Commit-Position: refs/heads/master@{#371810}

[modify] http://crrev.com/0049a9be71b7cf6c218c0f9ce71068950d776dee/chrome/android/java/src/org/chromium/chrome/browser/notifications/CustomNotificationBuilder.java
[delete] http://crrev.com/f352d974e43a73fed311c60c8fcb4dd043b16093/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilder.java
[add] http://crrev.com/0049a9be71b7cf6c218c0f9ce71068950d776dee/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilderBase.java
[modify] http://crrev.com/0049a9be71b7cf6c218c0f9ce71068950d776dee/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java
[modify] http://crrev.com/0049a9be71b7cf6c218c0f9ce71068950d776dee/chrome/android/java/src/org/chromium/chrome/browser/notifications/StandardNotificationBuilder.java
[modify] http://crrev.com/0049a9be71b7cf6c218c0f9ce71068950d776dee/chrome/android/javatests/src/org/chromium/chrome/browser/notifications/CustomNotificationBuilderTest.java

Comment 5 by bugdroid1@chromium.org, Jan 27 2016

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2cf1ff1a5efa4ea9cb61165aaf76007fae85942c

commit 2cf1ff1a5efa4ea9cb61165aaf76007fae85942c
Author: mvanouwerkerk <mvanouwerkerk@chromium.org>
Date: Wed Jan 27 18:38:26 2016

Notifications: restrict button icon to 32dp.

This size is as recommended in the addAction docs:
http://goo.gl/ajjlE0

BUG= 581336 

Review URL: https://codereview.chromium.org/1643493002

Cr-Commit-Position: refs/heads/master@{#371828}

[modify] http://crrev.com/2cf1ff1a5efa4ea9cb61165aaf76007fae85942c/chrome/android/java/res/layout/web_notification_button.xml

Comment 6 by bugdroid1@chromium.org, Feb 10 2016

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4a3e17c874bc4c4c90e5b0f8ec568520964695d4

commit 4a3e17c874bc4c4c90e5b0f8ec568520964695d4
Author: mvanouwerkerk <mvanouwerkerk@chromium.org>
Date: Wed Feb 10 13:03:42 2016

Notification actions may have an icon url.

This is behind a runtime flag for two reasons:
* The implementation is incomplete.
* We're still evaluating the API design.

Intent to Implement and Ship: Notification Action Icons
https://groups.google.com/a/chromium.org/d/msg/blink-dev/IM0HxOP7HOA/y8tu6iq1CgAJ

BUG= 581336 

Review URL: https://codereview.chromium.org/1644573002

Cr-Commit-Position: refs/heads/master@{#374649}

[modify] http://crrev.com/4a3e17c874bc4c4c90e5b0f8ec568520964695d4/content/browser/notifications/notification_database_data.proto
[modify] http://crrev.com/4a3e17c874bc4c4c90e5b0f8ec568520964695d4/content/browser/notifications/notification_database_data_conversions.cc
[modify] http://crrev.com/4a3e17c874bc4c4c90e5b0f8ec568520964695d4/content/browser/notifications/notification_database_data_unittest.cc
[modify] http://crrev.com/4a3e17c874bc4c4c90e5b0f8ec568520964695d4/content/child/notifications/notification_data_conversions.cc
[modify] http://crrev.com/4a3e17c874bc4c4c90e5b0f8ec568520964695d4/content/child/notifications/notification_data_conversions_unittest.cc
[modify] http://crrev.com/4a3e17c874bc4c4c90e5b0f8ec568520964695d4/content/common/platform_notification_messages.h
[modify] http://crrev.com/4a3e17c874bc4c4c90e5b0f8ec568520964695d4/content/public/common/platform_notification_data.h
[modify] http://crrev.com/4a3e17c874bc4c4c90e5b0f8ec568520964695d4/third_party/WebKit/LayoutTests/http/tests/notifications/serviceworker-notification-properties.html
[modify] http://crrev.com/4a3e17c874bc4c4c90e5b0f8ec568520964695d4/third_party/WebKit/Source/modules/notifications/Notification.cpp
[modify] http://crrev.com/4a3e17c874bc4c4c90e5b0f8ec568520964695d4/third_party/WebKit/Source/modules/notifications/NotificationAction.idl
[modify] http://crrev.com/4a3e17c874bc4c4c90e5b0f8ec568520964695d4/third_party/WebKit/Source/modules/notifications/NotificationData.cpp
[modify] http://crrev.com/4a3e17c874bc4c4c90e5b0f8ec568520964695d4/third_party/WebKit/Source/modules/notifications/NotificationDataTest.cpp
[modify] http://crrev.com/4a3e17c874bc4c4c90e5b0f8ec568520964695d4/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in
[modify] http://crrev.com/4a3e17c874bc4c4c90e5b0f8ec568520964695d4/third_party/WebKit/public/platform/modules/notifications/WebNotificationAction.h

Comment 7 by mvanouwe...@chromium.org, Feb 11 2016

Blockedon: chromium:585840

Comment 8 by mvanouwe...@chromium.org, Feb 11 2016

Blockedon: chromium:586082

Comment 9 by bugdroid1@chromium.org, Feb 11 2016

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/897451590bd2134f8952c40c5758b217c923ab03

commit 897451590bd2134f8952c40c5758b217c923ab03
Author: mvanouwerkerk <mvanouwerkerk@chromium.org>
Date: Thu Feb 11 10:58:57 2016

Fetch notification action icons and pass them through in resources.

* Adds an action_icons field to NotificationResources
* Turns PendingNotification into a real class.
* PendingNotification fetches potentially multiple resources before running its callback.
* Adds a unit test for this area of code.

BUG= 581336 , 423039 

Review URL: https://codereview.chromium.org/1644083002

Cr-Commit-Position: refs/heads/master@{#374881}

[modify] http://crrev.com/897451590bd2134f8952c40c5758b217c923ab03/content/child/notifications/notification_image_loader.cc
[modify] http://crrev.com/897451590bd2134f8952c40c5758b217c923ab03/content/child/notifications/notification_image_loader.h
[modify] http://crrev.com/897451590bd2134f8952c40c5758b217c923ab03/content/child/notifications/notification_manager.cc
[add] http://crrev.com/897451590bd2134f8952c40c5758b217c923ab03/content/child/notifications/pending_notification.cc
[add] http://crrev.com/897451590bd2134f8952c40c5758b217c923ab03/content/child/notifications/pending_notification.h
[modify] http://crrev.com/897451590bd2134f8952c40c5758b217c923ab03/content/child/notifications/pending_notifications_tracker.cc
[modify] http://crrev.com/897451590bd2134f8952c40c5758b217c923ab03/content/child/notifications/pending_notifications_tracker.h
[add] http://crrev.com/897451590bd2134f8952c40c5758b217c923ab03/content/child/notifications/pending_notifications_tracker_unittest.cc
[modify] http://crrev.com/897451590bd2134f8952c40c5758b217c923ab03/content/common/platform_notification_messages.h
[modify] http://crrev.com/897451590bd2134f8952c40c5758b217c923ab03/content/content_child.gypi
[modify] http://crrev.com/897451590bd2134f8952c40c5758b217c923ab03/content/content_common.gypi
[modify] http://crrev.com/897451590bd2134f8952c40c5758b217c923ab03/content/content_tests.gypi
[add] http://crrev.com/897451590bd2134f8952c40c5758b217c923ab03/content/public/common/notification_resources.cc
[modify] http://crrev.com/897451590bd2134f8952c40c5758b217c923ab03/content/public/common/notification_resources.h
[add] http://crrev.com/897451590bd2134f8952c40c5758b217c923ab03/content/test/data/notifications/100x100.png
[add] http://crrev.com/897451590bd2134f8952c40c5758b217c923ab03/content/test/data/notifications/110x110.png
[add] http://crrev.com/897451590bd2134f8952c40c5758b217c923ab03/content/test/data/notifications/120x120.png

Comment 10 by bugdroid1@chromium.org, Feb 11 2016

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/647546723325fa08774caf23c1aa71dc15b279c1

commit 647546723325fa08774caf23c1aa71dc15b279c1
Author: mvanouwerkerk <mvanouwerkerk@chromium.org>
Date: Thu Feb 11 19:27:29 2016

Plumb Notification action icons through to the UI layer.

* This makes the feature functional on both Desktop and Android.
* It is still behind the NotificationActionIcons runtime flag.
* Stops using NotificationCompat to enable using the Icon class.

BUG= 581336 

Review URL: https://codereview.chromium.org/1681123002

Cr-Commit-Position: refs/heads/master@{#374918}

[modify] http://crrev.com/647546723325fa08774caf23c1aa71dc15b279c1/chrome/android/java/src/org/chromium/chrome/browser/notifications/CustomNotificationBuilder.java
[modify] http://crrev.com/647546723325fa08774caf23c1aa71dc15b279c1/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilderBase.java
[modify] http://crrev.com/647546723325fa08774caf23c1aa71dc15b279c1/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java
[modify] http://crrev.com/647546723325fa08774caf23c1aa71dc15b279c1/chrome/android/java/src/org/chromium/chrome/browser/notifications/StandardNotificationBuilder.java
[modify] http://crrev.com/647546723325fa08774caf23c1aa71dc15b279c1/chrome/android/javatests/src/org/chromium/chrome/browser/notifications/CustomNotificationBuilderTest.java
[modify] http://crrev.com/647546723325fa08774caf23c1aa71dc15b279c1/chrome/android/javatests/src/org/chromium/chrome/browser/notifications/StandardNotificationBuilderTest.java
[modify] http://crrev.com/647546723325fa08774caf23c1aa71dc15b279c1/chrome/browser/notifications/notification_ui_manager_android.cc
[modify] http://crrev.com/647546723325fa08774caf23c1aa71dc15b279c1/chrome/browser/notifications/platform_notification_service_impl.cc
[modify] http://crrev.com/647546723325fa08774caf23c1aa71dc15b279c1/chrome/browser/notifications/platform_notification_service_unittest.cc

Comment 11 by bugdroid1@chromium.org, Feb 17 2016

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/683791c3ed0d450fc0c311d8b99be2c40cd6f5ff

commit 683791c3ed0d450fc0c311d8b99be2c40cd6f5ff
Author: mvanouwerkerk <mvanouwerkerk@chromium.org>
Date: Wed Feb 17 11:55:45 2016

Add browser test for notification action icons.

Also exposes the --enable-notification-action-icons runtime flag.

BUG= 581336 

Review URL: https://codereview.chromium.org/1693583002

Cr-Commit-Position: refs/heads/master@{#375858}

[modify] http://crrev.com/683791c3ed0d450fc0c311d8b99be2c40cd6f5ff/chrome/app/generated_resources.grd
[modify] http://crrev.com/683791c3ed0d450fc0c311d8b99be2c40cd6f5ff/chrome/browser/about_flags.cc
[modify] http://crrev.com/683791c3ed0d450fc0c311d8b99be2c40cd6f5ff/chrome/browser/notifications/platform_notification_service_browsertest.cc
[modify] http://crrev.com/683791c3ed0d450fc0c311d8b99be2c40cd6f5ff/chrome/test/data/notifications/platform_notification_service.html
[modify] http://crrev.com/683791c3ed0d450fc0c311d8b99be2c40cd6f5ff/chrome/test/data/notifications/platform_notification_service.js
[modify] http://crrev.com/683791c3ed0d450fc0c311d8b99be2c40cd6f5ff/content/browser/renderer_host/render_process_host_impl.cc
[modify] http://crrev.com/683791c3ed0d450fc0c311d8b99be2c40cd6f5ff/content/child/runtime_features.cc
[modify] http://crrev.com/683791c3ed0d450fc0c311d8b99be2c40cd6f5ff/content/public/common/content_switches.cc
[modify] http://crrev.com/683791c3ed0d450fc0c311d8b99be2c40cd6f5ff/content/public/common/content_switches.h
[modify] http://crrev.com/683791c3ed0d450fc0c311d8b99be2c40cd6f5ff/third_party/WebKit/Source/web/WebRuntimeFeatures.cpp
[modify] http://crrev.com/683791c3ed0d450fc0c311d8b99be2c40cd6f5ff/third_party/WebKit/public/web/WebRuntimeFeatures.h
[modify] http://crrev.com/683791c3ed0d450fc0c311d8b99be2c40cd6f5ff/tools/metrics/histograms/histograms.xml

Comment 12 by bugdroid1@chromium.org, Feb 23 2016

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/96390dca62e14a0faf254f226dc9b012e7a2e735

commit 96390dca62e14a0faf254f226dc9b012e7a2e735
Author: mvanouwerkerk <mvanouwerkerk@chromium.org>
Date: Tue Feb 23 15:42:30 2016

Promote Notification action icons to stable.

Intent to implement and ship: https://goo.gl/GXb7lo

BUG= 581336 

Review URL: https://codereview.chromium.org/1714903002

Cr-Commit-Position: refs/heads/master@{#376990}

[modify] https://crrev.com/96390dca62e14a0faf254f226dc9b012e7a2e735/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in

Comment 13 by mvanouwe...@chromium.org, Feb 26 2016

Status: Fixed (was: Assigned)

Comment 14 by bugdroid1@chromium.org, Apr 19 2016

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/876c529f700d91c9c3f30704362ea8876a35f5a9

commit 876c529f700d91c9c3f30704362ea8876a35f5a9
Author: mvanouwerkerk <mvanouwerkerk@chromium.org>
Date: Tue Apr 19 12:37:44 2016

Delete the NotificationActionIcons flag.

This feature shipped in M50:
http://blog.chromium.org/2016/03/chrome-50-beta-push-notification.html

BUG= 581336 
TBR=mkwst,avi

Review URL: https://codereview.chromium.org/1894143002

Cr-Commit-Position: refs/heads/master@{#388190}

[modify] https://crrev.com/876c529f700d91c9c3f30704362ea8876a35f5a9/chrome/app/generated_resources.grd
[modify] https://crrev.com/876c529f700d91c9c3f30704362ea8876a35f5a9/chrome/browser/about_flags.cc
[modify] https://crrev.com/876c529f700d91c9c3f30704362ea8876a35f5a9/chrome/browser/notifications/platform_notification_service_browsertest.cc
[modify] https://crrev.com/876c529f700d91c9c3f30704362ea8876a35f5a9/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/876c529f700d91c9c3f30704362ea8876a35f5a9/content/child/runtime_features.cc
[modify] https://crrev.com/876c529f700d91c9c3f30704362ea8876a35f5a9/content/public/common/content_switches.cc
[modify] https://crrev.com/876c529f700d91c9c3f30704362ea8876a35f5a9/content/public/common/content_switches.h
[modify] https://crrev.com/876c529f700d91c9c3f30704362ea8876a35f5a9/third_party/WebKit/Source/modules/notifications/NotificationAction.idl
[modify] https://crrev.com/876c529f700d91c9c3f30704362ea8876a35f5a9/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in
[modify] https://crrev.com/876c529f700d91c9c3f30704362ea8876a35f5a9/third_party/WebKit/Source/web/WebRuntimeFeatures.cpp
[modify] https://crrev.com/876c529f700d91c9c3f30704362ea8876a35f5a9/third_party/WebKit/public/web/WebRuntimeFeatures.h

Sign in to add a comment