Closed Bug 962787 Opened 10 years ago Closed 10 years ago

Implement New Designs for the InContent Untrusted and Attack! Pages

Categories

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

Product:
Firefox for Metro Graveyard
Component:
General
Platform:
x86_64
Windows 8.1
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
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)

Windows8-i03-inContent_caution.png
58.45 KB, image/png
Details
Windows8-i03-inContent_attack.png
53.86 KB, image/png
Details
Assets.zip
25.37 KB, application/zip
Details
snapped_display_none.jpg
86.80 KB, image/jpeg
Details
snapped_display_wrap.jpg
81.80 KB, image/jpeg
Details
merged.jpg
83.93 KB, image/jpeg
mmaslaney
: feedback+
Details
error_page.diff
43.59 KB, patch
sfoster
: review+
Details | Diff | Splinter Review 
Providing updated designs for the new InContent "Untrusted" and "Attack!" pages.
Those look great!
Blocks: metrov1backlog
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.diff
        — 
          — Splinter 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: 10 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.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: