Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(212)

Issue 1316853002: Organize security panel main view. (Closed)

Created:
5 years, 4 months ago by lgarron
Modified:
5 years, 3 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

Organize security panel main view. This includes the following changes: - Material design icons to match the current mocks better. - Rename securityPanel.css into mainView.css, because it only contains main view CSS anyhow. - Rewrite security panel main view DOM code to match the mocks. BUG=484387 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=201267

Patch Set 1 : Include mixed content explanations. #

Patch Set 2 : Change the securityPanel experiment to be non-hidden. #

Total comments: 9

Patch Set 3 : Address nits. #

Patch Set 4 : Remove experiment toggle. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+158 lines, -99 lines) Patch
M Source/devtools/devtools.gypi View 2 chunks +4 lines, -9 lines 0 comments Download
D Source/devtools/front_end/Images/securityStateInsecure.png View Binary file 0 comments Download
A Source/devtools/front_end/Images/securityStateInsecure.svg View 1 chunk +17 lines, -0 lines 0 comments Download
D Source/devtools/front_end/Images/securityStateInsecure_2x.png View Binary file 0 comments Download
D Source/devtools/front_end/Images/securityStateNeutral.png View Binary file 0 comments Download
A Source/devtools/front_end/Images/securityStateNeutral.svg View 1 chunk +11 lines, -0 lines 0 comments Download
D Source/devtools/front_end/Images/securityStateNeutral_2x.png View Binary file 0 comments Download
D Source/devtools/front_end/Images/securityStateSecure.png View Binary file 0 comments Download
A Source/devtools/front_end/Images/securityStateSecure.svg View 1 chunk +17 lines, -0 lines 0 comments Download
D Source/devtools/front_end/Images/securityStateSecure_2x.png View Binary file 0 comments Download
D Source/devtools/front_end/Images/securityStateWarning.png View Binary file 0 comments Download
D Source/devtools/front_end/Images/securityStateWarning_2x.png View Binary file 0 comments Download
M Source/devtools/front_end/security/SecurityModel.js View 1 2 1 chunk +18 lines, -1 line 0 comments Download
M Source/devtools/front_end/security/SecurityPanel.js View 1 2 4 chunks +28 lines, -14 lines 0 comments Download
M Source/devtools/front_end/security/lockIcon.css View 1 chunk +4 lines, -31 lines 0 comments Download
A Source/devtools/front_end/security/mainView.css View 1 2 1 chunk +58 lines, -0 lines 0 comments Download
M Source/devtools/front_end/security/module.json View 1 chunk +1 line, -1 line 0 comments Download
D Source/devtools/front_end/security/securityPanel.css View 1 chunk +0 lines, -43 lines 0 comments Download

Messages

Total messages: 12 (5 generated)
lgarron
dgozman@: Would you mind reviewing this change to the security panel main view? Mockup: https://lh3.googleusercontent.com/x9W29VssgJeyFFK0vQsAcoEwJvWGSIpreAnVzJQas4M=w1200-h638-no
5 years, 4 months ago (2015-08-25 23:23:21 UTC) #2
lgarron
https://codereview.chromium.org/1316853002/diff/40001/Source/devtools/front_end/main/Main.js File Source/devtools/front_end/main/Main.js (right): https://codereview.chromium.org/1316853002/diff/40001/Source/devtools/front_end/main/Main.js#newcode141 Source/devtools/front_end/main/Main.js:141: Runtime.experiments.register("securityPanel", "Security panel"); Paul: if I recall correctly, you ...
5 years, 3 months ago (2015-08-26 04:17:18 UTC) #4
dgozman
lgtm with comments https://codereview.chromium.org/1316853002/diff/40001/Source/devtools/front_end/main/Main.js File Source/devtools/front_end/main/Main.js (right): https://codereview.chromium.org/1316853002/diff/40001/Source/devtools/front_end/main/Main.js#newcode141 Source/devtools/front_end/main/Main.js:141: Runtime.experiments.register("securityPanel", "Security panel"); On 2015/08/26 04:17:18, ...
5 years, 3 months ago (2015-08-26 20:28:41 UTC) #5
lgarron
https://codereview.chromium.org/1316853002/diff/40001/Source/devtools/front_end/main/Main.js File Source/devtools/front_end/main/Main.js (right): https://codereview.chromium.org/1316853002/diff/40001/Source/devtools/front_end/main/Main.js#newcode141 Source/devtools/front_end/main/Main.js:141: Runtime.experiments.register("securityPanel", "Security panel"); On 2015/08/26 at 20:28:41, dgozman wrote: ...
5 years, 3 months ago (2015-08-26 21:42:44 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1316853002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1316853002/100001
5 years, 3 months ago (2015-08-26 22:00:19 UTC) #10
commit-bot: I haz the power
Committed patchset #4 (id:100001) as https://src.chromium.org/viewvc/blink?view=rev&revision=201267
5 years, 3 months ago (2015-08-27 00:24:27 UTC) #11
paulirish
5 years, 3 months ago (2015-08-27 01:36:38 UTC) #12
Message was sent while issue was closed.
> > On 2015/08/26 04:17:18, lgarron wrote:
> > > Paul: if I recall correctly, you were interested in changing the
securityPanel
> > > to a non-hidden experiment and merging to M46 along with
> > > https://codereview.chromium.org/1301833003 (and Pavel didn't feel
strongly, as
> > > long as we don't ship with a yellow icon before we remove the experimental
flag
> > > altogether). Still the case?
> > 
> > If you plan to merge to M46, create a separate patch with just this change.
> 
> That would mean three merges instead of two. Is that preferred?
> 

Three is fine.

Powered by Google App Engine
This is Rietveld 408576698