Make PEM debugging section fixed width in the SSL interstitial

Make PEM debugging section fixed width in the SSL interstitial

BUG=655245
Committed: https://crrev.com/f67daf2e939134b3f33a938c2d44a3dc2b93d406
Cr-Commit-Position: refs/heads/master@{#425486}

[meacer, 2016-10-12 19:42:13]

meacer@chromium.org changed reviewers:
+ lgarron@chromium.org

[meacer, 2016-10-12 19:42:14]

PTAL?

[meacer, 2016-10-12 19:42:37]

The CQ bit was checked by meacer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-12 19:42:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[lgarron, 2016-10-12 19:43:37]

https://codereview.chromium.org/2412993002/diff/1/components/security_interst...
File components/security_interstitials/core/browser/resources/interstitial_v2.js
(right):

https://codereview.chromium.org/2412993002/diff/1/components/security_interst...
components/security_interstitials/core/browser/resources/interstitial_v2.js:78:
spanTitle.classList.add('debugging-title');
Duplicate line?

[meacer, 2016-10-12 19:47:03]

https://codereview.chromium.org/2412993002/diff/1/components/security_interst...
File components/security_interstitials/core/browser/resources/interstitial_v2.js
(right):

https://codereview.chromium.org/2412993002/diff/1/components/security_interst...
components/security_interstitials/core/browser/resources/interstitial_v2.js:78:
spanTitle.classList.add('debugging-title');
On 2016/10/12 19:43:37, lgarron wrote:
> Duplicate line?

Done.

[elawrence, 2016-10-12 22:27:00]

On 2016/10/12 19:42:14, Mustafa Emre Acer wrote:
> PTAL?

Monospace shrinks the font down a bit. Maybe use 

  font-size: 1.2em
  margin-top: 0.5em

?

[meacer, 2016-10-12 23:40:06]

On 2016/10/12 22:27:00, elawrence wrote:
> On 2016/10/12 19:42:14, Mustafa Emre Acer wrote:
> > PTAL?
> 
> Monospace shrinks the font down a bit. Maybe use 
> 
>   font-size: 1.2em
>   margin-top: 0.5em
> 
> ?

Done, looks nicer. PTAL?

[elawrence, 2016-10-13 16:50:02]

One trivial nit, but this otherwise looks great!

https://codereview.chromium.org/2412993002/diff/40001/components/security_int...
File
components/security_interstitials/core/browser/resources/interstitial_v2.css
(right):

https://codereview.chromium.org/2412993002/diff/40001/components/security_int...
components/security_interstitials/core/browser/resources/interstitial_v2.css:68:
display: block;
I think display:block should go first, per
https://engdoc.corp.google.com/eng/doc/htmlcssguide.xml?cl=head#Declaration_O...

[meacer, 2016-10-13 17:36:49]

Thanks!

https://codereview.chromium.org/2412993002/diff/40001/components/security_int...
File
components/security_interstitials/core/browser/resources/interstitial_v2.css
(right):

https://codereview.chromium.org/2412993002/diff/40001/components/security_int...
components/security_interstitials/core/browser/resources/interstitial_v2.css:68:
display: block;
On 2016/10/13 16:50:02, elawrence wrote:
> I think display:block should go first, per
>
https://engdoc.corp.google.com/eng/doc/htmlcssguide.xml?cl=head#Declaration_O...

Ah, didn't know about alphabetical sorting rule. Thanks for the heads up.

[meacer, 2016-10-14 21:23:22]

meacer@chromium.org changed reviewers:
+ elawrence@chromium.org, felt@chromium.org

[meacer, 2016-10-14 21:23:22]

lgarron: Ping, would like to land this today.

[meacer, 2016-10-14 21:29:41]

Whops, felt, I added you for owner review. I'm not sure if a non-owner review is
sufficient as long as the CL author is an owner. If it is, feel free to ignore.

[lgarron, 2016-10-14 21:31:42]

LGTM (but I'm not an owner).

[felt, 2016-10-14 21:32:33]

https://codereview.chromium.org/2412993002/diff/60001/components/security_int...
File components/security_interstitials/core/browser/resources/interstitial_v2.js
(right):

https://codereview.chromium.org/2412993002/diff/60001/components/security_int...
components/security_interstitials/core/browser/resources/interstitial_v2.js:73:
function appendDebuggingField(title, value, extraCssClass) {
Do you plan to use this for other things in the future? If not, would it be
easier to read this in the future if it just directly said |fixedWidth| and had
a bool?

[lgarron, 2016-10-14 21:33:05]

https://codereview.chromium.org/2412993002/diff/60001/components/security_int...
File components/security_interstitials/core/browser/resources/interstitial_v2.js
(right):

https://codereview.chromium.org/2412993002/diff/60001/components/security_int...
components/security_interstitials/core/browser/resources/interstitial_v2.js:71:
* @param {string=} extraCssClass Additional CSS classes. Optional.
Nit: I just noticed that this is an incorrect plural. Could you change to
"Additional CSS class" (unless you want to turn this parameter into a list)?

[meacer, 2016-10-14 21:44:37]

https://codereview.chromium.org/2412993002/diff/60001/components/security_int...
File components/security_interstitials/core/browser/resources/interstitial_v2.js
(right):

https://codereview.chromium.org/2412993002/diff/60001/components/security_int...
components/security_interstitials/core/browser/resources/interstitial_v2.js:71:
* @param {string=} extraCssClass Additional CSS classes. Optional.
On 2016/10/14 21:33:05, lgarron wrote:
> Nit: I just noticed that this is an incorrect plural. Could you change to
> "Additional CSS class" (unless you want to turn this parameter into a list)?

Changed the param to a boolean and renamed it.

https://codereview.chromium.org/2412993002/diff/60001/components/security_int...
components/security_interstitials/core/browser/resources/interstitial_v2.js:73:
function appendDebuggingField(title, value, extraCssClass) {
On 2016/10/14 21:32:33, felt wrote:
> Do you plan to use this for other things in the future? If not, would it be
> easier to read this in the future if it just directly said |fixedWidth| and
had
> a bool?

Not sure if we'll need new stuff, but to keep it simple let's make it
fixedWidth.

[felt, 2016-10-14 21:45:24]

lgtm

[meacer, 2016-10-14 21:54:13]

The CQ bit was checked by meacer@chromium.org

[meacer, 2016-10-14 21:54:13]

The patchset sent to the CQ was uploaded after l-g-t-m from
lgarron@chromium.org, felt@chromium.org
Link to the patchset: https://codereview.chromium.org/2412993002/#ps100001
(title: "bool")

[commit-bot: I haz the power, 2016-10-14 21:54:51]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-14 22:04:13]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-10-14 22:06:51]

Description was changed from

==========
Make PEM debugging section fixed width in the SSL interstitial

BUG=655245
==========

to

==========
Make PEM debugging section fixed width in the SSL interstitial

BUG=655245

Committed: https://crrev.com/f67daf2e939134b3f33a938c2d44a3dc2b93d406
Cr-Commit-Position: refs/heads/master@{#425486}
==========

[commit-bot: I haz the power, 2016-10-14 22:06:52]

Patchset 6 (id:??) landed as
https://crrev.com/f67daf2e939134b3f33a938c2d44a3dc2b93d406
Cr-Commit-Position: refs/heads/master@{#425486}