Closed Bug 957806 Opened 6 years ago Closed 6 years ago

Stylize the Bad URL Error Page - Implementation

Categories

(Firefox for Metro Graveyard :: Theme, defect, P2)

x86_64
Windows 8.1
defect

Tracking

(firefox28 verified, firefox29 verified)

VERIFIED FIXED
Firefox 29
Tracking Status
firefox28 --- verified
firefox29 --- verified

People

(Reporter: MarcoM, Assigned: sfoster)

References

Details

(Whiteboard: [beta28] [defect] p=2)

Attachments

(3 files, 2 obsolete files)

+++ This bug was initially created as a clone of Bug #905621 +++

Implementation of the designs attached to resolved Bug 905621.
Keywords: uiwanted
Summary: Stylize the bad url error page → Stylize the Bad URL Error Page - Implementation
Assignee: nobody → sfoster
Blocks: metrov1it22
No longer blocks: metrov1backlog
Whiteboard: [beta28] [defect] p=0 → [beta28] [defect] p=2
Status: NEW → ASSIGNED
QA Contact: jbecerra
To test the patch, use a bogus url scheme like "xtqw:asd/asdas" for malformed URL and generic errors. For certificate errors, I've been using  https://woot.com and http://www.mozilla.org/firefox/its-an-attack.html for blocked site page

I tried to minimize the markup changes, while implementing the specs provided in bug 905621. Some of the dimensions indicated don't pan out - in particular the icon size is given as 40px, the image is 40x40 but from the comp it appears large - more like 60x60. I've scaled it to 60x60 for now. 

Also, the new in-browser content template doesn't currently have any provision for these pages which use color as a cue - the blocked site page has a red background. In the previous design the content is in a white box so its not quite so overwhelming. Likewise, the certificate error is yellow. I think we need further design input here. 

I'll attach screenshots for review also.
Attachment #8360198 - Flags: review?(rsilveira)
Flags: needinfo?(mmaslaney)
Sam, I'm currently working on mockup that will solve this issue, without having to re-theme the entire page.

Concerning the icon sizing, do they seem to small for the screen when sized at 40x40 pixels?
Flags: needinfo?(mmaslaney)
Hi Sam, I spoke with Michael and he will be adding themed error pages to the template which will be filed as a separate issue and doesn't impact the work you are doing for this current bug.
(In reply to Sam Foster [:sfoster] from comment #2)
> Created attachment 8360200 [details]
> netError-screenshots.png
> 
> Screenshot with attachment 8360198 [details] [diff] [review] applied

Looks great!
Here's how it looks with that icon at 40x40, with a little nip and tuck in the margins to re-balance it
The icon is currently at about opacity 0.8, I can bring that up if we want to better match the font color
Updates original patch to use our existing buttons look and feel, via platform.css
I was unable to get this to load from chrome://browser/skin/platform.css, so I've %include'd it as a stop-gap measure. 

The blocked page and bad certificate page were necessarily touched in this patch as they both share the same netError.css stylesheet. These will likely have follow-up work as :mmaslaney is working on new mockups for the error pages. 

I also restored the icons to 40px and got rid of the opacity. I think it looks ok? It was more a problem earlier on in my implementation before I had tightened up the spacing/margins. But that too might require follow-up.
Attachment #8360198 - Attachment is obsolete: true
Attachment #8360198 - Flags: review?(rsilveira)
Attachment #8360721 - Flags: review?(rsilveira)
Comment on attachment 8360721 [details] [diff] [review]
Implement new designs for netError (and certificate, blocked site) page

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

Looks awesome!

::: browser/metro/base/content/pages/netError.xhtml
@@ +357,4 @@
>  
>      </div>
>  
> +   <!--

super nit - indentation.
Attachment #8360721 - Flags: review?(rsilveira)
Attachment #8360721 - Flags: review+
On fx-team: https://hg.mozilla.org/integration/fx-team/rev/a01077805ed2
Whiteboard: [beta28] [defect] p=2 → [beta28] [defect] p=2 [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/a01077805ed2
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [beta28] [defect] p=2 [fixed-in-fx-team] → [beta28] [defect] p=2
Target Milestone: --- → Firefox 29
As landed, with whitepsace nits fixed. Carrying r+ from rsilveira
Attachment #8360721 - Attachment is obsolete: true
Attachment #8363057 - Flags: review+
Comment on attachment 8363057 [details] [diff] [review]
Implement new designs for netError (and certificate, blocked site) page (as landed)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a
User impact if declined: Not so nice looking bad URL page, bad certificate and blocked page UX
Testing completed (on m-c, etc.): Tested 
Risk to taking this patch (and alternatives if risky): Very low risk, metro-only patch
String or IDL/UUID changes made by this patch: None
Attachment #8363057 - Flags: approval-mozilla-aurora?
Keywords: checkin-needed
Whiteboard: [beta28] [defect] p=2 → [beta28] [defect] p=2 [approval-mozilla-aurora=metro-only]
Attachment #8363057 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/releases/mozilla-aurora/rev/f01a5df1733d
Keywords: checkin-needed
Whiteboard: [beta28] [defect] p=2 [approval-mozilla-aurora=metro-only] → [beta28] [defect] p=2
Kamil, please verify this is fixed in the latest Nightly and Aurora builds.
Flags: needinfo?(kamiljoz)
Verified as fixed for iteration #22, with latest Nightly and Aurora builds on Win 8 64-bit.
Status: RESOLVED → VERIFIED
Flags: needinfo?(kamiljoz)
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.