MD Bookmarks: Support policies for disabling bookmark editing

MD Bookmarks: Support policies for disabling bookmark editing

Bookmark editing can be disabled globally with the 'EditBookmarksEnabled'
policy, or for a single subtree with the 'ManagedBookmarks' policy.

This CL adds support for unmodifiable bookmarks, disabling the following
features for unmodifiable bookmarks:
* Drag and drop
* Toolbar overlay
* Toolbar overflow menu add/sort commands
* Edit/delete commands from context menus and keyboard shortcuts

BUG=708894
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2912893002
Cr-Commit-Position: refs/heads/master@{#479211}
Committed: https://chromium.googlesource.com/chromium/src/+/2437f9950a67ed3838443fb8f23b7d070f69f6a8

[tsergeant, 2017-05-30 06:36:09]

Description was changed from

==========
MD Bookmarks: Support policy for disabling bookmark editing

When the 'EditBookmarksEnabled' policy is disabled, the following bookmark
manager features are disabled:
* Drag and drop
* Toolbar overlay
* Editing commands from context menus and keyboard shortcuts

BUG=708894
==========

to

==========
MD Bookmarks: Support policy for disabling bookmark editing

When the 'EditBookmarksEnabled' policy is disabled, the following bookmark
manager features are disabled:
* Drag and drop
* Toolbar overlay
* Editing commands from context menus and keyboard shortcuts

BUG=708894
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[tsergeant, 2017-06-05 07:06:36]

Description was changed from

==========
MD Bookmarks: Support policy for disabling bookmark editing

When the 'EditBookmarksEnabled' policy is disabled, the following bookmark
manager features are disabled:
* Drag and drop
* Toolbar overlay
* Editing commands from context menus and keyboard shortcuts

BUG=708894
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Bookmarks: Support policy for disabling bookmark editing

Bookmark editing can be disabled globally with the 'EditBookmarksEnabled'
policy, or for a single subtree with the 'ManagedBookmarks' policy.

This CL adds support for unmodifiable bookmarks, disabling the following
features for unmodifiable bookmarks:
* Drag and drop
* Toolbar overlay
* Toolbar overflow menu add/sort commands
* Edit/delete commands from context menus and keyboard shortcuts

BUG=708894
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[tsergeant, 2017-06-07 01:11:18]

Description was changed from

==========
MD Bookmarks: Support policy for disabling bookmark editing

Bookmark editing can be disabled globally with the 'EditBookmarksEnabled'
policy, or for a single subtree with the 'ManagedBookmarks' policy.

This CL adds support for unmodifiable bookmarks, disabling the following
features for unmodifiable bookmarks:
* Drag and drop
* Toolbar overlay
* Toolbar overflow menu add/sort commands
* Edit/delete commands from context menus and keyboard shortcuts

BUG=708894
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Bookmarks: Support policies for disabling bookmark editing

Bookmark editing can be disabled globally with the 'EditBookmarksEnabled'
policy, or for a single subtree with the 'ManagedBookmarks' policy.

This CL adds support for unmodifiable bookmarks, disabling the following
features for unmodifiable bookmarks:
* Drag and drop
* Toolbar overlay
* Toolbar overflow menu add/sort commands
* Edit/delete commands from context menus and keyboard shortcuts

BUG=708894
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[tsergeant, 2017-06-07 06:57:35]

Patchset #3 (id:40001) has been deleted

[tsergeant, 2017-06-07 06:57:45]

Patchset #3 (id:60001) has been deleted

[tsergeant, 2017-06-07 06:58:01]

The CQ bit was checked by tsergeant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-07 06:58:12]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tsergeant, 2017-06-07 06:59:16]

tsergeant@chromium.org changed reviewers:
+ calamity@chromium.org

[tsergeant, 2017-06-07 06:59:17]

This ended up pretty large, which I guess is a consequence of the fact that
there are a few UI surfaces which care about whether editing is possible.

[commit-bot: I haz the power, 2017-06-07 09:24:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-07 09:24:43]

Dry run: This issue passed the CQ dry run.

[calamity, 2017-06-09 06:47:31]

https://codereview.chromium.org/2912893002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/command_manager.html (right):

https://codereview.chromium.org/2912893002/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/command_manager.html:19:
on-tap="onCommandClick_">
Just a note: the menu item hover style isn't removed when you mouse onto a
disabled item. e.g going from Copy URL to a disabled Edit

https://codereview.chromium.org/2912893002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/command_manager.js (right):

https://codereview.chromium.org/2912893002/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/command_manager.js:131: return
this.canEdit_;
Does anything bad happen if this is true?

https://codereview.chromium.org/2912893002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/toolbar.js (right):

https://codereview.chromium.org/2912893002/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/toolbar.js:38: selectedItems_: Number,
Number != Set

https://codereview.chromium.org/2912893002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/util.js (right):

https://codereview.chromium.org/2912893002/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/util.js:104: */
It's definitely worth mentioning here how unmodifiable propagates through
children.

https://codereview.chromium.org/2912893002/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/util.js:117: function
areChildrenUnmodifiable(state, itemId) {
As discussed, it would be good to invert these 2 functions to map to
capabilities.

[tsergeant, 2017-06-13 03:13:01]

The CQ bit was checked by tsergeant@chromium.org to run a CQ dry run

[tsergeant, 2017-06-13 03:13:11]

https://codereview.chromium.org/2912893002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/command_manager.html (right):

https://codereview.chromium.org/2912893002/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/command_manager.html:19:
on-tap="onCommandClick_">
On 2017/06/09 06:47:31, calamity wrote:
> Just a note: the menu item hover style isn't removed when you mouse onto a
> disabled item. e.g going from Copy URL to a disabled Edit

Hmm, interesting. I'll file a bug for that.

https://codereview.chromium.org/2912893002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/command_manager.js (right):

https://codereview.chromium.org/2912893002/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/command_manager.js:131: return
this.canEdit_;
On 2017/06/09 06:47:31, calamity wrote:
> Does anything bad happen if this is true?

The call to chrome.bookmarkManagerPrivate.undo() will throw an exception, but
nothing bad really happens.

https://codereview.chromium.org/2912893002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/toolbar.js (right):

https://codereview.chromium.org/2912893002/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/toolbar.js:38: selectedItems_: Number,
On 2017/06/09 06:47:31, calamity wrote:
> Number != Set

very true

https://codereview.chromium.org/2912893002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/util.js (right):

https://codereview.chromium.org/2912893002/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/util.js:104: */
On 2017/06/09 06:47:31, calamity wrote:
> It's definitely worth mentioning here how unmodifiable propagates through
> children.

Done.

https://codereview.chromium.org/2912893002/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/util.js:117: function
areChildrenUnmodifiable(state, itemId) {
On 2017/06/09 06:47:31, calamity wrote:
> As discussed, it would be good to invert these 2 functions to map to
> capabilities.

Done.

[commit-bot: I haz the power, 2017-06-13 03:13:24]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-13 04:47:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-13 04:47:16]

Dry run: This issue passed the CQ dry run.

[calamity, 2017-06-13 05:13:53]

lgtm

https://codereview.chromium.org/2912893002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/md_bookmarks/command_manager.js (right):

https://codereview.chromium.org/2912893002/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_bookmarks/command_manager.js:131: return
this.canEdit_;
On 2017/06/13 03:13:10, tsergeant wrote:
> On 2017/06/09 06:47:31, calamity wrote:
> > Does anything bad happen if this is true?
> 
> The call to chrome.bookmarkManagerPrivate.undo() will throw an exception, but
> nothing bad really happens.

Acknowledged.

https://codereview.chromium.org/2912893002/diff/100001/chrome/browser/resourc...
File chrome/browser/resources/md_bookmarks/command_manager.js (right):

https://codereview.chromium.org/2912893002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/md_bookmarks/command_manager.js:39: canEdit_: Boolean,
optional nit: consider renaming this globalCanEdit_.

https://codereview.chromium.org/2912893002/diff/100001/chrome/browser/resourc...
File chrome/browser/resources/md_bookmarks/util.js (right):

https://codereview.chromium.org/2912893002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/md_bookmarks/util.js:101: * folders).
Might also be worth mentioning that the unmodifiable nodes are going to be top
level folders and anything beneath the managed bookmarks folder (if that's all
that this will ever apply to).

[tsergeant, 2017-06-13 05:32:16]

https://codereview.chromium.org/2912893002/diff/100001/chrome/browser/resourc...
File chrome/browser/resources/md_bookmarks/command_manager.js (right):

https://codereview.chromium.org/2912893002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/md_bookmarks/command_manager.js:39: canEdit_: Boolean,
On 2017/06/13 05:13:53, calamity wrote:
> optional nit: consider renaming this globalCanEdit_.

Good idea. Done, both here and in <bookmarks-toolbar>

https://codereview.chromium.org/2912893002/diff/100001/chrome/browser/resourc...
File chrome/browser/resources/md_bookmarks/util.js (right):

https://codereview.chromium.org/2912893002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/md_bookmarks/util.js:101: * folders).
On 2017/06/13 05:13:53, calamity wrote:
> Might also be worth mentioning that the unmodifiable nodes are going to be top
> level folders and anything beneath the managed bookmarks folder (if that's all
> that this will ever apply to).

I think that comment will probably just end up restating the actual code, which
I'd probably rather avoid.

[tsergeant, 2017-06-13 05:32:29]

The CQ bit was checked by tsergeant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-13 05:32:44]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-13 06:51:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-13 06:51:54]

Dry run: This issue passed the CQ dry run.

[tsergeant, 2017-06-13 23:48:00]

The CQ bit was checked by tsergeant@chromium.org

[tsergeant, 2017-06-13 23:48:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
calamity@chromium.org
Link to the patchset: https://codereview.chromium.org/2912893002/#ps120001
(title: "canEdit -> globalCanEdit")

[commit-bot: I haz the power, 2017-06-13 23:48:24]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-13 23:54:33]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1497397680211420,
"parent_rev": "061d80fa409b0723b83c0c96661c550761021e1d", "commit_rev":
"089c7d61bf6b634e95a4637b24d94d96ff773991"}

[commit-bot: I haz the power, 2017-06-13 23:54:48]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1497397680211420,
"parent_rev": "c091c545513a4dd4ca9e1740d4f8eea606505d35", "commit_rev":
"2437f9950a67ed3838443fb8f23b7d070f69f6a8"}

[commit-bot: I haz the power, 2017-06-13 23:54:58]

Description was changed from

==========
MD Bookmarks: Support policies for disabling bookmark editing

Bookmark editing can be disabled globally with the 'EditBookmarksEnabled'
policy, or for a single subtree with the 'ManagedBookmarks' policy.

This CL adds support for unmodifiable bookmarks, disabling the following
features for unmodifiable bookmarks:
* Drag and drop
* Toolbar overlay
* Toolbar overflow menu add/sort commands
* Edit/delete commands from context menus and keyboard shortcuts

BUG=708894
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Bookmarks: Support policies for disabling bookmark editing

Bookmark editing can be disabled globally with the 'EditBookmarksEnabled'
policy, or for a single subtree with the 'ManagedBookmarks' policy.

This CL adds support for unmodifiable bookmarks, disabling the following
features for unmodifiable bookmarks:
* Drag and drop
* Toolbar overlay
* Toolbar overflow menu add/sort commands
* Edit/delete commands from context menus and keyboard shortcuts

BUG=708894
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2912893002
Cr-Commit-Position: refs/heads/master@{#479211}
Committed:
https://chromium.googlesource.com/chromium/src/+/2437f9950a67ed3838443fb8f23b...
==========

[commit-bot: I haz the power, 2017-06-13 23:55:00]

Committed patchset #5 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/2437f9950a67ed3838443fb8f23b...