Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2880603002:
Strings for new Chrome Cleanup prompt and Web UI (Closed)
DescriptionStrings for new Chrome Cleanup prompt and Web UI
BUG=690020
Review-Url: https://codereview.chromium.org/2880603002
Cr-Commit-Position: refs/heads/master@{#471344}
Committed: https://chromium.googlesource.com/chromium/src/+/97a209baff986a56009d28eae1bf6f9d20cde0b4
Patch Set 1 #Patch Set 2 : is_win for Chromium/Chrome strings #Patch Set 3 : Strings reviewed by UX #
Total comments: 8
Patch Set 4 : Addressed alito@'s comments #
Total comments: 4
Patch Set 5 : Addressed csharp@'s comments #
Messages
Total messages: 21 (13 generated)
The CQ bit was checked by ftirelo@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
ftirelo@chromium.org changed reviewers: + alito@chromium.org, csharp@chromium.org, srahim@chromium.org - ftirelo@google.com
Strings proposed by srahim@
https://codereview.chromium.org/2880603002/diff/40001/chrome/app/chromium_str... File chrome/app/chromium_strings.grd (right): https://codereview.chromium.org/2880603002/diff/40001/chrome/app/chromium_str... chrome/app/chromium_strings.grd:777: Chromium found harmful software on your computer. Chromium can remove it and restore your settings to make your browser work normally again. Is it possible that if we have two separate entries for this text that we might get two slightly different texts for the same language when they are translated? https://codereview.chromium.org/2880603002/diff/40001/chrome/app/chromium_str... chrome/app/chromium_strings.grd:779: <message name="IDS_CHROME_CLEANUP_WEBUI_EXPLANATION" desc="Description in the Chrome Cleanup dialog that Chrome browser shows when unwanted software, like ad injectors or software that changes the user's settings without their knowledge, is found on the user's computer. Appears under the title asking 'Remove harmful software?' Actor is Chrome; we are asking, Do you want Chrome to remove harmful software? 'it' is harmful software. User has the option of clicking 'Remove' to proceed with a cleanup, or 'Details' to see more details. "> The description for this second string still refers to the dialog and not the webui page. https://codereview.chromium.org/2880603002/diff/40001/chrome/app/chromium_str... chrome/app/chromium_strings.grd:779: <message name="IDS_CHROME_CLEANUP_WEBUI_EXPLANATION" desc="Description in the Chrome Cleanup dialog that Chrome browser shows when unwanted software, like ad injectors or software that changes the user's settings without their knowledge, is found on the user's computer. Appears under the title asking 'Remove harmful software?' Actor is Chrome; we are asking, Do you want Chrome to remove harmful software? 'it' is harmful software. User has the option of clicking 'Remove' to proceed with a cleanup, or 'Details' to see more details. "> tiny nit: remove space after the final period in the description. https://codereview.chromium.org/2880603002/diff/40001/chrome/app/generated_re... File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/2880603002/diff/40001/chrome/app/generated_re... chrome/app/generated_resources.grd:6393: <message name="IDS_CHROME_CLEANUP_PROMPT_REMOVE_BUTTON_LABEL" desc="A button in the Chrome Cleanup dialog. When clicked, Chrome will begin to remove unwanted software and restore browser settings to default values. Appears next to a 'Cancel' button and a 'Details' button that opens a page that shows which unwanted software will be deleted. 'Remove' is imperative."> tiny nit: there is double space in the description after "Chrome will begin to remove"
The CQ bit was checked by ftirelo@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
PTAL https://codereview.chromium.org/2880603002/diff/40001/chrome/app/chromium_str... File chrome/app/chromium_strings.grd (right): https://codereview.chromium.org/2880603002/diff/40001/chrome/app/chromium_str... chrome/app/chromium_strings.grd:777: Chromium found harmful software on your computer. Chromium can remove it and restore your settings to make your browser work normally again. On 2017/05/11 23:22:12, alito wrote: > Is it possible that if we have two separate entries for this text that we might > get two slightly different texts for the same language when they are translated? Added a note saying that preferably translations should match. I don't want to require them to be the same, since different languages/cultures may have different expectations for each text. https://codereview.chromium.org/2880603002/diff/40001/chrome/app/chromium_str... chrome/app/chromium_strings.grd:779: <message name="IDS_CHROME_CLEANUP_WEBUI_EXPLANATION" desc="Description in the Chrome Cleanup dialog that Chrome browser shows when unwanted software, like ad injectors or software that changes the user's settings without their knowledge, is found on the user's computer. Appears under the title asking 'Remove harmful software?' Actor is Chrome; we are asking, Do you want Chrome to remove harmful software? 'it' is harmful software. User has the option of clicking 'Remove' to proceed with a cleanup, or 'Details' to see more details. "> On 2017/05/11 23:22:12, alito wrote: > The description for this second string still refers to the dialog and not the > webui page. Good catch! Thanks. https://codereview.chromium.org/2880603002/diff/40001/chrome/app/chromium_str... chrome/app/chromium_strings.grd:779: <message name="IDS_CHROME_CLEANUP_WEBUI_EXPLANATION" desc="Description in the Chrome Cleanup dialog that Chrome browser shows when unwanted software, like ad injectors or software that changes the user's settings without their knowledge, is found on the user's computer. Appears under the title asking 'Remove harmful software?' Actor is Chrome; we are asking, Do you want Chrome to remove harmful software? 'it' is harmful software. User has the option of clicking 'Remove' to proceed with a cleanup, or 'Details' to see more details. "> On 2017/05/11 23:22:12, alito wrote: > tiny nit: remove space after the final period in the description. Done. https://codereview.chromium.org/2880603002/diff/40001/chrome/app/generated_re... File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/2880603002/diff/40001/chrome/app/generated_re... chrome/app/generated_resources.grd:6393: <message name="IDS_CHROME_CLEANUP_PROMPT_REMOVE_BUTTON_LABEL" desc="A button in the Chrome Cleanup dialog. When clicked, Chrome will begin to remove unwanted software and restore browser settings to default values. Appears next to a 'Cancel' button and a 'Details' button that opens a page that shows which unwanted software will be deleted. 'Remove' is imperative."> On 2017/05/11 23:22:12, alito wrote: > tiny nit: there is double space in the description after "Chrome will begin to > remove" Done.
lgtm https://codereview.chromium.org/2880603002/diff/60001/chrome/app/chromium_str... File chrome/app/chromium_strings.grd (right): https://codereview.chromium.org/2880603002/diff/60001/chrome/app/chromium_str... chrome/app/chromium_strings.grd:767: <!-- SRT bubble messages --> I don't think this section, or the new section, should be added, since they aren't used by the checked in code. https://codereview.chromium.org/2880603002/diff/60001/chrome/app/chromium_str... chrome/app/chromium_strings.grd:767: <!-- SRT bubble messages --> Probably worth adding a comment here saying that this is the old section and should be removed in the future since it is for the old version
https://codereview.chromium.org/2880603002/diff/60001/chrome/app/chromium_str... File chrome/app/chromium_strings.grd (right): https://codereview.chromium.org/2880603002/diff/60001/chrome/app/chromium_str... chrome/app/chromium_strings.grd:767: <!-- SRT bubble messages --> On 2017/05/12 14:39:55, csharp wrote: > Probably worth adding a comment here saying that this is the old section and > should be removed in the future since it is for the old version Done. https://codereview.chromium.org/2880603002/diff/60001/chrome/app/chromium_str... chrome/app/chromium_strings.grd:767: <!-- SRT bubble messages --> On 2017/05/12 14:39:55, csharp wrote: > I don't think this section, or the new section, should be added, since they > aren't used by the checked in code. As discussed offline, removing them brings up an odd hash collision error. Added a comment for posterity.
The CQ bit was checked by ftirelo@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from csharp@chromium.org Link to the patchset: https://codereview.chromium.org/2880603002/#ps80001 (title: "Addressed csharp@'s comments")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1494603147130410,
"parent_rev": "78c073d7b247c65931466623b9fbed4a38bbedeb", "commit_rev":
"97a209baff986a56009d28eae1bf6f9d20cde0b4"}
Message was sent while issue was closed.
Description was changed from ========== Strings for new Chrome Cleanup prompt and Web UI BUG=690020 ========== to ========== Strings for new Chrome Cleanup prompt and Web UI BUG=690020 Review-Url: https://codereview.chromium.org/2880603002 Cr-Commit-Position: refs/heads/master@{#471344} Committed: https://chromium.googlesource.com/chromium/src/+/97a209baff986a56009d28eae1bf... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/97a209baff986a56009d28eae1bf... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
