New issue
Advanced search Search tips

Issue 862638 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Jul 11
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Remove dead code for kEnableOverrideBookmarksUI.

Project Member Reported by erikc...@chromium.org, Jul 11

Issue description

This 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, 
 
SGTM.  To confirm, this is only about removing the switch, correct?  (Not the ability for stars to override the bookmarks UI, which I think is still used by the extension?)
Cc: wittman@chromium.org
errr, I was going to remove everything. The extension was a project from many years ago which we never launched. 
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.
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
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.
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...
Screen Shot 2018-07-11 at 2.22.39 PM.png
665 KB View Download
Screen Shot 2018-07-11 at 2.23.40 PM.png
88.9 KB View Download
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.
Status: WontFix (was: Started)
Right-o. Code isn't dead, so closing this bug.
We *could* still remove kEnableOverrideBookmarksUI (the switch) :)  I think that probably is "sufficiently dead".

Sign in to add a comment