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

Issue 1179353002: Surface lock icon explanations in the DevTools Security panel. (Closed)

Created:
4 years, 10 months ago by lgarron
Modified:
4 years, 10 months ago
Reviewers:
dgozman
CC:
apavlov+blink_chromium.org, blink-reviews, caseq+blink_chromium.org, devtools-reviews_chromium.org, kozyatinskiy+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, pfeldman, sergeyv+blink_chromium.org, yurys+blink_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Surface lock icon explanations in the DevTools Security panel. BUG=501551 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=197487

Patch Set 1 #

Patch Set 2 : Display formatted explanations. #

Patch Set 3 : Additional linter annotations. #

Total comments: 16

Patch Set 4 : Address dgozman's comments. #

Patch Set 5 : Use SecurityAgent.SecurityStateExplanation instead of creating a WebInspector.SecurityModel.Securit… #

Total comments: 6

Patch Set 6 : Address nits. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+56 lines, -10 lines) Patch
M Source/devtools/front_end/security/SecurityModel.js View 1 2 3 4 5 2 chunks +5 lines, -4 lines 0 comments Download
M Source/devtools/front_end/security/SecurityPanel.js View 1 2 3 4 5 5 chunks +26 lines, -6 lines 0 comments Download
M Source/devtools/front_end/security/securityPanel.css View 1 2 3 2 chunks +25 lines, -0 lines 0 comments Download

Messages

Total messages: 10 (3 generated)
lgarron
dgozman@: I've tried to follow everything I learned from last CL. Would you mind reviewing ...
4 years, 10 months ago (2015-06-18 02:55:17 UTC) #2
dgozman
https://codereview.chromium.org/1179353002/diff/40001/Source/devtools/front_end/security/SecurityModel.js File Source/devtools/front_end/security/SecurityModel.js (right): https://codereview.chromium.org/1179353002/diff/40001/Source/devtools/front_end/security/SecurityModel.js#newcode25 Source/devtools/front_end/security/SecurityModel.js:25: /** @typedef {!{ Just put this in a single ...
4 years, 10 months ago (2015-06-18 10:50:49 UTC) #3
lgarron
https://codereview.chromium.org/1179353002/diff/40001/Source/devtools/front_end/security/SecurityModel.js File Source/devtools/front_end/security/SecurityModel.js (right): https://codereview.chromium.org/1179353002/diff/40001/Source/devtools/front_end/security/SecurityModel.js#newcode25 Source/devtools/front_end/security/SecurityModel.js:25: /** @typedef {!{ On 2015/06/18 at 10:50:48, dgozman wrote: ...
4 years, 10 months ago (2015-06-18 18:16:18 UTC) #4
dgozman
lgtm modulo some nits https://codereview.chromium.org/1179353002/diff/80001/Source/devtools/front_end/security/SecurityModel.js File Source/devtools/front_end/security/SecurityModel.js (right): https://codereview.chromium.org/1179353002/diff/80001/Source/devtools/front_end/security/SecurityModel.js#newcode66 Source/devtools/front_end/security/SecurityModel.js:66: var data = ({ nit: ...
4 years, 10 months ago (2015-06-19 11:33:36 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1179353002/100001
4 years, 10 months ago (2015-06-19 16:06:06 UTC) #8
commit-bot: I haz the power
Committed patchset #6 (id:100001) as https://src.chromium.org/viewvc/blink?view=rev&revision=197487
4 years, 10 months ago (2015-06-19 17:15:46 UTC) #9
lgarron
4 years, 10 months ago (2015-06-19 17:35:41 UTC) #10
Message was sent while issue was closed.
https://codereview.chromium.org/1179353002/diff/80001/Source/devtools/front_e...
File Source/devtools/front_end/security/SecurityModel.js (right):

https://codereview.chromium.org/1179353002/diff/80001/Source/devtools/front_e...
Source/devtools/front_end/security/SecurityModel.js:66: var data = ({
On 2015/06/19 at 11:33:36, dgozman wrote:
> nit: remove () and make this one-liner.

Done.


(Forgot to remove these while removing the @type annotation).

https://codereview.chromium.org/1179353002/diff/80001/Source/devtools/front_e...
File Source/devtools/front_end/security/SecurityPanel.js (right):

https://codereview.chromium.org/1179353002/diff/80001/Source/devtools/front_e...
Source/devtools/front_end/security/SecurityPanel.js:26: 
On 2015/06/19 at 11:33:36, dgozman wrote:
> nit: extra blank lines.

Removed.

https://codereview.chromium.org/1179353002/diff/80001/Source/devtools/front_e...
Source/devtools/front_end/security/SecurityPanel.js:55: for (var explanation of
explanations) {
On 2015/06/19 at 11:33:36, dgozman wrote:
> nit: no {} for one-line blocks.

Done.

(Even if I like my unconditional for-braces. :-P)

Powered by Google App Engine
This is Rietveld 408576698