Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(291)

Issue 19789018: --disable-download-shelf (Closed)

Created:
7 years, 5 months ago by benjhayden
Modified:
7 years, 5 months ago
Reviewers:
asanka
CC:
chromium-reviews
Visibility:
Public.

Description

--disable-download-shelf Background: This is the original name of the --new-downloads-ui flag, which caused lots of confusion because there is no new downloads UI. http://codereview.chromium.org/7605003 https://chromiumcodereview.appspot.com/10573012 There are two expected use cases: * Some users do not like the shelf. * Some users will install extensions to provide a different downloads UI, and will not want the shelf in addition to their new UI. Note that these extensions cannot replace chrome://downloads. There are technical difficulties with putting this functionality in the extension API, which could be overcome, but it conceptually does not belong there anyway. If you like the shelf, then do not turn the flag on. If you rely on the shelf to be notified when downloads happen, and/or you don't trust your extension to notify you, then do not turn the flag on. If the UI team can ever agree on a good UI to replace the shelf and/or chrome://downloads natively, then this flag may become unnecessary, or it may be retargeted at disabling that UI. I would love to move this functionality into a checkbox in chrome://settings so that we can use it on our corp chromebooks, but do not expect that idea to gain any traction. PS1@r213456

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+20 lines, -0 lines) Patch
M chrome/app/generated_resources.grd View 1 chunk +6 lines, -0 lines 1 comment Download
M chrome/browser/about_flags.cc View 1 chunk +7 lines, -0 lines 1 comment Download
M chrome/browser/ui/browser.cc View 1 chunk +4 lines, -0 lines 1 comment Download
M chrome/common/chrome_switches.h View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/chrome_switches.cc View 1 chunk +2 lines, -0 lines 1 comment Download

Messages

Total messages: 2 (0 generated)
benjhayden
7 years, 5 months ago (2013-07-24 19:28:35 UTC) #1
asanka
7 years, 5 months ago (2013-07-24 20:09:57 UTC) #2
The general consensus seems to be to avoid permanent flags. It might be worth
bringing this up on crbug.com/89922 and seeing if a UI option is viable.
Alternatively we could consider adding an API to disable the shelf from an
extension.

https://codereview.chromium.org/19789018/diff/1/chrome/app/generated_resource...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/19789018/diff/1/chrome/app/generated_resource...
chrome/app/generated_resources.grd:7043: +        Disable the gray download
shelf at the bottom of the window. There will be no indication of downloads
unless an extension provides it.
The color of the shelf depends on the theme.

You could mention that the download would show up in the downloads page.

https://codereview.chromium.org/19789018/diff/1/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/19789018/diff/1/chrome/browser/about_flags.cc...
chrome/browser/about_flags.cc:1002: "disable-download-shelf",
Could you add a '// FLAGS:RECORD_UMA' and update chromeactions.txt so that we'll
have some metrics?

https://codereview.chromium.org/19789018/diff/1/chrome/browser/ui/browser.cc
File chrome/browser/ui/browser.cc (right):

https://codereview.chromium.org/19789018/diff/1/chrome/browser/ui/browser.cc#...
chrome/browser/ui/browser.cc:1258: if
(CommandLine::ForCurrentProcess()->HasSwitch(
Nit: Check this before the ShowShowInShelf() test since that test would be
irrelevant if the shelf is disabled.

https://codereview.chromium.org/19789018/diff/1/chrome/common/chrome_switches.cc
File chrome/common/chrome_switches.cc (right):

https://codereview.chromium.org/19789018/diff/1/chrome/common/chrome_switches...
chrome/common/chrome_switches.cc:305: const char kDisableDownloadShelf[] =
"disable-download-shelf";
Nit: It seems the convention here is to line up the '=' signs.

Powered by Google App Engine
This is Rietveld 408576698