extensions/common/api/_api_features.json - Issue 2379573008: Add SetImageData api to chrome.clipboard.

Unified Diff: extensions/common/api/_api_features.json

Issue 2379573008: Add SetImageData api to chrome.clipboard. (Closed)

Patch Set: Fix interactive_ui_tests issue. Created 4 years, 1 month ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

« extensions/browser/test_extensions_browser_client.cc ('K') | « extensions/browser/test_extensions_browser_client.cc ('k') | extensions/common/api/clipboard.idl » ('j') | extensions/common/api/clipboard.idl » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: extensions/common/api/_api_features.json

diff --git a/extensions/common/api/_api_features.json b/extensions/common/api/_api_features.json

index 5380a15a5e340fb2e79b95d93053db5e51838c87..217061df35a91138fcde7bae42ec862be1fbcc52 100644

--- a/extensions/common/api/_api_features.json

+++ b/extensions/common/api/_api_features.json

@@ -113,7 +113,9 @@

"contexts": ["blessed_extension"]

"clipboard": {

- "dependencies": ["permission:clipboard", "permission:clipboardRead"],

+ "dependencies": [

+ "permission:clipboard", "permission:clipboardReadWrite"

Devlin 2016/11/16 02:34:33 This will disable this entire api for any existing

jennyz 2016/12/07 01:21:48 This extension is a new one we added for Citrix re

Devlin 2016/12/09 15:23:43 Where is this whitelist enforced?

jennyz 2016/12/20 22:16:54 No, I didn't enforce the whitelist. According to y

Devlin 2016/12/22 16:07:49 But this API isn't *only* used for the new functio

jennyz 2016/12/22 22:45:20 Good point. I have removed clipboardReadWrite perm

jennyz 2016/12/22 23:31:40 I read your comment again. After I removed the add

I read your comment again. After I removed the additional clipboardReadWrite permission, if we need to make sure onClipboardDataChanged only requires clipboardRead permission and SetImageData only requires clipboardWrite, then the permission requirement is refined to API level instead of chrome.clipboard namespace level, do I need to change _api_features_.json file to reconfigure the permission dependency?

Devlin 2016/12/27 17:08:00 Yes, if you want different permissions for differe

On 2016/12/22 23:31:40, jennyz wrote: > On 2016/12/22 22:45:20, jennyz wrote: > > On 2016/12/22 16:07:49, Devlin wrote: > > > On 2016/12/20 22:16:54, jennyz wrote: > > > > On 2016/12/09 15:23:43, Devlin wrote: > > > > > On 2016/12/07 01:21:48, jennyz wrote: > > > > > > On 2016/11/16 02:34:33, Devlin wrote: > > > > > > > This will disable this entire api for any existing extensions. > > > > > > > > > > > > This extension is a new one we added for Citrix remote app, so far > they > > > are > > > > > the > > > > > > only client who uses this API, no one has use this extension yet, so > it > > > > should > > > > > > be ok to make change now. > > > > > > > > > > Where is this whitelist enforced? > > > > > > > > No, I didn't enforce the whitelist. According to your comment later, I > > thought > > > > it is OK for me to document the deprecation plan in the clipboard.idl, > > instead > > > > of enforcing the whitelist. > > > > > > But this API isn't *only* used for the new function being added; it's also > > used > > > for the onClipboardDataChanged event. Since the clipboard API is not > > restricted > > > to a whitelist, arbitrary apps and extensions can be relying on it. If we > add > > > an additional dependency, then any apps or extensions that don't specify > that > > > dependency will be unable to use it, and will suddenly break in this chrome > > > version. Additionally, there's no reason that an extension using the > > > onClipboardDataChanged event requires clipboardWrite, and no reason that > > setting > > > the image data to the clipboard requires clipboard read. Right? > > Good point. I have removed clipboardReadWrite permission in the new patch. So > > the app depending on onClipboardDataChanged will only need to require > > clipboardRead permission. > > I read your comment again. After I removed the additional clipboardReadWrite > permission, if we need to make sure onClipboardDataChanged only requires > clipboardRead permission and SetImageData only requires clipboardWrite, then the > permission requirement is refined to API level instead of chrome.clipboard > namespace level, do I need to change _api_features_.json file to reconfigure the > permission dependency?

Yes, if you want different permissions for different API methods, you need to adjust the _api_features.json file.

jennyz 2016/12/29 05:16:03 I made the change to adjust the permissions to API

On 2016/12/27 17:08:00, Devlin wrote: > On 2016/12/22 23:31:40, jennyz wrote: > > On 2016/12/22 22:45:20, jennyz wrote: > > > On 2016/12/22 16:07:49, Devlin wrote: > > > > On 2016/12/20 22:16:54, jennyz wrote: > > > > > On 2016/12/09 15:23:43, Devlin wrote: > > > > > > On 2016/12/07 01:21:48, jennyz wrote: > > > > > > > On 2016/11/16 02:34:33, Devlin wrote: > > > > > > > > This will disable this entire api for any existing extensions. > > > > > > > > > > > > > > This extension is a new one we added for Citrix remote app, so far > > they > > > > are > > > > > > the > > > > > > > only client who uses this API, no one has use this extension yet, so > > it > > > > > should > > > > > > > be ok to make change now. > > > > > > > > > > > > Where is this whitelist enforced? > > > > > > > > > > No, I didn't enforce the whitelist. According to your comment later, I > > > thought > > > > > it is OK for me to document the deprecation plan in the clipboard.idl, > > > instead > > > > > of enforcing the whitelist. > > > > > > > > But this API isn't *only* used for the new function being added; it's also > > > used > > > > for the onClipboardDataChanged event. Since the clipboard API is not > > > restricted > > > > to a whitelist, arbitrary apps and extensions can be relying on it. If we > > add > > > > an additional dependency, then any apps or extensions that don't specify > > that > > > > dependency will be unable to use it, and will suddenly break in this > chrome > > > > version. Additionally, there's no reason that an extension using the > > > > onClipboardDataChanged event requires clipboardWrite, and no reason that > > > setting > > > > the image data to the clipboard requires clipboard read. Right? > > > Good point. I have removed clipboardReadWrite permission in the new patch. > So > > > the app depending on onClipboardDataChanged will only need to require > > > clipboardRead permission. > > > > I read your comment again. After I removed the additional clipboardReadWrite > > permission, if we need to make sure onClipboardDataChanged only requires > > clipboardRead permission and SetImageData only requires clipboardWrite, then > the > > permission requirement is refined to API level instead of chrome.clipboard > > namespace level, do I need to change _api_features_.json file to reconfigure > the > > permission dependency? > > Yes, if you want different permissions for different API methods, you need to > adjust the _api_features.json file.

I made the change to adjust the permissions to API method level.

+ ],

"contexts": ["blessed_extension"]

"declarativeWebRequest": {