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

Issue 699687 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

[Downloads] Fix download summary icon priorities

Project Member Reported by dtrainor@chromium.org, Mar 8 2017

Issue description

The current download summary prioritizes icons in the following order:

In Progress
Paused
Pending
Completed
Failed

This is wrong.  The order in the spec should be:
In Progress
Pending
Failed
Paused
Completed

Make this change in the DownloadNotificationService.  This will require registering intent handlers for dismissing failed notifications so that we know to update the icon as well.

 
Status: Started (was: Assigned)
CL sent out for review at https://codereview.chromium.org/2751813004/
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 17 2017

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

commit 8055a28acd2bf29534913610369be7ae1b0f894f
Author: dtrainor <dtrainor@chromium.org>
Date: Fri Mar 17 20:18:09 2017

Fix the download summary notification icons

Make the icon ordering follow the expected behavior: progress, pending,
failed, paused, completed.  This requires additional changes to support
listening to when completed and failed notifications are dismissed so we
can update the icon state, so I added a pending intent to those delete
actions as well, but prevented it from restarting the service in case
it's the last notification.  For proper handling of all intent actions,
I still added code to handle the intent for the case where the service
does get started with that action.

BUG= 699687 

Review-Url: https://codereview.chromium.org/2751813004
Cr-Commit-Position: refs/heads/master@{#457873}

[modify] https://crrev.com/8055a28acd2bf29534913610369be7ae1b0f894f/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastReceiver.java
[modify] https://crrev.com/8055a28acd2bf29534913610369be7ae1b0f894f/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java
[modify] https://crrev.com/8055a28acd2bf29534913610369be7ae1b0f894f/chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java
[modify] https://crrev.com/8055a28acd2bf29534913610369be7ae1b0f894f/chrome/android/javatests/src/org/chromium/chrome/browser/download/MockDownloadNotificationService.java

Labels: Merge-Request-58
Status: Fixed (was: Started)
Project Member

Comment 4 by sheriffbot@chromium.org, Mar 17 2017

Labels: -Merge-Request-58 Hotlist-Merge-Approved Merge-Approved-58
Your change meets the bar and is auto-approved for M58. Please go ahead and merge the CL to branch 3029 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 20 2017

Labels: -merge-approved-58 merge-merged-3029
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a965ff59517676e50cbdf4a35c69cd26a627afda

commit a965ff59517676e50cbdf4a35c69cd26a627afda
Author: David Trainor <dtrainor@chromium.org>
Date: Mon Mar 20 05:37:22 2017

Fix the download summary notification icons

Make the icon ordering follow the expected behavior: progress, pending,
failed, paused, completed.  This requires additional changes to support
listening to when completed and failed notifications are dismissed so we
can update the icon state, so I added a pending intent to those delete
actions as well, but prevented it from restarting the service in case
it's the last notification.  For proper handling of all intent actions,
I still added code to handle the intent for the case where the service
does get started with that action.

BUG= 699687 

Review-Url: https://codereview.chromium.org/2751813004
Cr-Commit-Position: refs/heads/master@{#457873}
(cherry picked from commit 8055a28acd2bf29534913610369be7ae1b0f894f)

Review-Url: https://codereview.chromium.org/2757183002 .
Cr-Commit-Position: refs/branch-heads/3029@{#288}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[modify] https://crrev.com/a965ff59517676e50cbdf4a35c69cd26a627afda/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadBroadcastReceiver.java
[modify] https://crrev.com/a965ff59517676e50cbdf4a35c69cd26a627afda/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java
[modify] https://crrev.com/a965ff59517676e50cbdf4a35c69cd26a627afda/chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java
[modify] https://crrev.com/a965ff59517676e50cbdf4a35c69cd26a627afda/chrome/android/javatests/src/org/chromium/chrome/browser/download/MockDownloadNotificationService.java

Sign in to add a comment