New issue
Advanced search Search tips

Issue 855259 link

Starred by 1 user
Status: Fixed
Owner:
dbeam@chromium.org
Closed: Dec 11
Cc:
markchang@chromium.org
rbpotter@chromium.org
namratakannan@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Downloads card flickers, for .bin files.

Project Member Reported by dpa...@chromium.org, Jun 21 2018 
See attached screencast. Repro steps:

1) Go to https://speed.hetzner.de/
2) Start downloading the 1GB .bin file.
3) Go to chrome://downloads.

The card flickers. It seems that it tries to display an icon, but the icon has an invalid path (revealed by opening the DevTools), which in turn causes a constant flickering of the card contents.
download_flicker.mp4
187 KB View Download

Comment 1 by dpa...@chromium.org, Jul 9

Cc: dbeam@chromium.org

Comment 2 by dbeam@chromium.org, Jul 10 

this code is being hit for me on Linux with google-chrome-unstable (and I locally added some LOG(ERROR)s):

https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/fileicon_source.cc?type=cs&sq=package:chromium&g=0&l=165

oooh, a TODO(glen).  old school.

Comment 3 by dbeam@chromium.org, Jul 10 

for what it's worth, seems like this might be an OK candidate to borrow [inspiration from]: https://cs.chromium.org/chromium/src/chrome/app/theme/default_200_percent/cros/file_types/thumbnails/generic.png?q=generic.png&sq=package:chromium&dr

Comment 4 by dbeam@chromium.org, Jul 10

Cc: -dbeam@chromium.org
Labels: -OS-Mac
Owner: dbeam@chromium.org
Status: Started (was: Available)
i probably need a better image from a designer, but this is what the code change would potentially be (very simple): crrev.com/c/1131005

btw, fairly sure this is platform-specific (i.e. not happening for me on Mac).

Comment 5 by dbeam@chromium.org, Jul 11 

an alternate, more local fix for just downloads: crrev.com/c/1132560

Comment 6 by dbeam@chromium.org, Jul 11 

this is what the second solutiong (from comment 5, crrev.com/c/1132560) looks like
download_linux_filetype_fix.png
53.3 KB View Download

Comment 7 by dbeam@chromium.org, Jul 11

Cc: markchang@chromium.org

Comment 8 by dbeam@chromium.org, Jul 11

Cc: namratakannan@chromium.org

Comment 9 by markchang@chromium.org, Jul 11 

namrata, do we have a stock/standard icon file for this filetype?

Comment 11 by dbeam@chromium.org, Nov 8 

finally got back to this.  before & after: https://imgur.com/a/txRbfIG

Comment 12 by namratakannan@chromium.org, Nov 28 

LGTM1
Project Member

Comment 13 by bugdroid1@chromium.org, Dec 7 

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

commit 6eba080dcab3baea32ab38db4ced27c82eba927e
Author: Dan Beam <dbeam@chromium.org>
Date: Fri Dec 07 01:28:22 2018

WebUI: Add PromiseResolver#isFulfilled to avoid double resolution/rejection

R=dpapad@chromium.org
BUG= 855259 

Change-Id: I42160189ba60c849bcb9cd9578f5e27c47c02791
Reviewed-on: https://chromium-review.googlesource.com/c/1364693
Commit-Queue: Dan Beam <dbeam@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614561}
[modify] https://crrev.com/6eba080dcab3baea32ab38db4ced27c82eba927e/chrome/test/data/webui/promise_resolver_test.html
[modify] https://crrev.com/6eba080dcab3baea32ab38db4ced27c82eba927e/ui/webui/resources/js/promise_resolver.js
Project Member

Comment 14 by bugdroid1@chromium.org, Dec 7 

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

commit af08a78177b3a33571ff54a367494cd83003aecd
Author: Dan Beam <dbeam@chromium.org>
Date: Fri Dec 07 01:41:18 2018

Downloads: separate BrowserProxy from Mojo classes for clarity

R=calamity@chromium.org,dpapad@chromium.org
BUG= 855259 

Change-Id: I8e3ea89831ce9c39ac05afa0d4f18c1c51c3a8cb
Reviewed-on: https://chromium-review.googlesource.com/c/1364233
Commit-Queue: Dan Beam <dbeam@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: calamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614570}
[modify] https://crrev.com/af08a78177b3a33571ff54a367494cd83003aecd/chrome/browser/resources/md_downloads/item.js
[modify] https://crrev.com/af08a78177b3a33571ff54a367494cd83003aecd/chrome/browser/resources/md_downloads/manager.js
[modify] https://crrev.com/af08a78177b3a33571ff54a367494cd83003aecd/chrome/browser/resources/md_downloads/search_service.js
[modify] https://crrev.com/af08a78177b3a33571ff54a367494cd83003aecd/chrome/browser/resources/md_downloads/toolbar.js
[modify] https://crrev.com/af08a78177b3a33571ff54a367494cd83003aecd/chrome/test/data/webui/md_downloads/downloads_browsertest.js
[modify] https://crrev.com/af08a78177b3a33571ff54a367494cd83003aecd/chrome/test/data/webui/md_downloads/manager_tests.js
[add] https://crrev.com/af08a78177b3a33571ff54a367494cd83003aecd/chrome/test/data/webui/md_downloads/test_support.js
Project Member

Comment 15 by bugdroid1@chromium.org, Dec 7 

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

commit 1ec126531ceaebdd0b9ee340ec4d750507290fd7
Author: Dan Beam <dbeam@chromium.org>
Date: Fri Dec 07 04:31:18 2018

Downloads: detect failed file icons and show default thumbnail

On the downloads page, we consult the system for a themed image for the
type of file downloads. Ex: zip files -> archive icon.

When a system doesn't know why type of icon to show for a specific file
type, it can either return a default or nothing. Right now, Linux
returns nothing, so we show nothing.  chrome://fileicon tries to warn
us by giving a network error (which has been ignored until now).

So, when a chrome://fileicon fails to load, show a default filetype
icon in respond.

Before & after screenshots: https://imgur.com/a/txRbfIG

R=dpapad@chromium.org
BUG= 855259 

Change-Id: I370fd8ea119b194ad8e1ba7e0dfad32c6cea1bbe
Reviewed-on: https://chromium-review.googlesource.com/c/1132560
Commit-Queue: Dan Beam <dbeam@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614581}
[modify] https://crrev.com/1ec126531ceaebdd0b9ee340ec4d750507290fd7/chrome/browser/browser_resources.grd
[modify] https://crrev.com/1ec126531ceaebdd0b9ee340ec4d750507290fd7/chrome/browser/resources/md_downloads/BUILD.gn
[add] https://crrev.com/1ec126531ceaebdd0b9ee340ec4d750507290fd7/chrome/browser/resources/md_downloads/icon_loader.html
[add] https://crrev.com/1ec126531ceaebdd0b9ee340ec4d750507290fd7/chrome/browser/resources/md_downloads/icon_loader.js
[modify] https://crrev.com/1ec126531ceaebdd0b9ee340ec4d750507290fd7/chrome/browser/resources/md_downloads/item.html
[modify] https://crrev.com/1ec126531ceaebdd0b9ee340ec4d750507290fd7/chrome/browser/resources/md_downloads/item.js
[modify] https://crrev.com/1ec126531ceaebdd0b9ee340ec4d750507290fd7/chrome/browser/resources/print_preview/data/destination.js
[modify] https://crrev.com/1ec126531ceaebdd0b9ee340ec4d750507290fd7/chrome/browser/resources/print_preview/icons.html
[modify] https://crrev.com/1ec126531ceaebdd0b9ee340ec4d750507290fd7/chrome/browser/resources/print_preview/new/app.html
[modify] https://crrev.com/1ec126531ceaebdd0b9ee340ec4d750507290fd7/chrome/browser/ui/webui/md_downloads/md_downloads_ui.cc
[modify] https://crrev.com/1ec126531ceaebdd0b9ee340ec4d750507290fd7/chrome/test/data/webui/icon_test.html
[modify] https://crrev.com/1ec126531ceaebdd0b9ee340ec4d750507290fd7/chrome/test/data/webui/md_downloads/downloads_browsertest.js
[modify] https://crrev.com/1ec126531ceaebdd0b9ee340ec4d750507290fd7/chrome/test/data/webui/md_downloads/item_tests.js
[modify] https://crrev.com/1ec126531ceaebdd0b9ee340ec4d750507290fd7/chrome/test/data/webui/md_downloads/test_support.js
[modify] https://crrev.com/1ec126531ceaebdd0b9ee340ec4d750507290fd7/ui/webui/resources/cr_elements/icons.html
[modify] https://crrev.com/1ec126531ceaebdd0b9ee340ec4d750507290fd7/ui/webui/resources/js/icon.js
Project Member

Comment 16 by bugdroid1@chromium.org, Dec 7 

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

commit 14d25fb9050e3a5b9eea892514325e7a44d9e894
Author: Samuel Huang <huangs@chromium.org>
Date: Fri Dec 07 16:16:54 2018

Revert "Downloads: detect failed file icons and show default thumbnail"

This reverts commit 1ec126531ceaebdd0b9ee340ec4d750507290fd7.

Reason for revert: Likely to cause consistent failures in single_process_mash_interactive_ui_tests for linux-chromeos-dbg due to assertion failure from missing icon "cr:insert-drive-file".

Bug:  912678 

Original change's description:
> Downloads: detect failed file icons and show default thumbnail
> 
> On the downloads page, we consult the system for a themed image for the
> type of file downloads. Ex: zip files -> archive icon.
> 
> When a system doesn't know why type of icon to show for a specific file
> type, it can either return a default or nothing. Right now, Linux
> returns nothing, so we show nothing.  chrome://fileicon tries to warn
> us by giving a network error (which has been ignored until now).
> 
> So, when a chrome://fileicon fails to load, show a default filetype
> icon in respond.
> 
> Before & after screenshots: https://imgur.com/a/txRbfIG
> 
> R=​dpapad@chromium.org
> BUG= 855259 
> 
> Change-Id: I370fd8ea119b194ad8e1ba7e0dfad32c6cea1bbe
> Reviewed-on: https://chromium-review.googlesource.com/c/1132560
> Commit-Queue: Dan Beam <dbeam@chromium.org>
> Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#614581}

TBR=dbeam@chromium.org,dpapad@chromium.org

Change-Id: I4ef27f132954754a80e3b3def4aadf5774b3875d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  855259 
Reviewed-on: https://chromium-review.googlesource.com/c/1367933
Reviewed-by: Samuel Huang <huangs@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614713}
[modify] https://crrev.com/14d25fb9050e3a5b9eea892514325e7a44d9e894/chrome/browser/browser_resources.grd
[modify] https://crrev.com/14d25fb9050e3a5b9eea892514325e7a44d9e894/chrome/browser/resources/md_downloads/BUILD.gn
[delete] https://crrev.com/b6cfb7f00be42160f7e1d0cdcc78996dd03617d4/chrome/browser/resources/md_downloads/icon_loader.html
[delete] https://crrev.com/b6cfb7f00be42160f7e1d0cdcc78996dd03617d4/chrome/browser/resources/md_downloads/icon_loader.js
[modify] https://crrev.com/14d25fb9050e3a5b9eea892514325e7a44d9e894/chrome/browser/resources/md_downloads/item.html
[modify] https://crrev.com/14d25fb9050e3a5b9eea892514325e7a44d9e894/chrome/browser/resources/md_downloads/item.js
[modify] https://crrev.com/14d25fb9050e3a5b9eea892514325e7a44d9e894/chrome/browser/resources/print_preview/data/destination.js
[modify] https://crrev.com/14d25fb9050e3a5b9eea892514325e7a44d9e894/chrome/browser/resources/print_preview/icons.html
[modify] https://crrev.com/14d25fb9050e3a5b9eea892514325e7a44d9e894/chrome/browser/resources/print_preview/new/app.html
[modify] https://crrev.com/14d25fb9050e3a5b9eea892514325e7a44d9e894/chrome/browser/ui/webui/md_downloads/md_downloads_ui.cc
[modify] https://crrev.com/14d25fb9050e3a5b9eea892514325e7a44d9e894/chrome/test/data/webui/icon_test.html
[modify] https://crrev.com/14d25fb9050e3a5b9eea892514325e7a44d9e894/chrome/test/data/webui/md_downloads/downloads_browsertest.js
[modify] https://crrev.com/14d25fb9050e3a5b9eea892514325e7a44d9e894/chrome/test/data/webui/md_downloads/item_tests.js
[modify] https://crrev.com/14d25fb9050e3a5b9eea892514325e7a44d9e894/chrome/test/data/webui/md_downloads/test_support.js
[modify] https://crrev.com/14d25fb9050e3a5b9eea892514325e7a44d9e894/ui/webui/resources/cr_elements/icons.html
[modify] https://crrev.com/14d25fb9050e3a5b9eea892514325e7a44d9e894/ui/webui/resources/js/icon.js
Project Member

Comment 17 by bugdroid1@chromium.org, Dec 11 

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

commit ee261c6dcce9b2d11bd54a3378291341c9105234
Author: Dan Beam <dbeam@chromium.org>
Date: Tue Dec 11 03:21:00 2018

Revert "Revert "Downloads: detect failed file icons and show default thumbnail""

This reverts commit 14d25fb9050e3a5b9eea892514325e7a44d9e894.

This is a roll-forward with a fix to the URL of an iconset to load
(see the diff between patchsets 1 and 2).

NOPRESUBMIT=true  # existing issue in destination.js

Bug:  855259 ,  912678 
Change-Id: I3aeb26263de682ecb2f0596e336ec20ab5c19807
Reviewed-on: https://chromium-review.googlesource.com/c/1371079
Commit-Queue: Dan Beam <dbeam@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615412}
[modify] https://crrev.com/ee261c6dcce9b2d11bd54a3378291341c9105234/chrome/browser/browser_resources.grd
[modify] https://crrev.com/ee261c6dcce9b2d11bd54a3378291341c9105234/chrome/browser/resources/md_downloads/BUILD.gn
[add] https://crrev.com/ee261c6dcce9b2d11bd54a3378291341c9105234/chrome/browser/resources/md_downloads/icon_loader.html
[add] https://crrev.com/ee261c6dcce9b2d11bd54a3378291341c9105234/chrome/browser/resources/md_downloads/icon_loader.js
[modify] https://crrev.com/ee261c6dcce9b2d11bd54a3378291341c9105234/chrome/browser/resources/md_downloads/item.html
[modify] https://crrev.com/ee261c6dcce9b2d11bd54a3378291341c9105234/chrome/browser/resources/md_downloads/item.js
[modify] https://crrev.com/ee261c6dcce9b2d11bd54a3378291341c9105234/chrome/browser/resources/print_preview/data/destination.js
[modify] https://crrev.com/ee261c6dcce9b2d11bd54a3378291341c9105234/chrome/browser/resources/print_preview/icons.html
[modify] https://crrev.com/ee261c6dcce9b2d11bd54a3378291341c9105234/chrome/browser/resources/print_preview/new/destination_dialog.html
[modify] https://crrev.com/ee261c6dcce9b2d11bd54a3378291341c9105234/chrome/browser/resources/print_preview/new/destination_list_item.html
[modify] https://crrev.com/ee261c6dcce9b2d11bd54a3378291341c9105234/chrome/browser/ui/webui/md_downloads/md_downloads_ui.cc
[modify] https://crrev.com/ee261c6dcce9b2d11bd54a3378291341c9105234/chrome/test/data/webui/icon_test.html
[modify] https://crrev.com/ee261c6dcce9b2d11bd54a3378291341c9105234/chrome/test/data/webui/md_downloads/downloads_browsertest.js
[modify] https://crrev.com/ee261c6dcce9b2d11bd54a3378291341c9105234/chrome/test/data/webui/md_downloads/item_tests.js
[modify] https://crrev.com/ee261c6dcce9b2d11bd54a3378291341c9105234/chrome/test/data/webui/md_downloads/test_support.js
[modify] https://crrev.com/ee261c6dcce9b2d11bd54a3378291341c9105234/ui/webui/resources/cr_elements/icons.html
[modify] https://crrev.com/ee261c6dcce9b2d11bd54a3378291341c9105234/ui/webui/resources/js/icon.js

Comment 18 by dbeam@chromium.org, Dec 11

Status: Fixed (was: Started)
►

Sign in to add a comment