New issue
Advanced search Search tips

Issue 851345 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 31
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Make 1st level of "Play Files" root read-only.

Project Member Reported by fukino@chromium.org, Jun 11 2018

Issue description

By default, "Play Files" shows only 4 folders: Pictures, Movies, Music, and Documents.

If we allow users to create folders/files under "Play Files" root, the newly created folders/files will be immediately disappeared.

To avoid this confusion, how about disallowing create files/folders under "Play Files"?
"Play Files/Documents", "Play Files/Snapseed", etc... will stay writable.

Weifang, WDYT?
 
Cc: -fukino@chromium.org weifangsun@chromium.org
Owner: fukino@chromium.org
To confirm, let's go with this approach for M69 as discussed yesterday. Thanks!!
Cc: sdantul...@chromium.org
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/400f1d0cbe91615e3e3b7d962d669f2db26eb72d

commit 400f1d0cbe91615e3e3b7d962d669f2db26eb72d
Author: Naoki Fukino <fukino@chromium.org>
Date: Tue Jul 17 13:56:10 2018

Files app: Make "Play files" root read-only.

Although "Play files" root directory is writable in file system level,
we prohibit write operations on it in the UI level to avoid confusion.
Users can still have write access in sub directories like
Play files/Pictures, /Play files/DCIM, etc...

Bug:  851345 
Test: Manually tested on Kevin
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: Ib5052088aa75a934646081c18c5266fa3fbbac28
Reviewed-on: https://chromium-review.googlesource.com/1140173
Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Commit-Queue: Naoki Fukino <fukino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575628}
[modify] https://crrev.com/400f1d0cbe91615e3e3b7d962d669f2db26eb72d/ui/file_manager/file_manager/background/js/volume_manager_impl.js
[modify] https://crrev.com/400f1d0cbe91615e3e3b7d962d669f2db26eb72d/ui/file_manager/file_manager/background/js/volume_manager_unittest.js

fukino@ - Just checking per your last comment on issue 863638, I didn't realize that the permission update is a blanket read-only, preventing users from deleting folders as well.

Is there a way to separate the two?
It is technically possible to assign different permissions to "create a folder", "delete a folder", "rename a folder", "create a file", "delete a file", and "rename a file."

That said, the implementation will be much more complex and I'm concerned with that the asymmetric property (allowing DELETE but disallowing CREATE/RENAME) could generate unexpected corner cases.

If acceptable, I'd like to make it as just read-only to be on the safe side.
Labels: CrOSFilesFeature-ARC
Thanks for the explanation. Let's go with this approach for M69 launch and revisit the proper permissioning for these use cases on the follow.
Status: Fixed (was: Assigned)

Sign in to add a comment