Remove dead code for kEnableOverrideBookmarksUI. |
|||
Issue descriptionThis flag: """ // Enables extensions to hide bookmarks UI elements. const char kEnableOverrideBookmarksUI[] = "enable-override-bookmarks-ui"; """ was added for the stars project, which is now defunct. Removing it will simplify extension accelerator handling,
,
Jul 11
errr, I was going to remove everything. The extension was a project from many years ago which we never launched.
,
Jul 11
The project did launch as a first party extension and has 860k users: https://chrome.google.com/webstore/detail/bookmark-manager/gmlllbghnfkpflemihljekbapjopfjik You'll need to coordinate with the team behind that to remove support that's in use by the extension.
,
Jul 11
Something odd is going on. Their extension manifest.json has:
"""
6 "chrome_ui_overrides": {
7 "bookmarks_ui": {
8 "remove_bookmark_shortcut": true,
9 "remove_button": true
10 }
11 },
"""
However - those should have no effect unless the user has started chrome with a special switch. Is it because the validation code is missing an early return?
https://cs.chromium.org/chromium/src/chrome/common/extensions/manifest_handlers/ui_overrides_handler.cc?type=cs&q=enable_override_bookmarks_ui&g=0&l=156
,
Jul 11
The extension is whitelisted for access to that feature, among other things: https://cs.chromium.org/chromium/src/chrome/common/extensions/api/_manifest_features.json?rcl=3cc018ce0214d784aa6a1f2a6e6e70d7ccffcb7c&l=68-70
,
Jul 11
See also this code: https://chromium.googlesource.com/chromium/src/+/8fc67b341c8d30821f9df226a769d97e86523f02/chrome/browser/ui/location_bar/location_bar.cc#65 if (extensions::UIOverrides::RemovesBookmarkButton(i->get()) && ((*i)->permissions_data()->HasAPIPermission( extensions::APIPermission::kBookmarkManagerPrivate) || extensions::FeatureSwitch::enable_override_bookmarks_ui() ->IsEnabled())) return true; We check if *either* the extension has the BookmarkManagerPrivate permission *or* the switch is enabled.
,
Jul 11
I installed the extension and it doesn't appear to be working properly. The images aren't populating. Furthermore, recent user reviews also seem to imply that it isn't being maintained...
,
Jul 11
I don't think it is being maintained. I'd be supportive of fully deprecating and removing it, but we'll need to run it by more folks than just us, and likely not a discussion for this bug.
,
Jul 11
Right-o. Code isn't dead, so closing this bug.
,
Jul 11
We *could* still remove kEnableOverrideBookmarksUI (the switch) :) I think that probably is "sufficiently dead". |
|||
►
Sign in to add a comment |
|||
Comment 1 by rdevlin....@chromium.org
, Jul 11