Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(12)

Issue 2951043002: Prepare security bullets for Android: add issuer and change connection details. (Closed)

Created:
5 months ago by lgarron
Modified:
5 months ago
Reviewers:
estark
CC:
chromium-reviews, darin-cc_chromium.org, emilyschechter, jam
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

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/+/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 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+73 lines, -25 lines) Patch
M chrome/browser/ssl/security_state_tab_helper_browser_tests.cc View 1 2 3 4 3 chunks +12 lines, -7 lines 0 comments Download
M components/security_state/content/content_utils.cc View 1 2 3 2 chunks +19 lines, -5 lines 0 comments Download
M components/security_state/content/content_utils_unittest.cc View 1 2 3 4 5 6 4 chunks +39 lines, -10 lines 0 comments Download
M components/security_state_strings.grdp View 2 chunks +3 lines, -3 lines 0 comments Download

Messages

Total messages: 41 (30 generated)
lgarron
estark@, do these strings sound good to you? emilyschechter@: Should we consult Shimi?
5 months ago (2017-06-21 04:18:38 UTC) #9
estark
lg but looks like some tests are failing https://codereview.chromium.org/2951043002/diff/10003/components/security_state/content/content_utils.cc File components/security_state/content/content_utils.cc (right): https://codereview.chromium.org/2951043002/diff/10003/components/security_state/content/content_utils.cc#newcode310 components/security_state/content/content_utils.cc:310: l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_UNKNOWN_PARTY)); ...
5 months ago (2017-06-21 16:47:52 UTC) #15
lgarron
On 2017/06/21 at 16:47:52, estark wrote: > lg but looks like some tests are failing ...
5 months ago (2017-06-21 20:05:21 UTC) #16
lgarron
estark@, could you review again? https://codereview.chromium.org/2951043002/diff/10003/components/security_state/content/content_utils.cc File components/security_state/content/content_utils.cc (right): https://codereview.chromium.org/2951043002/diff/10003/components/security_state/content/content_utils.cc#newcode310 components/security_state/content/content_utils.cc:310: l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_UNKNOWN_PARTY)); On 2017/06/21 at ...
5 months ago (2017-06-22 00:48:05 UTC) #17
estark
On 2017/06/22 00:48:05, lgarron wrote: > estark@, could you review again? > > https://codereview.chromium.org/2951043002/diff/10003/components/security_state/content/content_utils.cc > ...
5 months ago (2017-06-22 15:24:56 UTC) #24
lgarron
Bots are green! Sorry about that. :-(
5 months ago (2017-06-22 20:27:03 UTC) #29
estark
lgtm https://codereview.chromium.org/2951043002/diff/70001/components/security_state/content/content_utils_unittest.cc File components/security_state/content/content_utils_unittest.cc (right): https://codereview.chromium.org/2951043002/diff/70001/components/security_state/content/content_utils_unittest.cc#newcode230 components/security_state/content/content_utils_unittest.cc:230: // Test that obsolete connection explanations are formated ...
5 months ago (2017-06-23 01:25:20 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2951043002/110001
5 months ago (2017-06-23 01:38:46 UTC) #33
commit-bot: I haz the power
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_rel_ng/builds/455980)
5 months ago (2017-06-23 03:31:01 UTC) #36
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2951043002/110001
5 months ago (2017-06-23 03:40:50 UTC) #38
commit-bot: I haz the power
5 months ago (2017-06-23 05:02:47 UTC) #41
Message was sent while issue was closed.
Committed patchset #7 (id:110001) as
https://chromium.googlesource.com/chromium/src/+/021de9b69af552ffb0daf1db0ee2...

Powered by Google App Engine
This is Rietveld efc10ee0f