451131 - Polish the design of the certificate error page

Status: RESOLVED FIXED

(Camino Graveyard :: Security, defect)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Two items spun off of bug 398417:

> - Pretty-ify the error page's "add an exception" section if we want to
> (Firefox's yellow rounded rect, or something of that nature)

> The part that bothers me is that the human-readable description of the actual
> error is "buried" between the intro sentence and the NSS error code; can we fix
> something in our netError to make them all paragraphs?
> 
> cbo.ardisson.org uses an invalid security certificate.
> 
> The certificate is not trusted because it is self signed.
> The certificate is only valid for box183.bluehost.com
> 
> (Error code: sec_error_untrusted_issuer)
> 
> (Oddly, above is how it appears when pasted from the page into this bug, which
> implies the <p>s or <br>s are there but aren't appearing properly in the HTML
> page.)

Comment 1

[philippe (part-time)]

The second part is trivial, 'fox uses white-space:pre-wrap, and playing with line-height a little.

basically you have 3 lines of text in the source, like this:
xxx

xxx

xxx

no <br> or whatever.

---

The first part (yellow box) is not difficult to implement either, bits of colour, bits of background-colour some padding and a touch of -moz-border-radius.

As far as I can tell, the XHMTL source is identical between 'fox and Camino.

Comment 2

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Yeah, I diffed the css files at one point a few weeks ago and it looked to me like we just needed to add a few add a few rules.

Comment 3

[philippe (part-time)]

Attachment: screenshot

with the 'fox styling, looks like this.
patch coming up soon

Comment 4

[philippe (part-time)]

Attachment: minimal fixes

A yellow box and some line-breaking...
(and 1 white-space clean-up, while I was there)

Smokey, does that sound OK, at least for now ?

Comment 5

[Stuart Morgan]

We should try to left-align the text with the bullets, otherwise there are too many different left alignments too close together. Also, the yellows clash. Could we take a color from the warning icon (which is a bit more orange) and lighten it without changing the hue?

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 336967 [details] [diff] [review]
minimal fixes

A couple of things here:

1) Please add yourself to the contributors here; you've now had a hand in all of the Camino fixes to this file ;)

2) There's been a fair bit of drift between our file and the toolkit/themes/pinstripe over the course of the SSL fun (with a few late malware changes, too); I'd really like to see us pick up the missing rules and any relevant missing properties.

3) Specific comments on the CSS:

> #errorShortDesc > p {
>   overflow: auto;
>   border-bottom: 1px solid ThreeDLightShadow;
>   padding-bottom: 1em;
>   font-size: 130%;
>-   color: #0141b7;
>+  color: #0141b7;
>+  white-space:pre-wrap;

Nit: space after the colon, to be consistent with the file.

>+#securityOverrideDiv {
>+  padding-top: 10px;
>+}

Not this rule, but one of the things that bothers me about the yellow box (and it exists in the Fx version, too) is that while the space between the box border and the main content is equal (or nearly so), but the space between the yellow box's content and the edge of the yellow box is completely inconsistent.  Can we make the empty space widths more uniform?

Also, I don't know if it's possible (or just an artifact of not having bug 389140) or not, but the button would be more Mac-like if it left-aligned to the left edge of the yellow box's text rather than being indented.

I messed around with these but couldn't get them exactly right.

>+#securityOverrideContent {
>+  background-color: #FFF090; /* Pale yellow */
>+  padding: 10px;
>+  -moz-border-radius: 10px;
>+}

We use 15px border-radius elsewhere, and I think this box looks better with 15px corners, too.

Please fix at least 1, 2, the nit, and the border-radius ;) and thanks for working on this.

Comment 7

[philippe (part-time)]

(In reply to comment #5)
> We should try to left-align the text with the bullets, otherwise there are too
> many different left alignments too close together.
You mean, the text _in_ the yellow box ? That shouldn't be difficult. Moving the box itself is certainly possible, I'll play with it.

The main paragraph at the top: the same styling is used for all netError messages. I didn't want to to touch anything but the white-space property.

Also, the yellows clash.
> Could we take a color from the warning icon (which is a bit more orange) and
> lighten it without changing the hue?

I'll play with that too.

(In reply to comment #6)
> (From update of attachment 336967 [details] [diff] [review])

I'l post an updated patch with those later (tonight local time, kids are coming back from school in a moment).

Comment 8

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I think (hope) fixing the space issue I mentioned will probably fix the alignment issue that Stuart raised while I was reviewing, or at least bring it close.

Comment 9

[philippe (part-time)]

(In reply to comment #6) 
> 1) Please add yourself to the contributors here; you've now had a hand in all
> of the Camino fixes to this file ;)

Done
 
> 2) There's been a fair bit of drift between our file and the
> toolkit/themes/pinstripe over the course of the SSL fun (with a few late
> malware changes, too); I'd really like to see us pick up the missing rules and
> any relevant missing properties.

I've diffed the Camino netError.css with the one from /toolkit/themes/pinstripe but found no differences, with the exception of one block related to the ssl fun: they use a different icon at the top of the page. I've added this block, commented out, to be complete.
If there is more (properties in netError.dtd ? you mean ?) I'll have to dig deeper.

> 3) Specific comments on the CSS:
> 
> > #errorShortDesc > p {
> >   overflow: auto;
> >   border-bottom: 1px solid ThreeDLightShadow;
> >   padding-bottom: 1em;
> >   font-size: 130%;
> >-   color: #0141b7;
> >+  color: #0141b7;
> >+  white-space:pre-wrap;
> 
> Nit: space after the colon, to be consistent with the file.

Shame on me :-(. Blame me or the OS on missing the first character while copying. Fixed.

> 
> >+#securityOverrideDiv {
> >+  padding-top: 10px;
> >+}
> 
> Not this rule, but one of the things that bothers me about the yellow box (and
> it exists in the Fx version, too) is that while the space between the box
> border and the main content is equal (or nearly so), but the space between the
> yellow box's content and the edge of the yellow box is completely inconsistent.
>  Can we make the empty space widths more uniform?

OK, I've reworked the 'yellow' box a little.
* the left and right sides of the text align with the text in the bulleted list above.
* the padding for the yellow box should be consistent on all sides, independently of font-size (em size)
* the 'link' ('Or you can add an exception…') is also aligned with the list
* I've made the background-colour more orange, per Stuart's comment
* the right-margin of the yellow box matches the right-margin on the bulleted list.
* the border-radius is set to 15px

(note: the 'fox version has much more space at the top of the content, coming from the margin-top on the paragraph of text)

> 
> Also, I don't know if it's possible (or just an artifact of not having bug
> 389140) or not, but the button would be more Mac-like if it left-aligned to the
> left edge of the yellow box's text rather than being indented.

Partly it depends on an updated buttons.css (bug 389140) that gives a screwy impression. Partly, XUL buttons have a default margin. I set them to '0', limited to this particular error page.
 
> ... and thanks for working on this.
My pleasure. At least something I can fix :-)

Comment 10

[philippe (part-time)]

Attachment: updated netError.css patch v1.1

sorry about that whole data:url in it, but -u8 took it :-(.

Comment 11

[philippe (part-time)]

Attachment: screenshot with updated patch

On the left is Camino, on the right is Minefield. Screenshot was taken with a static file. The comparison with Minefield is there to show how it looks like with a correct button (buttons.css issue).
Also, Minefield is running with colour management turned on, Camino at plain defaults

Comment 12

[Stuart Morgan]

(In reply to comment #7)
> You mean, the text _in_ the yellow box ?

Sorry, yes. Your new version looks pretty much exactly like what I was envisioning all around :)

Comment 13

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 337033 [details] [diff] [review]
updated netError.css patch v1.1

I'm not as fond of this color as the old yellow, but it's not obnoxious and Stuart likes it, so I'll live ;)

A couple of little things that we can fix on checkin:

The toolkit version of the file has this bizarre rule

#errorLongDesc > p {
}

which, uh, looks like it does nothing, but for completeness's sake our file should have it there doing nothing, too.

Toolkit's "Custom styling for 'blacklist' error class - bug 380932" block had 

:root.blacklist a

added to it, so we should add it, too.

Finally, we should add a comment explaining why we have these three rule-blocks, since it's not immediately obv:

+/* the 'link' */
+#securityOverrideLink {
+  -moz-margin-start: 1.5em;
+}
+
+#securityOverrideContent > p {
+  margin: 0 0 1em;
+}
+
+#securityOverrideContent > button {
+  margin: 0;
+}

Something like "Ensure all text is left-aligned to the same position - bug 451131 comment 9"

Again, no reason to spin a new patch; we can make these changes on checkin if Stuart doesn't find anything. r=ardissone

Comment 14

[Stuart Morgan]

Comment on attachment 337033 [details] [diff] [review]
updated netError.css patch v1.1

sr=smorgan with Smokey's changes.

Comment 15

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Landed on cvs trunk with the changes in comment 13; thanks!