What about the use case of a user manually triggering the translation from the context menu?

That's good point. Even if the feature is disabled, we still provide the context menu entry point to translate the page.

This should not be good if the feature is disabled via PolicyList.
http://dev.chromium.org/administrators/policy-list-3#TranslateEnabled

Also one aside: the new material design settings still have per language translation on/off settings AFAIK based on what I see in Canary 59.0.3071.0 (and they should). Not sure if you were saying that they don't above: "But new material designed settings doe not have it today." 

If there is no reason that users who have translation disabled by administrator may need that list of languages, then agreed: we should not make this request. 

Oops, I failed to find the interface, but now I confirmed that the per language settings are still there.

chrome://translate-internals also downloads the list on the setting disabled.

FYI, the background of this discussion is a codereview of https://codereview.chromium.org/2796323002/

Probably, it would be ok to make a network request if it does not send user data, but we need to clarify it in the annotation. But probably it is not also difficult to stop updating the language list if the feature is disabled.

I think my preference would be with the first solution (continue to make the network request if no user data is sent but add annotation) just in case there is any other reason for the network request that we're missing. 

I see, it sounds reasonable.
We will update the annotation to explain it.

Also, I will land a small patch to fix the problem of #1.

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

commit c64ccfd0e457a5b54ac57ed095064b0f0bd59203
Author: toyoshim <toyoshim@chromium.org>
Date: Tue Apr 18 08:34:21 2017

Translate: do not show the context menu entry when the feature is disabled

Currently, Chrome has a context menu entry to translate the page
even when the translate feature is disabled by PolicyList.
But, it should be better to stop adding the entry when the feature
is disabled.

BUG= 711217 
TBR=sky@chromium.org, michaeldo@chromium.org

Review-Url: https://codereview.chromium.org/2819793002
Cr-Commit-Position: refs/heads/master@{#465177}

[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/chrome/browser/android/preferences/pref_service_bridge.cc
[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/chrome/browser/extensions/api/preference/preference_api.cc
[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/chrome/browser/extensions/api/preference/preference_apitest.cc
[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/chrome/browser/extensions/api/settings_private/prefs_util.cc
[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/chrome/browser/policy/configuration_policy_handler_list_factory.cc
[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/chrome/browser/renderer_context_menu/render_view_context_menu.cc
[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/chrome/browser/translate/translate_manager_render_view_host_unittest.cc
[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/chrome/browser/ui/browser_ui_prefs.cc
[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/chrome/browser/ui/webui/translate_internals/translate_internals_handler.cc
[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/components/translate/core/browser/BUILD.gn
[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/components/translate/core/browser/translate_download_manager.cc
[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/components/translate/core/browser/translate_manager.cc
[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/components/translate/core/browser/translate_manager_unittest.cc
[rename] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/components/translate/core/browser/translate_pref_names.cc
[add] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/components/translate/core/browser/translate_pref_names.h
[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/components/translate/core/browser/translate_prefs.cc
[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/components/translate/core/browser/translate_prefs.h
[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/components/translate/core/common/BUILD.gn
[delete] https://crrev.com/71abad621416fd3cbb69ebb2dae0d90b84be0287/components/translate/core/common/translate_pref_names.h
[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/components/translate/ios/browser/BUILD.gn
[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/components/translate/ios/browser/language_detection_controller.mm
[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/components/translate/ios/browser/language_detection_controller_unittest.mm
[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/ios/chrome/browser/prefs/browser_prefs.mm
[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/ios/chrome/browser/translate/translate_egtest.mm
[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/ios/chrome/browser/ui/settings/content_settings_collection_view_controller.mm
[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/ios/chrome/browser/ui/settings/translate_collection_view_controller.mm
[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/ios/chrome/browser/ui/settings/translate_collection_view_controller_unittest.mm
[modify] https://crrev.com/c64ccfd0e457a5b54ac57ed095064b0f0bd59203/ios/web_view/internal/web_view_browser_state.mm

Hey toyoshim@, the description for the patch is a little vague. I want to confirm that we're only not showing "Translate to.." in the right-click context menu when Translation is disabled by PolicyList. If it's disabled by the user (which means the user asked to not have us OFFER translations in chrome://settings/language), we should still be showing the context menu as this is why it's there: for users who don't want to be bothered by frequent translation prompts but want to be able to prompt a translation when they want it.

Sorry, my change disables the context menu translation even when a user disable Translation in the setting. This is because current PolicyList implementation just overrides the setting.

If it's still major use case, I need to consider to differentiate PolicyList banning case from disabled setting case.

Set the M-60 label so that I decide to revert, modify, or keep this change before this change goes to a release branch.

Hey toyoshim@ - that is an important use case. This allows users to opt-out of Translate prompts that may be annoying to them while being able to manually trigger translations through the context menu. Please revert then.

ok, I will revert the change now.
https://codereview.chromium.org/2872813004/

+rhalavati for further discussion.

Today, the PolicyList for translate only works to disable the auto page scan for language detection and prompting. But it still allows for users to invoke a translation manually from the context menu.

Do you think it's important to disable the context menu trigger too when the PolicyList is set? (We didn't disable it, so I think anyway this is not urgent though)

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

commit a942138fe716b963ce8247cdcbd3082e841fd19c
Author: toyoshim <toyoshim@chromium.org>
Date: Wed May 10 05:34:52 2017

Revert a change of disabling translation from context menu temporarily

The translation should be disabled if the feature is disabled by
PolicyList, but the submitted change disables the translation even when
a user disables it from the setting.

Manually triggering a translation from the context menu is one important
use case for users who do not want Chrome to suggest translation, but
need manual translation.

I quickly revert the last change, and will revisit later to remove the
menu only when the feature is disabled by PolicyList.

BUG= 711217 

Review-Url: https://codereview.chromium.org/2872813004
Cr-Commit-Position: refs/heads/master@{#470479}

[modify] https://crrev.com/a942138fe716b963ce8247cdcbd3082e841fd19c/chrome/browser/renderer_context_menu/render_view_context_menu.cc

+pastarmovj@

The "TranslateEnabled" policy says that "If you enable this setting, Google Chrome will show an integrated toolbar offering to translate the page for the user, when appropriate.", so it seems that at least directly it is not talking about translation possibility by context menu. But the question is, if an enterprise admin wishes to ban translation by Google, shouldn't the policy cover this case as well? I think this needs policy clarification.

yes this logic sounds about right. 

Adding blumberg for opinion.

Agreed, translation via context menu should not be possible/allowed if TranslateEnabled is disabled.
 

The policy language is not entirely clear here. If I were an admin I may disable TranslateEnabled thinking: "We don't want to spam our users with translation prompts" rather than that they want to ban all translation by Google.

+1 for clarifying policy description. I think both interpretations are possible.

Definitely agreed with #18, the policy should cover all built-in translation UI. We can clarify that in the policy description. For EDU we need admins to be able to block any kind of built-in translation, not just from the toolbar.

[modified]

IMO, what admin really want to decide here is not to disallow prompting, but to disallow translate feature because the former is a matter of users' UI preference, and the latter could be an operational policy.

On the other hand, I completely agreed that some users want to use translation only from the context menu without any prompting. So, we should not change anything about what the setting disallows.

So, what we can do here is

Plan A) Clarify the description of TranslateEnabled policy to disallow all built-in translation features by the policy. But Translate setting still allows to use translation from the context menu as it does today.

Plan B) Add another policy that disallows all built-in translation features, and keep the existing policy and setting as is, that means TranslateEnabled policy and setting disallows only prompting, and the new policy disable all translate features separately.

We should avoid adding new policies if at all possible.  Something like this policy we should just expand to also block the context menu.

Whatever we decide to do with the policy but, let's just make sure that we decouple disabled by policy and disabled by preference so that the second category is not affected.

Agreed with c24 and c25.

So, probably we want to change the TranslateEnabled policy description to say the policy disable all built-in translation features explicitly, then disable context menu feature only when the feature is disabled by the policy, but it should still work when the feature is just disabled by the preference.

Does anyone know the (launch) process to change the policy description?

The launch process is pretty simple :)

Fix the description in https://cs.chromium.org/chromium/src/components/policy/resources/policy_templates.json?q=policy_templates.json and send a CL to some of the owners (me :) ) for review. Profit.

yyushkina: could you find another owner who can start working on this soon?

Jon - let's talk about who can work on this in our 1:1. Given feedback from Enterprise team, this is important. What we need to do is disable context menu translations when, and only when, they are disabled by policy (but not when they are disabled by user preference) and then update policy language (I can help with that part).

 Issue 782059  has been merged into this issue.

Question for pastarmovj@

Hi Julian,
What's the state of policy on iOS? From what I can see, some relevant code exists but is never called:
https://cs.chromium.org/chromium/src/components/policy/core/browser/browser_policy_connector_ios.h

At some point we wanted to implement cloud policy on mobile the same way it is done on desktop platforms and chromeos (through the signed in user in the browser) however we soon realized that on mobile platforms the right path forward is through third party MDM providers. I know that this is already the state on Android but don't quite know the current stae of iOS. I think it might have been left out for now or use some different MDM channels.

Thanks for the context Julian.

Yana, I have a product question that's come up while examining current translate logic:

Unchecking the "Offer to translate pages that aren't in a language you read" setting also disables the user's auto-translate preferences (which isn't suggested by the setting text).

Clearly this is the correct behaviour for policy-disabled translate, but should we change the logic to always perform auto-translate when it's allowed by policy?

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

commit 1f41a382402d5d81d1034e477c37318ff31a5f14
Author: Michael Martis <martis@chromium.org>
Date: Tue Nov 21 04:33:57 2017

Removed right-click translate when translate is disabled by policy

Right-click translate is still available whenever translate is enabled by policy
(i.e. even when the "offer translate" setting is disabled).

This is implemented as follows:
  - Renaming the existing translate preference (and associated methods) to "offer
    translate", to match its settings page description / behaviour.
  - Gating right-click translate on the translate policy (i.e. allow right-click
    translate only when the "offer translate" pref is unmanaged or enabled).

Bug:  711217 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I1aacdeb4705941319c46f0c681ebaa3773330cad
Reviewed-on: https://chromium-review.googlesource.com/776173
Commit-Queue: Michael Martis <martis@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Eric Noyau <noyau@chromium.org>
Reviewed-by: Renjie Liu <renjieliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518114}
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/chrome/browser/android/preferences/pref_service_bridge.cc
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/chrome/browser/extensions/api/preference/preference_api.cc
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/chrome/browser/extensions/api/preference/preference_apitest.cc
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/chrome/browser/extensions/api/settings_private/prefs_util.cc
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/chrome/browser/policy/configuration_policy_handler_list_factory.cc
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/chrome/browser/renderer_context_menu/render_view_context_menu.cc
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/chrome/browser/translate/translate_manager_render_view_host_unittest.cc
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/chrome/browser/ui/browser_ui_prefs.cc
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/chrome/browser/ui/webui/translate_internals/translate_internals_handler.cc
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/components/translate/core/browser/translate_download_manager.cc
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/components/translate/core/browser/translate_download_manager.h
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/components/translate/core/browser/translate_manager.cc
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/components/translate/core/browser/translate_manager_unittest.cc
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/components/translate/core/browser/translate_pref_names.cc
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/components/translate/core/browser/translate_pref_names.h
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/components/translate/core/browser/translate_prefs.cc
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/components/translate/core/browser/translate_prefs.h
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/components/translate/ios/browser/language_detection_controller.mm
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/components/translate/ios/browser/language_detection_controller_unittest.mm
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/ios/chrome/browser/prefs/browser_prefs.mm
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/ios/chrome/browser/translate/translate_egtest.mm
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/ios/chrome/browser/ui/settings/content_settings_collection_view_controller.mm
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/ios/chrome/browser/ui/settings/translate_collection_view_controller.mm
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/ios/chrome/browser/ui/settings/translate_collection_view_controller_unittest.mm
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/ios/web_view/internal/cwv_preferences.mm
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/ios/web_view/internal/cwv_preferences_unittest.mm
[modify] https://crrev.com/1f41a382402d5d81d1034e477c37318ff31a5f14/ios/web_view/internal/web_view_browser_state.mm

Let's think through this together. Outside of the policy case, the toggle "Offer to translate pages that aren't in a language you read" is in theory there to enable users who never want translations to get rid of dialogues they find potentially annoying. I suppose it's possible that a user may still want the languages they chose to auto-translate to be translated while not seeing any other Translate prompts but that seems an odd use case to me. I'd leave things as is for now and monitor bugs/feedbacks to see if my intuition is not correct on this.

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

commit 42d4673067cc41ed6a88a99e36727d056cdeece6
Author: Michael Martis <martis@chromium.org>
Date: Thu Nov 23 04:43:14 2017

Disabled language list fetch when translate is disabled by policy.

Bug:  711217 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I40a338f4d47a93b8789589f4684b55ce2e0c2c6e
Reviewed-on: https://chromium-review.googlesource.com/775547
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Commit-Queue: Michael Martis <martis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518849}
[modify] https://crrev.com/42d4673067cc41ed6a88a99e36727d056cdeece6/chrome/browser/extensions/api/language_settings_private/language_settings_private_api.cc
[modify] https://crrev.com/42d4673067cc41ed6a88a99e36727d056cdeece6/chrome/browser/extensions/api/language_settings_private/language_settings_private_api.h
[modify] https://crrev.com/42d4673067cc41ed6a88a99e36727d056cdeece6/chrome/browser/translate/translate_manager_render_view_host_unittest.cc
[modify] https://crrev.com/42d4673067cc41ed6a88a99e36727d056cdeece6/chrome/browser/ui/webui/translate_internals/translate_internals_handler.cc
[modify] https://crrev.com/42d4673067cc41ed6a88a99e36727d056cdeece6/components/translate/core/browser/translate_download_manager.cc
[modify] https://crrev.com/42d4673067cc41ed6a88a99e36727d056cdeece6/components/translate/core/browser/translate_download_manager.h
[modify] https://crrev.com/42d4673067cc41ed6a88a99e36727d056cdeece6/components/translate/core/browser/translate_language_list.cc
[modify] https://crrev.com/42d4673067cc41ed6a88a99e36727d056cdeece6/components/translate/core/browser/translate_language_list.h
[modify] https://crrev.com/42d4673067cc41ed6a88a99e36727d056cdeece6/components/translate/core/browser/translate_language_list_unittest.cc
[modify] https://crrev.com/42d4673067cc41ed6a88a99e36727d056cdeece6/components/translate/core/browser/translate_prefs.cc
[modify] https://crrev.com/42d4673067cc41ed6a88a99e36727d056cdeece6/components/translate/core/browser/translate_prefs.h
[modify] https://crrev.com/42d4673067cc41ed6a88a99e36727d056cdeece6/components/translate/core/browser/translate_prefs_unittest.cc
[modify] https://crrev.com/42d4673067cc41ed6a88a99e36727d056cdeece6/components/translate/core/browser/translate_ui_delegate.cc
[modify] https://crrev.com/42d4673067cc41ed6a88a99e36727d056cdeece6/components/translate/core/browser/translate_ui_delegate_unittest.cc
[modify] https://crrev.com/42d4673067cc41ed6a88a99e36727d056cdeece6/ios/web_view/internal/translate/cwv_translation_controller.mm

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

commit 91c3a9f37116eb7e08eaa9e78ce490f89b29ad8e
Author: Michael Martis <martis@chromium.org>
Date: Fri Nov 24 04:42:02 2017

Updated translate policy description

Bug:  711217 
Change-Id: I0da4a67212f25fdf2a2faf53ca32b79398bbd92d
Reviewed-on: https://chromium-review.googlesource.com/778582
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Commit-Queue: Michael Martis <martis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519058}
[modify] https://crrev.com/91c3a9f37116eb7e08eaa9e78ce490f89b29ad8e/components/policy/resources/policy_templates.json

 Issue 557702  has been merged into this issue.

Hi,

Sorry to reopen a closed bug, but it seems to me that the topic of the bug and the final fix do not match.
Aren't we going to stop loading the language list if translation is disabled? It is true that no user data is sent, but we are doing something unnecessary and some may argue that even IP is considered user data and should not be sent to Google if not required.

As explained above, we have to download the language list to allow for manually triggered translations. The user-controlled Translate on/off setting is only for the auto-triggered UI (which is evident in its wording) and is there to avoid Translate UI pop-ups for users who do not want them while still allowing them the option to trigger a translation when needed. 

Thank you, sorry I missed it.

 Issue 777855  has been merged into this issue.