New issue
Advanced search Search tips

Issue 703882 link

Starred by 1 user
Status: Fixed
Owner:
est...@chromium.org
Closed: Mar 2017
Cc:
elawrence@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Clean up SecurityStyleExplanations fields

Project Member Reported by est...@chromium.org, Mar 21 2017 
The std::vector<SecurityStyleExplanation> fields are confusingly named and not well-documented.

|unauthenticated_explanations| is really "This is why we removed your lock icon"; |broken_explanations| is really "This is why we're showing Red Dangerous", and |info_explanations| is "This is something you should know but doesn't currently affect your lock icon."

We should fix the names and comments to make that clearer.

Comment 1 by lgar...@chromium.org, Mar 22 2017 

+1

I actually got that wrong on the code review, because I instinctively assumed that unauthenticated_explanations meant "bad" in contrast to "neutral".
(I think I switched over all the DevTools code to use "neutral" instead of e.g. "unauthenticated" a while back but never got around to this.)

Comment 2 by est...@chromium.org, Mar 22 2017

Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 24 2017 

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

commit 90ff901e2d8127ff990319818f93de838fcd8990
Author: estark <estark@chromium.org>
Date: Fri Mar 24 21:34:27 2017

Rename SecurityStyleExplanations and WebSecurityStyle fields

This CL renames various SecurityStyleExplanations fields and WebSecurityStyle
values to be more accurate and consistent with the rest of the codebase.
- unauthenticated => neutral
- authenticated => secure (a page can be fully authenticated but not marked with
  the state that was formerly called authenticated, e.g. because of Safe
  Browsing checks)
- authentication broken => insecure

BUG= 703882 

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

[modify] https://crrev.com/90ff901e2d8127ff990319818f93de838fcd8990/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc
[modify] https://crrev.com/90ff901e2d8127ff990319818f93de838fcd8990/chrome/browser/ssl/security_state_tab_helper_browser_tests.cc
[modify] https://crrev.com/90ff901e2d8127ff990319818f93de838fcd8990/components/security_state/content/content_utils.cc
[modify] https://crrev.com/90ff901e2d8127ff990319818f93de838fcd8990/components/security_state/content/content_utils_unittest.cc
[modify] https://crrev.com/90ff901e2d8127ff990319818f93de838fcd8990/content/browser/devtools/protocol/security_handler.cc
[modify] https://crrev.com/90ff901e2d8127ff990319818f93de838fcd8990/content/child/web_url_loader_impl.cc
[modify] https://crrev.com/90ff901e2d8127ff990319818f93de838fcd8990/content/public/browser/security_style_explanations.h
[modify] https://crrev.com/90ff901e2d8127ff990319818f93de838fcd8990/third_party/WebKit/public/platform/WebSecurityStyle.h

Comment 4 by est...@chromium.org, Mar 24 2017

Status: Fixed (was: Started)
►

Sign in to add a comment