Closed Bug 962787 Opened 7 years ago Closed 7 years ago

Implement New Designs for the InContent Untrusted and Attack! Pages

Categories

(Firefox for Metro Graveyard :: General, defect, P1)

x86_64
Windows 8.1
defect

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 28

People

(Reporter: mmaslaney, Assigned: azasypkin)

References

Details

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

Attachments

(7 files, 2 obsolete files)

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
Attached 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+
Attached 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+
Attached patch error_page.diffSplinter Review
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
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: 7 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.