When read and modify permissions for "accessibility" are requested or read and write permissions fo…

Combine the read and modify warning message for "accessibilitySettings" access.

When the extension/app requests to read and modify the accessibility settings, two messages are displayed to the user during install-time. The CL aims to combine these into one message to say "Read and modify ...".

BUG=344566
R=jww,meacer,kalman

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=278854

[mhm, 2014-06-18 00:10:05]

jww@ and meacer@ can you please have a look at this form me.

[jww, 2014-06-18 00:17:57]

lgtm

[meacer, 2014-06-18 02:19:57]

Nit: The first line of the CL description is generally a brief summary. You can
say "Combine permission messages for accessibilityRead and accessibilityWrite"
and then have the long explanation.

https://codereview.chromium.org/324273003/diff/1/chrome/common/extensions/per...
File chrome/common/extensions/permissions/chrome_permission_message_provider.cc
(right):

https://codereview.chromium.org/324273003/diff/1/chrome/common/extensions/per...
chrome/common/extensions/permissions/chrome_permission_message_provider.cc:142:
case PermissionMessage::kFileSystemDirectory:
fileSystem.write already suppresses fileSystem:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/common/exte...

[mhm, 2014-06-18 17:02:16]

meacer@ thanks for catching that. Can you look at the updated CL for me.

[meacer, 2014-06-18 17:08:18]

https://codereview.chromium.org/324273003/diff/20001/chrome/app/generated_res...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/324273003/diff/20001/chrome/app/generated_res...
chrome/app/generated_resources.grd:4369: +        Read and write to files and
folders that you open in the application
Shouldn't need this anymore, right?

https://codereview.chromium.org/324273003/diff/20001/chrome/app/generated_res...
chrome/app/generated_resources.grd:4420: +        Read and change your
accessibility settings
Random rant :) The "read" and "write" strings are all different among warnings.
So far we have:

Read and write
Read and modify
Read and change
Access and modify
Access and change

Would be good if we could make this consistent, but not a problem for this CL.

https://codereview.chromium.org/324273003/diff/20001/chrome/common/extensions...
File chrome/common/extensions/permissions/chrome_permission_message_provider.cc
(right):

https://codereview.chromium.org/324273003/diff/20001/chrome/common/extensions...
chrome/common/extensions/permissions/chrome_permission_message_provider.cc:116:
bool file_system_read = false;
file_system_read and file_system_write not needed anymore

https://codereview.chromium.org/324273003/diff/20001/chrome/common/extensions...
chrome/common/extensions/permissions/chrome_permission_message_provider.cc:163:
IDS_EXTENSION_PROMPT_WARNING_ACCESSIBILITY_FEATURES_READ_AND_MODIFY));
Line width over 80 characters

[mhm, 2014-06-18 17:33:58]

meacer@ apologies for the stupid mistakes. I cleaned it up.

https://codereview.chromium.org/324273003/diff/20001/chrome/app/generated_res...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/324273003/diff/20001/chrome/app/generated_res...
chrome/app/generated_resources.grd:4369: +        Read and write to files and
folders that you open in the application
Oops! I removed the wrong one.

https://codereview.chromium.org/324273003/diff/20001/chrome/app/generated_res...
chrome/app/generated_resources.grd:4420: +        Read and change your
accessibility settings
filed bug 386224 for it.

[meacer, 2014-06-18 17:39:53]

No worries at all :) Lgtm with a nit.

https://codereview.chromium.org/324273003/diff/40001/chrome/common/extensions...
File chrome/common/extensions/permissions/chrome_permission_message_provider.cc
(right):

https://codereview.chromium.org/324273003/diff/40001/chrome/common/extensions...
chrome/common/extensions/permissions/chrome_permission_message_provider.cc:161:
IDS_EXTENSION_PROMPT_WARNING_ACCESSIBILITY_FEATURES_READ_AND_MODIFY));
Should be indented two more spaces, but then you'll hit 80 char limit again.
Maybe you can rename the id to
IDS_EXTENSION_PROMPT_WARNING_ACCESSIBILITY_FEATURES_READ_MODIFY or something
like that?

[mhm, 2014-06-18 19:32:29]

kalman@ can you please have a look at this for me.

[not at google - send to devlin, 2014-06-19 20:01:03]

lgtm

https://codereview.chromium.org/324273003/diff/60001/chrome/common/extensions...
File chrome/common/extensions/permissions/chrome_permission_message_provider.cc
(right):

https://codereview.chromium.org/324273003/diff/60001/chrome/common/extensions...
chrome/common/extensions/permissions/chrome_permission_message_provider.cc:141:
break;
we can replace o lines 109-143 by putting these permission IDs into a set. it's
not going to be a big set. it doesn't have to be in this CL but could you do it
at some point?

[mhm, 2014-06-20 17:16:40]

The CQ bit was checked by mohammed@chromium.org

[commit-bot: I haz the power, 2014-06-20 17:21:07]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mohammed@chromium.org/324273003/60001

[commit-bot: I haz the power, 2014-06-20 20:54:39]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-06-20 23:22:16]

Change committed as 278854