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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 183883033: Move bookmarks_ui manifest key from chrome_settings_overrides to chrome_ui_overrides (Closed)

Created:
6 years, 9 months ago by Mike Wittman
Modified:
6 years, 9 months ago
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org
Visibility:
Public.

Description

Move bookmarks_ui manifest key from chrome_settings_overrides to chrome_ui_overrides Separates bookmark UI overrides from settings overrides to avoid future conflict in requirements/goals of the two different pieces of functionality. Retains deprecated support under chrome_settings_overrides for backwards compatibility. This will be removed for the M36 release. BUG=349049 R=kalman@chromium.org, pkasting@chromium.org, vasilii@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=255959

Patch Set 1 #

Total comments: 2

Patch Set 2 : fix indentation #

Patch Set 3 : formatting fixes #

Total comments: 6

Patch Set 4 : address comments and expand docs #

Patch Set 5 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+191 lines, -383 lines) Patch
M chrome/browser/extensions/api/commands/command_service.cc View 1 2 3 4 2 chunks +7 lines, -2 lines 0 comments Download
M chrome/browser/extensions/extension_keybinding_apitest.cc View 1 chunk +1 line, -9 lines 0 comments Download
M chrome/browser/ui/omnibox/location_bar.cc View 1 2 chunks +12 lines, -13 lines 0 comments Download
M chrome/chrome_common.gypi View 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/chrome_tests_unit.gypi View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/extensions/api/_manifest_features.json View 1 2 3 4 1 chunk +12 lines, -0 lines 0 comments Download
M chrome/common/extensions/api/manifest_types.json View 1 2 3 3 chunks +27 lines, -2 lines 0 comments Download
M chrome/common/extensions/chrome_manifest_handlers.cc View 2 chunks +2 lines, -0 lines 0 comments Download
M chrome/common/extensions/docs/templates/articles/settings_override.html View 1 2 3 1 chunk +1 line, -44 lines 0 comments Download
M chrome/common/extensions/docs/templates/articles/ui_override.html View 1 2 3 6 chunks +12 lines, -53 lines 0 comments Download
M chrome/common/extensions/docs/templates/json/manifest.json View 1 2 3 1 chunk +9 lines, -0 lines 0 comments Download
A chrome/common/extensions/docs/templates/public/extensions/ui_override.html View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/extensions/manifest_handlers/settings_overrides_handler.cc View 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/common/extensions/manifest_handlers/settings_overrides_handler_unittest.cc View 2 chunks +4 lines, -1 line 0 comments Download
M chrome/common/extensions/manifest_handlers/ui_overrides_handler.h View 1 2 2 chunks +18 lines, -25 lines 0 comments Download
M chrome/common/extensions/manifest_handlers/ui_overrides_handler.cc View 1 2 2 chunks +41 lines, -165 lines 0 comments Download
M chrome/common/extensions/manifest_handlers/ui_overrides_handler_unittest.cc View 4 chunks +32 lines, -62 lines 0 comments Download
M chrome/test/data/extensions/api_test/keybinding/overwrite_bookmark_shortcut/manifest.json View 1 chunk +1 line, -1 line 0 comments Download
M extensions/common/manifest_constants.h View 1 2 3 4 3 chunks +2 lines, -1 line 0 comments Download
M extensions/common/manifest_constants.cc View 1 2 3 4 4 chunks +3 lines, -3 lines 0 comments Download

Messages

Total messages: 21 (0 generated)
Mike Wittman
PTAL kalman: everything vasilii: changes touching settings override functionality pkasting: chrome/browser/ui/omnibox/location_bar.cc Copies of several files ...
6 years, 9 months ago (2014-03-06 19:05:26 UTC) #1
Peter Kasting
LGTM https://codereview.chromium.org/183883033/diff/1/chrome/browser/ui/omnibox/location_bar.cc File chrome/browser/ui/omnibox/location_bar.cc (right): https://codereview.chromium.org/183883033/diff/1/chrome/browser/ui/omnibox/location_bar.cc#newcode31 chrome/browser/ui/omnibox/location_bar.cc:31: const SettingsOverrides* settings_overrides = Nit: Fix indenting
6 years, 9 months ago (2014-03-06 19:11:26 UTC) #2
Mike Wittman
https://codereview.chromium.org/183883033/diff/1/chrome/browser/ui/omnibox/location_bar.cc File chrome/browser/ui/omnibox/location_bar.cc (right): https://codereview.chromium.org/183883033/diff/1/chrome/browser/ui/omnibox/location_bar.cc#newcode31 chrome/browser/ui/omnibox/location_bar.cc:31: const SettingsOverrides* settings_overrides = On 2014/03/06 19:11:26, Peter Kasting ...
6 years, 9 months ago (2014-03-06 19:48:24 UTC) #3
not at google - send to devlin
lgtm https://codereview.chromium.org/183883033/diff/30001/chrome/common/extensions/api/manifest_types.json File chrome/common/extensions/api/manifest_types.json (right): https://codereview.chromium.org/183883033/diff/30001/chrome/common/extensions/api/manifest_types.json#newcode46 chrome/common/extensions/api/manifest_types.json:46: // TODO(wittman): Remove for M36. also put "nodoc": ...
6 years, 9 months ago (2014-03-06 22:48:30 UTC) #4
Mike Wittman
Also expanded the docs slightly to mention the menu removal/override that occurs with the shortcut ...
6 years, 9 months ago (2014-03-07 02:02:06 UTC) #5
vasilii
lgtm
6 years, 9 months ago (2014-03-07 10:55:39 UTC) #6
Mike Wittman
The CQ bit was checked by wittman@chromium.org
6 years, 9 months ago (2014-03-07 18:21:38 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/wittman@chromium.org/183883033/50001
6 years, 9 months ago (2014-03-07 18:23:30 UTC) #8
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-07 18:23:41 UTC) #9
commit-bot: I haz the power
Failed to apply patch for extensions/common/manifest_constants.cc: While running patch -p1 --forward --force --no-backup-if-mismatch; patching file ...
6 years, 9 months ago (2014-03-07 18:23:42 UTC) #10
Mike Wittman
The CQ bit was checked by wittman@chromium.org
6 years, 9 months ago (2014-03-07 19:55:13 UTC) #11
Mike Wittman
The CQ bit was checked by wittman@chromium.org
6 years, 9 months ago (2014-03-07 19:55:30 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/wittman@chromium.org/183883033/70001
6 years, 9 months ago (2014-03-07 20:23:36 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/wittman@chromium.org/183883033/70001
6 years, 9 months ago (2014-03-08 10:49:58 UTC) #14
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-08 13:39:26 UTC) #15
commit-bot: I haz the power
Retried try job too often on mac_rel for step(s) telemetry_perf_unittests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&number=234289
6 years, 9 months ago (2014-03-08 13:39:26 UTC) #16
Mike Wittman
The CQ bit was checked by wittman@chromium.org
6 years, 9 months ago (2014-03-08 19:32:24 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/wittman@chromium.org/183883033/70001
6 years, 9 months ago (2014-03-08 19:32:45 UTC) #18
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-08 20:17:27 UTC) #19
commit-bot: I haz the power
Retried try job too often on mac_rel for step(s) telemetry_perf_unittests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&number=234553
6 years, 9 months ago (2014-03-08 20:17:28 UTC) #20
Mike Wittman
6 years, 9 months ago (2014-03-10 17:29:36 UTC) #21
Message was sent while issue was closed. 
Committed patchset #5 manually as r255959 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698