Issue metadata
Sign in to add a comment
|
Replace `Android Page Info > Details` with Security panel bullet points |
||||||||||||||||||||||
Issue descriptionDesign doc: https://docs.google.com/document/d/1oYEKqoL-tKC1EwOmoYrT5TCBttQ9eCK9q2TfyC8uneA/edit#heading=h.au203bjfkch0 Android Page Info has a "Details" link that goes to a popup view based on the old desktop connection tab (which is gone since April: Issue 421248 ). The Android implementation requires us to leave a bunch of technical debt lying around in the WebsiteSettings code. This has gotten in the way of seemingly unrelated feature work, like Issue 646201 (Implement colored security summary). In addition, the Android strings are also unmaintained, and the popup even still uses the old Material/mobile icons (we've done a good job of keeping it intact!). The Security panel overview bullets do a much better job of breaking down and explaining the security state. In addition, we have done a good job of keeping them very simple [1]: - A summary string - A description string. - A boolean describing whether to show a button that opens the certificate viewer for the current page. So, I think that we should show these on Android instead. However, there are two complications: - The description of mixed content is synthesized in DevTools. However, we can probably figure out a simplified explanation to show instead. - The strings are not localized, and the TLS connection description is assembled using a template that assumes English grammar. However, I can think of some pretty easy ways to adapt our implementation for localization. estark@, do you foresee any additional problems? [1] https://cs.chromium.org/chromium/src/content/public/browser/security_style_explanation.h?q=struct+SecurityStyleExplanation&sq=package:chromium&l=19&dr=CSs ⛆ |
|
|
,
Jan 31 2017
,
May 12 2017
Design doc: https://docs.google.com/document/d/1oYEKqoL-tKC1EwOmoYrT5TCBttQ9eCK9q2TfyC8uneA/edit#heading=h.au203bjfkch0 Attached mocks are by me, and not sanctioned in any way by UI review. :-P
,
May 27 2017
I had many Android building issues this week, but the basic plumbing turned out to be as straightforward as I was hoping. Screenshots of two bullet points attached.
,
Jun 5 2017
,
Jun 7 2017
,
Jun 20 2017
,
Jun 23 2017
,
Jun 23 2017
,
Jun 23 2017
,
Jun 23 2017
,
Jun 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/021de9b69af552ffb0daf1db0ee23bfad1917170 commit 021de9b69af552ffb0daf1db0ee23bfad1917170 Author: lgarron <lgarron@chromium.org> Date: Fri Jun 23 05:02:31 2017 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} [modify] https://crrev.com/021de9b69af552ffb0daf1db0ee23bfad1917170/chrome/browser/ssl/security_state_tab_helper_browser_tests.cc [modify] https://crrev.com/021de9b69af552ffb0daf1db0ee23bfad1917170/components/security_state/content/content_utils.cc [modify] https://crrev.com/021de9b69af552ffb0daf1db0ee23bfad1917170/components/security_state/content/content_utils_unittest.cc [modify] https://crrev.com/021de9b69af552ffb0daf1db0ee23bfad1917170/components/security_state_strings.grdp
,
Jul 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/68fe95883c015e32cdc96f107a2442630b14595c commit 68fe95883c015e32cdc96f107a2442630b14595c Author: Lucas Garron <lgarron@chromium.org> Date: Thu Jul 06 03:56:52 2017 Add a mixed content type field to SecurityStyleExplanation and send it to DevTools. This prepares us for a future where mixed content explanations are calculated in the browser. This change also refactors the DevTools protocol to share the MixedContentType definition across the Security and Network protocols. (Previously, there was only an inline definition in Network.Resource). Bug: 657299, 736183 Change-Id: I5a53570114f4eff2e95865da015e078b01db518d Reviewed-on: https://chromium-review.googlesource.com/550854 Commit-Queue: Lucas Garron <lgarron@chromium.org> Reviewed-by: Blaise Bruer <allada@chromium.org> Reviewed-by: Pavel Feldman <pfeldman@chromium.org> Reviewed-by: Emily Stark <estark@chromium.org> Cr-Commit-Position: refs/heads/master@{#484459} [modify] https://crrev.com/68fe95883c015e32cdc96f107a2442630b14595c/components/security_state/DEPS [modify] https://crrev.com/68fe95883c015e32cdc96f107a2442630b14595c/components/security_state/content/content_utils.cc [modify] https://crrev.com/68fe95883c015e32cdc96f107a2442630b14595c/components/security_state/content/content_utils_unittest.cc [modify] https://crrev.com/68fe95883c015e32cdc96f107a2442630b14595c/content/browser/devtools/protocol/security_handler.cc [modify] https://crrev.com/68fe95883c015e32cdc96f107a2442630b14595c/content/public/browser/security_style_explanation.h [modify] https://crrev.com/68fe95883c015e32cdc96f107a2442630b14595c/third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.cpp [modify] https://crrev.com/68fe95883c015e32cdc96f107a2442630b14595c/third_party/WebKit/Source/core/inspector/browser_protocol.json [modify] https://crrev.com/68fe95883c015e32cdc96f107a2442630b14595c/third_party/WebKit/Source/devtools/front_end/network/NetworkLogView.js [modify] https://crrev.com/68fe95883c015e32cdc96f107a2442630b14595c/third_party/WebKit/Source/devtools/front_end/sdk/NetworkManager.js [modify] https://crrev.com/68fe95883c015e32cdc96f107a2442630b14595c/third_party/WebKit/Source/devtools/front_end/sdk/NetworkRequest.js [modify] https://crrev.com/68fe95883c015e32cdc96f107a2442630b14595c/third_party/WebKit/Source/devtools/front_end/security/SecurityPanel.js
,
Jul 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/38a3c2ce990afbabf043bf71035e885460389ca6 commit 38a3c2ce990afbabf043bf71035e885460389ca6 Author: Lucas Garron <lgarron@chromium.org> Date: Tue Jul 11 03:22:01 2017 Add ConnectionInfoPopupTest and PageInfoPopupTest classes on Android. Right now, these just test that ConnectionInfoPopupTest.show() and PageInfoPopupTest.show() can be invoked without crashing. Bug: 657299 Change-Id: Ic075b86e7d12df9893173bb35692ecc8b7e3df6d Reviewed-on: https://chromium-review.googlesource.com/557450 Commit-Queue: Lucas Garron <lgarron@chromium.org> Reviewed-by: Ted Choc <tedchoc@chromium.org> Cr-Commit-Position: refs/heads/master@{#485511} [modify] https://crrev.com/38a3c2ce990afbabf043bf71035e885460389ca6/chrome/android/java_sources.gni [add] https://crrev.com/38a3c2ce990afbabf043bf71035e885460389ca6/chrome/android/javatests/src/org/chromium/chrome/browser/page_info/ConnectionInfoPopupTest.java [add] https://crrev.com/38a3c2ce990afbabf043bf71035e885460389ca6/chrome/android/javatests/src/org/chromium/chrome/browser/page_info/PageInfoPopupTest.java
,
Jul 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bb87a597a382716b402cc20d3593f0b066cd9940 commit bb87a597a382716b402cc20d3593f0b066cd9940 Author: Lucas Garron <lgarron@chromium.org> Date: Tue Jul 25 17:35:45 2017 Move mixed content explanation calculation from DevTools to GetSecurityStyle(). # Layout Test Updates ## active-and-passive-subresources-with-cert-errors.html - Removed. - Now tested by `SecurityStateContentUtilsTest.CertErrorContentExplanations`. ## active-subresource-with-cert-errors.html - Removed. - Now tested by `SecurityStateContentUtilsTest.CertErrorContentExplanations`. ## blocked-mixed-content-and-subresources-with-cert-errors.html - "subresources-with-cert-errors" part removed. - Now handled by `SecurityStateContentUtilsTest.CertErrorContentExplanations`. - Renamed to `blocked-mixed-content.html` ## mixed-content-active-and-passive-reload.html - Explicitly pass explanations for passive mixed content (neutral) and active mixed content (insecure) into the SecurityStateChanged calls. - Add before/after refresh markers. ## mixed-content-and-subresources-with-cert-errors.html - Removed. - Now tested by `SecurityStateContentUtilsTest.MixedContentAndCertErrorExplanations`. ## mixed-content-reload.html - Explicitly pass explanation for passive mixed content (neutral) into the SecurityStateChanged calls. - Add before/after refresh markers. ## mixed-form.html - Removed - Mixed forms tested by `SecurityStateContentUtilsTest.MixedContentExplanations`. ## passive-subresource-with-cert-errors.html - Removed. - Now tested by `SecurityStateContentUtilsTest.CertErrorContentExplanations`. ## security-all-resources-secure.html - Removed. - Now tested by `SecurityStateContentUtilsTest.SecureContentExplanation`. ## security-blocked-mixed-content-and-malicious.html - Removed. - Mixed content explanation test redundant with `security-blocked-mixed- content.html` - Created `security-summary.html` to test the incidental security summary "override" previously in this test. ## security-blocked-mixed-content.html - Removed secure content explanation (not handled in DevTools anymore). ## security-explanation-ordering.html - Added `Secure resources` explanation to the list used for this test. ## security-secure-but-malicious.html - Removed. - Summary override handled by new `security-summary.html` test. - Safe Browsing state was already being calculated in browser. Added a `SecurityStateContentUtilsTest.GetSecurityStyleForSafeBrowsing` test for it. Bug: 736183 , 657299 Change-Id: I63738fa85b38302ce6024711be083ab60bde0bc6 Reviewed-on: https://chromium-review.googlesource.com/557427 Commit-Queue: Lucas Garron <lgarron@chromium.org> Reviewed-by: Pavel Feldman <pfeldman@chromium.org> Reviewed-by: Emily Stark <estark@chromium.org> Cr-Commit-Position: refs/heads/master@{#489355} [modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/chrome/browser/ssl/security_state_tab_helper_browser_tests.cc [modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/components/security_state/content/BUILD.gn [modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/components/security_state/content/content_utils.cc [modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/components/security_state/content/content_utils_unittest.cc [modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/components/security_state_strings.grdp [modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG [delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/active-and-passive-subresources-with-cert-errors-expected.txt [delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/active-and-passive-subresources-with-cert-errors.html [delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/active-subresource-with-cert-errors-expected.txt [delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/active-subresource-with-cert-errors.html [delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/blocked-mixed-content-and-subresources-with-cert-errors-expected.txt [add] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/third_party/WebKit/LayoutTests/http/tests/inspector/security/blocked-mixed-content-expected.txt [rename] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/third_party/WebKit/LayoutTests/http/tests/inspector/security/blocked-mixed-content.html [modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/third_party/WebKit/LayoutTests/http/tests/inspector/security/mixed-content-active-and-passive-reload-expected.txt [modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/third_party/WebKit/LayoutTests/http/tests/inspector/security/mixed-content-active-and-passive-reload.html [delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/mixed-content-and-subresources-with-cert-errors-expected.txt [delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/mixed-content-and-subresources-with-cert-errors.html [modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/third_party/WebKit/LayoutTests/http/tests/inspector/security/mixed-content-reload-expected.txt [modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/third_party/WebKit/LayoutTests/http/tests/inspector/security/mixed-content-reload.html [delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/mixed-form-expected.txt [delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/mixed-form.html [delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/passive-subresource-with-cert-errors-expected.txt [delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/passive-subresource-with-cert-errors.html [delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-all-resources-secure-expected.txt [delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-all-resources-secure.html [delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-blocked-mixed-content-and-malicious-expected.txt [delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-blocked-mixed-content-and-malicious.html [modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-blocked-mixed-content-expected.txt [modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-explanation-ordering.html [delete] https://crrev.com/65d1cd6e7454a3fd383d0deeaaad67918f97be3b/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-secure-but-malicious-expected.txt [add] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-summary-expected.txt [rename] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-summary.html [modify] https://crrev.com/bb87a597a382716b402cc20d3593f0b066cd9940/third_party/WebKit/Source/devtools/front_end/security/SecurityPanel.js
,
Aug 8 2017
,
Sep 13 2017
,
Nov 2 2017
,
Nov 10 2017
,
Nov 30 2017
,
Feb 18 2018
,
Apr 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/265017f3e9df214f8adecb482c5122e664ce32bb commit 265017f3e9df214f8adecb482c5122e664ce32bb Author: Mustafa Emre Acer <meacer@chromium.org> Date: Wed Apr 18 22:41:53 2018 Implement GetSecurityStyle for security bullets on Android (for DevTools and Connection Info). This CL fixes the long-standing bug where security panel bullets were not working for Android remote debugging via DevTools. BUG= 503216 , 657299 TEST=Following steps: 1) Connect an Android test device to a computer for USB debugging https://developers.google.com/web/tools/chrome-devtools/remote-debugging/ 2) Open chrome://inspect in Chrome on the computer and inspect a tab on the Android device. 3) In DevTools, go to Security > Overview. (Overview is selected by default.) 4) Visit google.com and check that the overview shows three green bullet points. 5) Visit https://mixed.badssl.com/ and check that there is a red "Mixed content" bullet along with two green bullets. Change-Id: Idb3c146cd597c31bcd0ec82d377766bf951c1edf Reviewed-on: https://chromium-review.googlesource.com/1006362 Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org> Reviewed-by: Emily Stark <estark@chromium.org> Commit-Queue: Mustafa Emre Acer <meacer@chromium.org> Cr-Commit-Position: refs/heads/master@{#551863} [modify] https://crrev.com/265017f3e9df214f8adecb482c5122e664ce32bb/chrome/browser/android/tab_web_contents_delegate_android.cc [modify] https://crrev.com/265017f3e9df214f8adecb482c5122e664ce32bb/chrome/browser/android/tab_web_contents_delegate_android.h
,
Dec 17
meacer: Is this good to close out?
,
Dec 17
The original bug (changing the page info to use bullet points à la security panel) still seems to be there. The details page has text and old icons (triangles and locks) for security states. I'm not sure if anyone has cycles to work on this now, but it would be good to at least clean up the old icons. |
||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by lafo...@chromium.org
, Oct 20 2016