Implement New Designs for the InContent Untrusted and Attack! Pages

VERIFIED FIXED in Firefox 28

Status

defect
P1
normal
VERIFIED FIXED
6 years ago
5 years ago

People

(Reporter: mmaslaney, Assigned: azasypkin)

Tracking

unspecified
Firefox 28
x86_64
Windows 8.1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [release28] p=1 s=it-30c-29a-28b.1 r=ff28)

Attachments

(7 attachments, 2 obsolete attachments)

Providing updated designs for the new InContent "Untrusted" and "Attack!" pages.
Those look great!
Summary: New Designs for the InContent Untrusted and Attack! Pages → Implement New Designs for the InContent Untrusted and Attack! Pages
Whiteboard: [triage] [feature] p=0
Yeah, nice solution :mmaslaney, I propose this as release blocking and p=1
Whiteboard: [triage] [feature] p=0 → [release28] [feature] p=1
Blocks: 961193
Assignee: nobody → azasypkin
Status: NEW → ASSIGNED
Posted image merged.jpg
Are there any thoughts how to display title icon when there is no enough space for it(snapped view or portrait mode). I've attached 3 possible ways that I currently see: hide icon, wrap it or leave along with the text.
Attachment #8368608 - Flags: feedback?(mmaslaney)
Comment on attachment 8368608 [details]
merged.jpg

I like option #2 for it's scaling capabilities.
Attachment #8368608 - Flags: feedback?(mmaslaney) → feedback+
Posted patch error_page.diff (obsolete) — Splinter Review
"untrusted certificate" and "blocked site" error pages updated with the new design. Also we probably need to update icon (increase size to 60x60) for the last untouched error page (neterror) to simplify and generalize styles.
Attachment #8368837 - Flags: feedback?(sfoster)
Comment on attachment 8368837 [details] [diff] [review]
error_page.diff

Review of attachment 8368837 [details] [diff] [review]:
-----------------------------------------------------------------

This is looking good. Just a few comments below.

::: browser/metro/base/content/pages/aboutCertError.xhtml
@@ +179,5 @@
>  
>        function toggle(id) {
>          var el = document.getElementById(id);
> +        if (el.getAttribute("collapsed") == "true") {
> +           el.setAttribute("collapsed", false);

I know this was already implemented this way, but usually if we just need a boolean check we setAttribute("collapsed", true) and removeAttribute("collapsed"). That lets us write selectors element[collapsed] that just test the presence of the attribute and not worry about its value. If you're touching this code, lets fix it

@@ +188,5 @@
>      ]]></script>
>    </head>
>  
>    <body id="errorPage" class="certerror" dir="&locale.dir;">
> +    <div class="top-stripes"></div>

maybe class=top-decoration? That way its a little less specific to today's design solution

::: browser/metro/theme/netError.css
@@ +73,5 @@
> +    margin: 5px 10px 5px 0;
> +}
> +
> +#errorPage .top-stripes {
> +    position: absolute;

Lets use '>' here as .top-stripes is a child of #errorPage

@@ +109,5 @@
> +    color: #0095dd;
> +}
> +
> +#errorPage div[collapsed] {
> +    padding-left: 0;

should this be -moz-padding-start?

@@ +114,5 @@
> +}
> +
> +#errorPage .expandable-heading {
> +    cursor: pointer;
> +    padding-left: 15px;

ditto here
Attachment #8368837 - Flags: feedback?(sfoster) → feedback+
Thanks for feedback! Never thought about all this RTL stuff before, but we definitely should care about it :)
Attachment #8368837 - Attachment is obsolete: true
Attachment #8369331 - Flags: review?(sfoster)
Priority: -- → P1
QA Contact: jbecerra
Whiteboard: [release28] [feature] p=1 → [release28] [feature] p=1 s=it-30c-29a-28b.1
Comment on attachment 8369331 [details] [diff] [review]
error_page.diff

Review of attachment 8369331 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great, nice work. Tested various aspect ratios, with ltr and rtl.
Attachment #8369331 - Flags: review?(sfoster) → review+
Thanks for review!
Keywords: checkin-needed

Comment 15

5 years ago
Making the urlbar black would be a nice touch for the attack page.
Target Milestone: --- → Firefox 28
OS: Mac OS X → Windows 8.1
Hardware: x86 → x86_64
https://hg.mozilla.org/integration/fx-team/rev/cb7a850bcb4b
Keywords: checkin-needed
Whiteboard: [release28] [feature] p=1 s=it-30c-29a-28b.1 → [release28] [feature] p=1 s=it-30c-29a-28b.1[fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/cb7a850bcb4b
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [release28] [feature] p=1 s=it-30c-29a-28b.1[fixed-in-fx-team] → [release28] [feature] p=1 s=it-30c-29a-28b.1
Whiteboard: [release28] [feature] p=1 s=it-30c-29a-28b.1 → [release28] p=1 s=it-30c-29a-28b.1 r=ff28
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:30.0) Gecko/20100101 Firefox/30.0

Verified as fixed on latest Nightly (build ID: 20140219030203) using Microsoft Surface Pro 2.

The "Content Untrusted" and "Attack! Page" are updated accordingly to the design.
Marking as 'Verified' based on QA testing in comment #18.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.