New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 716165 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome , Mac
Pri: 2
Type: Bug
Team-Security-UX



Sign in to add a comment

Quota Management prompt uses a danger triangle permission icon.

Project Member Reported by lgar...@chromium.org, Apr 27 2017

Issue description

What steps will reproduce the problem?
(1) Visit permission.site on Chrome OS
(2) Press "Quota Management"

What is the expected result?
The prompt uses a permission icon related to storage or quota management.

What happens instead?
The prompt use a danger triangle permission icon.

Since we use this icon as the "dangerous" security indicator, I think it's inappropriate for a permission that we let the user grant.

Max, are you aware of this feature? Is there any icon for it?

Dom, could you triage?
 
Screenshot 2017-04-27 at 1.13.48 PM (1).png
80.1 KB View Download
Components: Blink>Storage>Quota

Comment 2 by xiy...@chromium.org, Apr 28 2017

Cc: -maxwalker@chromium.org
Owner: maxwalker@chromium.org
Status: Assigned (was: Untriaged)

Comment 3 by rpop@chromium.org, Sep 7 2017

Cc: ainslie@chromium.org emilyschechter@chromium.org rpop@chromium.org
This just came up in UI review for macviews permission prompts. Let's fix this.

Comment 4 by rpop@chromium.org, Sep 7 2017

Labels: -Pri-3 Pri-2
Cc: dk...@chromium.org
+dknox, can you explain why we have this permission and when it's introduced? ideally we only have permissions about things that are privacy choices.
Cc: maxwalker@chromium.org
Labels: -M-60 M-63 OS-Mac
Owner: dk...@chromium.org
I'm also observing this on Mac.
If we have a prompt for this permission, it should definitely have its own icon and be in Page Info.

Comment 7 by jsb...@chromium.org, Sep 11 2017

This is an old prompt, c/o navigator.webkitPersistentStorage.queryUsageAndQuota() - introduced circa 2011. It's considered deprecated but we haven't actually gone through the exercise of issuing a scary console warning. 

(I also don't think we have UMA stats for webkitPersistentStorage vs. webkitTemporaryStorage, d'oh.)

Anyway, it's an API we wish we'd go away and so we can do whatever we want with it, modulo breaking user experience/sites.

...

The same message/icon is ALSO used when a site tries to trigger the automatic download of multiple (>10?) files at once, which explains the cryptic "Store files on this device" message. Combining the messages presumably made sense at the time. (And storage questions are particularly confusing, so *shrug*). But anyway, this is probably why the dangerous triangle icon is used - downloading multiple files like that is probably naughty.

I believe that one is tied into page info as the "Automatic Downloads" feature.

> The same message/icon is ALSO used when a site tries to trigger the automatic download of multiple (>10?) files at once, which explains the cryptic "Store files on this device" message.
> I believe that one is tied into page info as the "Automatic Downloads" feature.

This is empirically false. You can trigger prompts for "Quota Management" and "Auto Download" independently at permission.site, regardless of what the other is set it.

How about we reuse the download icon for it in the interim, though?
Or some other file-like icon?
Screen Shot 2017-09-11 at 16.58.25.png
7.4 KB View Download

Comment 9 by jsb...@chromium.org, Sep 12 2017

Hrm, I misremembered then. Sorry. Might have been a similar discussion about a newer version of the API (that doesn't ever prompt).

> How about we reuse the download icon for it in the interim, though?

If we have anything more file-ish/storage-ish that'd be better, but I'm not picky.

>> If we have a prompt for this permission, it should definitely have its own icon and be in Page Info.

+1 From a user's perspective it doesn't makes sense that all other permissions can be changed in page info or Settings while this one cannot.

I think reusing the download icon could cause confusion: a person might look for the "Store files" permission in page info after seeing the permission prompt and then find its icon next to the unrelated "Automatic downloads" permission.

WDYT about this icon? https://icons.googleplex.com/#search=new%20folder
Folders are commonly associated with file storage and the integrated plus icon communicates that the site creates a new (sandboxed) folder/file system rather than accessing existing files.
Cc: gbillock@chromium.org
Apparently there's a TODO by gbillock@, who doesn't appear to work in Chrome anymore?

https://cs.chromium.org/chromium/src/chrome/browser/chrome_quota_permission_context.cc?type=cs&q=%22PermissionRequest::IconId+QuotaPermissionRequest::GetIconId()%22&sq=package:chromium&l=90

dknox@, jsbell@: Could one of you take this or triage it for M63?
dknox@, jsbell@: ping :-)
For the prompt icon - how's this?

I'm unfamiliar with the code but https://chromium.googlesource.com/chromium/src/+/1e5c67abd6325c2c316fa9a9430b85079174f0d6/components/vector_icons/README.md was easy enough to follow. Can those be used in Android or does one of the IDR_ANDROID_INFOBAR_* constants need to be used?
folder_icon_permission.png
43.5 KB View Download
Hrm, well, we'll see what the trybots say.
Is it possible we can just deprecate the prompt and use the durable storage permission to gate this instead (which uses a heuristic)?
We'd like to just rip out the feature entirely, but need to understand how it's used. Usage is low, but non-zero (0.002552%). We suspect it's a small number of oldish sites that function like document editors - they prompt the user and the user is likely to accept if they want the functionality. If we just switched to the heuristic we'd likely break them. 

(The behavior is radically different than what happens with the modern persistent permission where existing data just gets a better durability guarantee. With these old APIs the data gets stored in a different place and needs to be accessed differently. Which is why the APIs are flawed and we'd like to remove them.)

FYI, I stuck a CL up at https://chromium-review.googlesource.com/c/chromium/src/+/675582 but it's gotten into the "that would work, can you refactor some code you've never seen before instead..." stage, so I'm not actively working on it.


I ran into that exact same issue on iOS once, and I have had time to fix it again. :-/

Could you push back on that (as long as you're not adding new technical debt)?
If that gets stuck, emilyschechter@, could you help jsbell@ get it unstuck?
I poked on the CL. The reviewers are looking to replace "add an icon resource" with "change how bookmark icons are drawn so that a common icon can be used", if that helps attract the attention of anyone who could help.
Project Member

Comment 21 by bugdroid1@chromium.org, Oct 12 2017

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

commit 0b523e5eb4b00ada5f1cda413caf34476104ea57
Author: Theresa Wellington <twellington@google.com>
Date: Thu Oct 12 01:18:29 2017

[Android] Replace bookmark_folder.png with ic_folder_blue_24dp.png

Replace the grey bookmark_folder.png with ic_folder_blue_24dp.png and
tint existing uses of bookamrk_folder.png. ic_folder_blue_24dp will also
be used as an infobar icon. Infobars are harder to tint than the current
usages of bookmark_folder, so the grey icon is being swapped for a blue
one.

BUG= 716165 

Change-Id: Ia7d3de557bde2164dfe1ad1dcfa5c1d40efafd16
Reviewed-on: https://chromium-review.googlesource.com/709897
Commit-Queue: Theresa <twellington@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508200}
[delete] https://crrev.com/1b4f3c6bdfaf9fd610d2dabf11a686fd1df56951/chrome/android/java/res/drawable-hdpi/bookmark_folder.png
[add] https://crrev.com/0b523e5eb4b00ada5f1cda413caf34476104ea57/chrome/android/java/res/drawable-hdpi/ic_folder_blue_24dp.png
[delete] https://crrev.com/1b4f3c6bdfaf9fd610d2dabf11a686fd1df56951/chrome/android/java/res/drawable-mdpi/bookmark_folder.png
[add] https://crrev.com/0b523e5eb4b00ada5f1cda413caf34476104ea57/chrome/android/java/res/drawable-mdpi/ic_folder_blue_24dp.png
[delete] https://crrev.com/1b4f3c6bdfaf9fd610d2dabf11a686fd1df56951/chrome/android/java/res/drawable-xhdpi/bookmark_folder.png
[add] https://crrev.com/0b523e5eb4b00ada5f1cda413caf34476104ea57/chrome/android/java/res/drawable-xhdpi/ic_folder_blue_24dp.png
[delete] https://crrev.com/1b4f3c6bdfaf9fd610d2dabf11a686fd1df56951/chrome/android/java/res/drawable-xxhdpi/bookmark_folder.png
[add] https://crrev.com/0b523e5eb4b00ada5f1cda413caf34476104ea57/chrome/android/java/res/drawable-xxhdpi/ic_folder_blue_24dp.png
[delete] https://crrev.com/1b4f3c6bdfaf9fd610d2dabf11a686fd1df56951/chrome/android/java/res/drawable-xxxhdpi/bookmark_folder.png
[add] https://crrev.com/0b523e5eb4b00ada5f1cda413caf34476104ea57/chrome/android/java/res/drawable-xxxhdpi/ic_folder_blue_24dp.png
[modify] https://crrev.com/0b523e5eb4b00ada5f1cda413caf34476104ea57/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkFolderRow.java
[modify] https://crrev.com/0b523e5eb4b00ada5f1cda413caf34476104ea57/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkFolderSelectActivity.java
[modify] https://crrev.com/0b523e5eb4b00ada5f1cda413caf34476104ea57/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkUtils.java
[modify] https://crrev.com/0b523e5eb4b00ada5f1cda413caf34476104ea57/chrome/android/java/src/org/chromium/chrome/browser/bookmarkswidget/BookmarkWidgetService.java

Project Member

Comment 22 by bugdroid1@chromium.org, Oct 14 2017

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

commit 82b9c71dc0a69a47cb6164c974dfa80bd604ceb0
Author: Joshua Bell <jsbell@chromium.org>
Date: Sat Oct 14 00:23:52 2017

Use folder icon for persistent quota prompt

The danger triangle icon was used, which is scary. Use a folder icon
instead.

This applies only to the prompt shown used when the Chrome-only
navigator.webkitPersistentStorage.requestQuota() method is used, which
is itself only useful with the Chrome-only
webkitRequestFileSystem(PERSISTENT,...) API.

Bug:  716165 
Change-Id: Ia1d48e1c555a8196aaae57b2c2aaa15fff1712eb
Reviewed-on: https://chromium-review.googlesource.com/675582
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Lucas Garron <lgarron@chromium.org>
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508888}
[modify] https://crrev.com/82b9c71dc0a69a47cb6164c974dfa80bd604ceb0/chrome/app/vector_icons/BUILD.gn
[add] https://crrev.com/82b9c71dc0a69a47cb6164c974dfa80bd604ceb0/chrome/app/vector_icons/folder.icon
[modify] https://crrev.com/82b9c71dc0a69a47cb6164c974dfa80bd604ceb0/chrome/browser/android/resource_id.h
[modify] https://crrev.com/82b9c71dc0a69a47cb6164c974dfa80bd604ceb0/chrome/browser/chrome_quota_permission_context.cc

Status: Fixed (was: Assigned)

Sign in to add a comment