Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2951043002:
Prepare security bullets for Android: add issuer and change connection details. (Closed)
DescriptionPrepare security bullets for Android: add issuer and change connection details.
This is the first of a series of CLs to expose the GetSecurityStyle()
explanations (currently only used in the DevTools Security panel overview,
untranslated) in the Android connection info popup (translated). It does two
things:
- Mention the issuer in the certificate bullet (for feature parity with the old
Android connection info popup).
- Swap TLS connection settings values with their descriptions to avoid phrase
substitution within a clause (for straightforward translation).
The relevant strings retain `translateable="false"` until a future CL, due to
non-trivial runtime logic about when to translate the strings.
BUG=587255, 657299
TEST=Open the DevTools Security panel, visit https://google.com, and verify that there is a green bullet stating "The connection to this site is using a valid, trusted server certificate issued by Google Internet Authority G2."
Review-Url: https://codereview.chromium.org/2951043002
Cr-Commit-Position: refs/heads/master@{#481806}
Committed: https://chromium.googlesource.com/chromium/src/+/021de9b69af552ffb0daf1db0ee23bfad1917170
Patch Set 1 #Patch Set 2 #
Total comments: 2
Patch Set 3 : Update existing tests and add SecurityStateContentUtilsTest.ObsoleteConnectionExplanation #Patch Set 4 : Add that comment. #Patch Set 5 : Fix comma. #
Total comments: 1
Patch Set 6 : Typo #Patch Set 7 : Typo #
Messages
Total messages: 41 (30 generated)
Description was changed from ========== Prepare security bullets for Android: add issuer and change obsolete cipher message. In the near future, the security panel overview bullets will be translated shared with a new version of the connection info popup on Android. This change does two things to prepare us: - Mention the issuer in the certificate bullet (for feature parity with the old Android connection info poup). - Change the connection settings bullet to avoid phrase substitution within a clause (for straightforward translation). BUG=587255, 657299 ========== to ========== Prepare security bullets for Android: add issuer and change connection details. In the near future, the security panel overview bullets will be translated shared with a new version of the connection info popup on Android. This change does two things to prepare us: - Mention the issuer in the certificate bullet (for feature parity with the old Android connection info poup). - Change the connection details bullet to avoid phrase substitution within a clause (for straightforward translation). BUG=587255, 657299 ==========
Description was changed from ========== Prepare security bullets for Android: add issuer and change connection details. In the near future, the security panel overview bullets will be translated shared with a new version of the connection info popup on Android. This change does two things to prepare us: - Mention the issuer in the certificate bullet (for feature parity with the old Android connection info poup). - Change the connection details bullet to avoid phrase substitution within a clause (for straightforward translation). BUG=587255, 657299 ========== to ========== Prepare security bullets for Android: add issuer and change connection details. In the near future, the security panel overview bullets will be translated shared with a new version of the connection info popup on Android. This change does two things to prepare us: - Mention the issuer in the certificate bullet (for feature parity with the old Android connection info poup). - Swap TLS connection settings values with their descriptions to avoid phrase substitution within a clause (for straightforward translation). BUG=587255, 657299 ==========
Description was changed from ========== Prepare security bullets for Android: add issuer and change connection details. In the near future, the security panel overview bullets will be translated shared with a new version of the connection info popup on Android. This change does two things to prepare us: - Mention the issuer in the certificate bullet (for feature parity with the old Android connection info poup). - Swap TLS connection settings values with their descriptions to avoid phrase substitution within a clause (for straightforward translation). BUG=587255, 657299 ========== to ========== Prepare security bullets for Android: add issuer and change connection details. This is the first of a serious of CLs to expose the GetSecurityStyle() explanations (currently only used in DevTools, untranslated) in the Android connection info popup (translated). Does two things to prepare us: - Mention the issuer in the certificate bullet (for feature parity with the old Android connection info popup). - Swap TLS connection settings values with their descriptions to avoid phrase substitution within a clause (for straightforward translation). The relevant strings retain `translateable="false"` until a future CL, due to the non-trivial logic about when to translate the strings. BUG=587255, 657299 ==========
The CQ bit was checked by lgarron@chromium.org to run a CQ dry run
The CQ bit was unchecked by lgarron@chromium.org
The CQ bit was checked by lgarron@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...
lgarron@chromium.org changed reviewers: + estark@chromium.org
estark@, do these strings sound good to you? emilyschechter@: Should we consult Shimi?
Description was changed from ========== Prepare security bullets for Android: add issuer and change connection details. This is the first of a serious of CLs to expose the GetSecurityStyle() explanations (currently only used in DevTools, untranslated) in the Android connection info popup (translated). Does two things to prepare us: - Mention the issuer in the certificate bullet (for feature parity with the old Android connection info popup). - Swap TLS connection settings values with their descriptions to avoid phrase substitution within a clause (for straightforward translation). The relevant strings retain `translateable="false"` until a future CL, due to the non-trivial logic about when to translate the strings. BUG=587255, 657299 ========== to ========== Prepare security bullets for Android: add issuer and change connection details. This is the first of a series of CLs to expose the GetSecurityStyle() explanations (currently only used in the DevTools Security panel overview, untranslated) in the Android connection info popup (translated). It does two things: - Mention the issuer in the certificate bullet (for feature parity with the old Android connection info popup). - Swap TLS connection settings values with their descriptions to avoid phrase substitution within a clause (for straightforward translation). The relevant strings retain `translateable="false"` until a future CL, due to the non-trivial logic about when to translate the strings. BUG=587255, 657299 ==========
Description was changed from ========== Prepare security bullets for Android: add issuer and change connection details. This is the first of a series of CLs to expose the GetSecurityStyle() explanations (currently only used in the DevTools Security panel overview, untranslated) in the Android connection info popup (translated). It does two things: - Mention the issuer in the certificate bullet (for feature parity with the old Android connection info popup). - Swap TLS connection settings values with their descriptions to avoid phrase substitution within a clause (for straightforward translation). The relevant strings retain `translateable="false"` until a future CL, due to the non-trivial logic about when to translate the strings. BUG=587255, 657299 ========== to ========== Prepare security bullets for Android: add issuer and change connection details. This is the first of a series of CLs to expose the GetSecurityStyle() explanations (currently only used in the DevTools Security panel overview, untranslated) in the Android connection info popup (translated). It does two things: - Mention the issuer in the certificate bullet (for feature parity with the old Android connection info popup). - Swap TLS connection settings values with their descriptions to avoid phrase substitution within a clause (for straightforward translation). The relevant strings retain `translateable="false"` until a future CL, due to non-trivial logic about when to translate the strings. BUG=587255, 657299 ==========
Description was changed from ========== Prepare security bullets for Android: add issuer and change connection details. This is the first of a series of CLs to expose the GetSecurityStyle() explanations (currently only used in the DevTools Security panel overview, untranslated) in the Android connection info popup (translated). It does two things: - Mention the issuer in the certificate bullet (for feature parity with the old Android connection info popup). - Swap TLS connection settings values with their descriptions to avoid phrase substitution within a clause (for straightforward translation). The relevant strings retain `translateable="false"` until a future CL, due to non-trivial logic about when to translate the strings. BUG=587255, 657299 ========== to ========== Prepare security bullets for Android: add issuer and change connection details. This is the first of a series of CLs to expose the GetSecurityStyle() explanations (currently only used in the DevTools Security panel overview, untranslated) in the Android connection info popup (translated). It does two things: - Mention the issuer in the certificate bullet (for feature parity with the old Android connection info popup). - Swap TLS connection settings values with their descriptions to avoid phrase substitution within a clause (for straightforward translation). The relevant strings retain `translateable="false"` until a future CL, due to non-trivial runtime logic about when to translate the strings. BUG=587255, 657299 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
lg but looks like some tests are failing https://codereview.chromium.org/2951043002/diff/10003/components/security_sta... File components/security_state/content/content_utils.cc (right): https://codereview.chromium.org/2951043002/diff/10003/components/security_sta... components/security_state/content/content_utils.cc:310: l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_UNKNOWN_PARTY)); Took me a minute to understand why this can't just go in line 306; I'm assuming it's because GetDisplayName() can return an empty string? If so, maybe mention that in a comment above line 303.
On 2017/06/21 at 16:47:52, estark wrote: > lg but looks like some tests are failing > > https://codereview.chromium.org/2951043002/diff/10003/components/security_sta... > File components/security_state/content/content_utils.cc (right): > > https://codereview.chromium.org/2951043002/diff/10003/components/security_sta... > components/security_state/content/content_utils.cc:310: l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_UNKNOWN_PARTY)); > Took me a minute to understand why this can't just go in line 306; I'm assuming it's because GetDisplayName() can return an empty string? If so, maybe mention that in a comment above line 303. I'm doing the lazy thing where I get the commit queue to tel me everywhere strings use these tests. Will update now. :-P
estark@, could you review again? https://codereview.chromium.org/2951043002/diff/10003/components/security_sta... File components/security_state/content/content_utils.cc (right): https://codereview.chromium.org/2951043002/diff/10003/components/security_sta... components/security_state/content/content_utils.cc:310: l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_UNKNOWN_PARTY)); On 2017/06/21 at 16:47:51, estark wrote: > Took me a minute to understand why this can't just go in line 306; I'm assuming it's because GetDisplayName() can return an empty string? If so, maybe mention that in a comment above line 303. Will do.
The CQ bit was checked by lgarron@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 checked by estark@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: Try jobs failed on following builders: linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
On 2017/06/22 00:48:05, lgarron wrote: > estark@, could you review again? > > https://codereview.chromium.org/2951043002/diff/10003/components/security_sta... > File components/security_state/content/content_utils.cc (right): > > https://codereview.chromium.org/2951043002/diff/10003/components/security_sta... > components/security_state/content/content_utils.cc:310: > l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_UNKNOWN_PARTY)); > On 2017/06/21 at 16:47:51, estark wrote: > > Took me a minute to understand why this can't just go in line 306; I'm > assuming it's because GetDisplayName() can return an empty string? If so, maybe > mention that in a comment above line 303. > > Will do. (holding off on re-reviewing until the bots are green)
The CQ bit was checked by lgarron@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.
Bots are green! Sorry about that. :-(
lgtm https://codereview.chromium.org/2951043002/diff/70001/components/security_sta... File components/security_state/content/content_utils_unittest.cc (right): https://codereview.chromium.org/2951043002/diff/70001/components/security_sta... components/security_state/content/content_utils_unittest.cc:230: // Test that obsolete connection explanations are formated as expected. nit: formatted
The CQ bit was checked by lgarron@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from estark@chromium.org Link to the patchset: https://codereview.chromium.org/2951043002/#ps110001 (title: "Typo")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Prepare security bullets for Android: add issuer and change connection details. This is the first of a series of CLs to expose the GetSecurityStyle() explanations (currently only used in the DevTools Security panel overview, untranslated) in the Android connection info popup (translated). It does two things: - Mention the issuer in the certificate bullet (for feature parity with the old Android connection info popup). - Swap TLS connection settings values with their descriptions to avoid phrase substitution within a clause (for straightforward translation). The relevant strings retain `translateable="false"` until a future CL, due to non-trivial runtime logic about when to translate the strings. BUG=587255, 657299 ========== to ========== Prepare security bullets for Android: add issuer and change connection details. This is the first of a series of CLs to expose the GetSecurityStyle() explanations (currently only used in the DevTools Security panel overview, untranslated) in the Android connection info popup (translated). It does two things: - Mention the issuer in the certificate bullet (for feature parity with the old Android connection info popup). - Swap TLS connection settings values with their descriptions to avoid phrase substitution within a clause (for straightforward translation). The relevant strings retain `translateable="false"` until a future CL, due to non-trivial runtime logic about when to translate the strings. BUG=587255, 657299 TEST=Open the DevTools Security panel, visit https://google.com, and verify that there is a green bullet stating "The connection to this site is using a valid, trusted server certificate issued by Google Internet Authority G2." ==========
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by lgarron@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 110001, "attempt_start_ts": 1498189243302160,
"parent_rev": "f11d62080a80c7de948ec4c4d7e9c1673becb257", "commit_rev":
"021de9b69af552ffb0daf1db0ee23bfad1917170"}
Message was sent while issue was closed.
Description was changed from ========== Prepare security bullets for Android: add issuer and change connection details. This is the first of a series of CLs to expose the GetSecurityStyle() explanations (currently only used in the DevTools Security panel overview, untranslated) in the Android connection info popup (translated). It does two things: - Mention the issuer in the certificate bullet (for feature parity with the old Android connection info popup). - Swap TLS connection settings values with their descriptions to avoid phrase substitution within a clause (for straightforward translation). The relevant strings retain `translateable="false"` until a future CL, due to non-trivial runtime logic about when to translate the strings. BUG=587255, 657299 TEST=Open the DevTools Security panel, visit https://google.com, and verify that there is a green bullet stating "The connection to this site is using a valid, trusted server certificate issued by Google Internet Authority G2." ========== to ========== Prepare security bullets for Android: add issuer and change connection details. This is the first of a series of CLs to expose the GetSecurityStyle() explanations (currently only used in the DevTools Security panel overview, untranslated) in the Android connection info popup (translated). It does two things: - Mention the issuer in the certificate bullet (for feature parity with the old Android connection info popup). - Swap TLS connection settings values with their descriptions to avoid phrase substitution within a clause (for straightforward translation). The relevant strings retain `translateable="false"` until a future CL, due to non-trivial runtime logic about when to translate the strings. BUG=587255, 657299 TEST=Open the DevTools Security panel, visit https://google.com, and verify that there is a green bullet stating "The connection to this site is using a valid, trusted server certificate issued by Google Internet Authority G2." Review-Url: https://codereview.chromium.org/2951043002 Cr-Commit-Position: refs/heads/master@{#481806} Committed: https://chromium.googlesource.com/chromium/src/+/021de9b69af552ffb0daf1db0ee2... ==========
Message was sent while issue was closed.
Committed patchset #7 (id:110001) as https://chromium.googlesource.com/chromium/src/+/021de9b69af552ffb0daf1db0ee2... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
