Last Comment Bug 747608 - The identity-box should have a smaller font-size and a gradient end border instead of a solid line
: The identity-box should have a smaller font-size and a gradient end border in...
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Theme (show other bugs)
: 14 Branch
: All All
: -- normal with 1 vote (vote)
: Firefox 15
Assigned To: Jared Wein [:jaws] (please needinfo? me)
:
Mentors:
https://bugzilla.mozilla.org/attachme...
Depends on:
Blocks: 742419
  Show dependency treegraph
 
Reported: 2012-04-20 23:20 PDT by Jared Wein [:jaws] (please needinfo? me)
Modified: 2013-11-13 03:07 PST (History)
8 users (show)
jaws: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
WIP of patch (Windows only) (1.64 KB, patch)
2012-04-20 23:20 PDT, Jared Wein [:jaws] (please needinfo? me)
fryn: feedback+
Details | Diff | Splinter Review
Screen shot showing w/ and w/o line (55.88 KB, image/png)
2012-04-23 12:26 PDT, Brian Smith (:briansmith, :bsmith, use NEEDINFO?)
no flags Details
Emboldened text (40.30 KB, image/png)
2012-04-23 14:14 PDT, Frank Yan (:fryn)
no flags Details
Patch for bug (Windows, Linux, OS X) (4.76 KB, patch)
2012-04-23 15:18 PDT, Jared Wein [:jaws] (please needinfo? me)
no flags Details | Diff | Splinter Review
Patch for bug v2 (Windows, Linux, OS X) (4.76 KB, patch)
2012-04-23 15:45 PDT, Jared Wein [:jaws] (please needinfo? me)
fryn: feedback+
Details | Diff | Splinter Review
Patch for bug v3 (Windows, Linux, and OS X) (4.75 KB, patch)
2012-05-03 18:20 PDT, Jared Wein [:jaws] (please needinfo? me)
dao+bmo: review-
Details | Diff | Splinter Review
Screenshot of patch (22.04 KB, image/png)
2012-05-03 18:21 PDT, Jared Wein [:jaws] (please needinfo? me)
no flags Details
Patch for bug v4 (Windows, Linux, and OS X) (4.87 KB, patch)
2012-05-03 21:19 PDT, Jared Wein [:jaws] (please needinfo? me)
dao+bmo: review-
Details | Diff | Splinter Review
Patch for bug v5 (4.90 KB, patch)
2012-05-04 07:52 PDT, Jared Wein [:jaws] (please needinfo? me)
no flags Details | Diff | Splinter Review
Patch for bug v6 (4.90 KB, patch)
2012-05-04 12:57 PDT, Jared Wein [:jaws] (please needinfo? me)
dao+bmo: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Jared Wein [:jaws] (please needinfo? me) 2012-04-20 23:20:56 PDT
Created attachment 617187 [details] [diff] [review]
WIP of patch (Windows only)

As the mockup in the URL shows, there should be a smaller font-size for the identity-box text. The end border on the identity-box when viewing an https-ev site be a linear gradient instead of a solid line.
Comment 1 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2012-04-23 12:26:51 PDT
Created attachment 617596 [details]
Screen shot showing w/ and w/o line

Why should there be a separator line/gradient between the site identity block and the URL in the EV case, but no separator line/gradient in the non-EV case? IMO, it is easier to read and looks good w/o any separator, and that would make the EV and non-EV cases consistent. See screenshot for comparison.
Comment 2 Jared Wein [:jaws] (please needinfo? me) 2012-04-23 12:31:19 PDT
(In reply to Brian Smith (:bsmith) from comment #1)
> Why should there be a separator line/gradient between the site identity
> block and the URL in the EV case, but no separator line/gradient in the
> non-EV case?

This is because the EV case has text shown, so we want to provide a separator between the two text groups.
Comment 3 Frank Yan (:fryn) 2012-04-23 14:14:35 PDT
Created attachment 617642 [details]
Emboldened text

Jared, the text should have bold font-weight as in the screenshot I just made.
The font-size of 85% is what that Stephen and I came up with in-person.

P.S. "embolden" is a real word. Stephen didn't believe me:
https://www.google.com/search?q=define+embolden
Comment 4 Frank Yan (:fryn) 2012-04-23 14:20:40 PDT
Comment on attachment 617187 [details] [diff] [review]
WIP of patch (Windows only)

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

::: browser/themes/winstripe/browser.css
@@ +1343,5 @@
>  /* identity box */
>  
>  #identity-box {
>    padding: 2px;
> +  font-size: .9em;

I think .9em is fine. The .85 that I suggested in the previous comment might be a bit too small.
Please embolden this.

@@ +1398,5 @@
> +}
> +
> +#identity-box.verifiedIdentity:after {
> +  content: "";
> +  display: block;

Can you verify the height of this pseudo-element extends from the top of the identity box to the bottom of it (e.g. setting a CSS outline on it to check its dimensions, etc.)?
We don't want it to be only as tall as the font-size.

@@ +1401,5 @@
> +  content: "";
> +  display: block;
> +  width: 1px;
> +  -moz-margin-start: 2px;
> +  background-image: -moz-linear-gradient(to bottom, hsla(92,81%,16%,0),

`to bottom, ` can be omitted.
Comment 5 Jared Wein [:jaws] (please needinfo? me) 2012-04-23 15:18:16 PDT
Created attachment 617667 [details] [diff] [review]
Patch for bug (Windows, Linux, OS X)

.85em looks too small for Windows, so I stuck with .9em on Windows (which will make it look the same as on Mac).
Comment 6 Dão Gottwald [:dao] 2012-04-23 15:39:38 PDT
Can I have some context here? I vaguely remember that we used to have smaller and/or bolder text but got rid of it in an attempt to streamline this. Also note that bold text rendering is different across operating systems; this isn't going to look like attachment 617642 [details] on Windows and Linux.
Comment 7 Jared Wein [:jaws] (please needinfo? me) 2012-04-23 15:45:26 PDT
Created attachment 617680 [details] [diff] [review]
Patch for bug v2 (Windows, Linux, OS X)

This patch has the same font-size for the identity-box on all platforms as Frank requested offline.
Comment 8 Frank Yan (:fryn) 2012-04-23 15:55:35 PDT
Comment on attachment 617680 [details] [diff] [review]
Patch for bug v2 (Windows, Linux, OS X)

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

The gradient pseudo-element's height is derived from the font-size, but I'm okay with that, since it's just a separator, sorta like a pipe character.

We're gonna need a peer to r or rs this.
Comment 9 Jared Wein [:jaws] (please needinfo? me) 2012-05-03 18:20:45 PDT
Created attachment 620917 [details] [diff] [review]
Patch for bug v3 (Windows, Linux, and OS X)

This patch removes the bolding of the text due to platform differences and the way that bold fonts get aliased. I also lightened up the green font color so that it is not as close to black.
Comment 10 Jared Wein [:jaws] (please needinfo? me) 2012-05-03 18:21:21 PDT
Created attachment 620918 [details]
Screenshot of patch
Comment 11 Dão Gottwald [:dao] 2012-05-03 19:35:13 PDT
Comment on attachment 620917 [details] [diff] [review]
Patch for bug v3 (Windows, Linux, and OS X)

Please set the background on #identity-box without :after.
Comment 12 Jared Wein [:jaws] (please needinfo? me) 2012-05-03 21:19:22 PDT
Created attachment 620949 [details] [diff] [review]
Patch for bug v4 (Windows, Linux, and OS X)

No longer using :after for the background-image.
Comment 13 Dão Gottwald [:dao] 2012-05-04 05:09:44 PDT
Comment on attachment 620949 [details] [diff] [review]
Patch for bug v4 (Windows, Linux, and OS X)

You're making the text brighter on Windows on Mac but not on Linux, where it was already darker anyway -- this doesn't seem right.
Comment 14 Jared Wein [:jaws] (please needinfo? me) 2012-05-04 07:52:30 PDT
Created attachment 621051 [details] [diff] [review]
Patch for bug v5

Increased the brightness on the text for Linux.
Comment 15 Dão Gottwald [:dao] 2012-05-04 09:33:05 PDT
(In reply to Jared Wein [:jaws] from comment #14)
> Created attachment 621051 [details] [diff] [review]
> Patch for bug v5
> 
> Increased the brightness on the text for Linux.

It's still less saturated and less bright than on Windows and Mac. Is this intentional?
Comment 16 Jared Wein [:jaws] (please needinfo? me) 2012-05-04 12:57:53 PDT
Created attachment 621142 [details] [diff] [review]
Patch for bug v6

The colors were different before but I talked with Stephen and he said to make them all the same (color: hsl(92,100%,30%)).
Comment 17 Jared Wein [:jaws] (please needinfo? me) 2012-05-07 10:20:14 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/0de23000ee3d
Comment 18 Jared Wein [:jaws] (please needinfo? me) 2012-05-07 10:46:39 PDT
Comment on attachment 621142 [details] [diff] [review]
Patch for bug v6

[Approval Request Comment]
Regression caused by (bug #): bug 742419
User impact if declined:
Slightly altered look of identity-box between Fx14 & Fx15. The identity-box got a makeover for Fx14 and since this is security-sensitive UI, we want to have the ability to tell a clear message about how to use it.

Testing completed (on m-c, etc.): locally, just landed on mozilla-inbound for Fx15
Risk to taking this patch (and alternatives if risky): none expected
String changes made by this patch: none
Comment 19 Ryan VanderMeulen [:RyanVM] 2012-05-07 18:10:39 PDT
http://hg.mozilla.org/mozilla-central/rev/0de23000ee3d
Comment 20 Jared Wein [:jaws] (please needinfo? me) 2012-05-12 16:08:34 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/eff91fd2ec87

Note You need to log in before you can comment on or make changes to this bug.