406321 - Identity portion of urlbar doesn't turn green with EV Certificates under win32 and Linux

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P3)

Description

[Bill Gianopoulos [:WG9s]]

Like it or not, I think we are going to be stuck with the green bar for EV Certificates.

I think the CAs have publicized the green bar enough that it is kind of too late to do something different.

I think something like the way it displays if you add the following to userChrome.css is what is probably needed.

#identity-box.verifiedIdentity {
  -moz-appearance: none !important;
  background-color: #AFA !important;
}

Comment 1

[Reed Loden [:reed]]

http://blog.johnath.com/index.php/2007/06/04/will-firefox-have-a-green-bar/

Comment 2

[Bill Gianopoulos [:WG9s]]

(In reply to comment #1)
> http://blog.johnath.com/index.php/2007/06/04/will-firefox-have-a-green-bar/
> 
Well, that notwithstanding, what prompted me to file this was Johnathan's comment in bug 404592 on 2007-11-29 which follows:

The extension is indeed out of date at this point - it was a convenient test
bed but the active code is now on trunk.  On the other hand, you absolutely
will get the green background, and the Company Name (Country) treatment for
_EV_ certificates, but neither of those for basic DV SSL.

Comment 3

[Johnathan Nightingale [:johnath]]

So Bill, in your initial comments, you describe a solution that would turn the address bar green.  We're not doing that, so if that is the intent of this bug, then I can mark it WONTFIX.

In your reply to reed, you indicate that you meant to refer to the identity box, which already does turn Green, with Company Name (Country) text on EV sites.  This code exists and works (though see below) on current trunk, except that there are currently no EV roots in our store.  The instructions in bug 404592 describe how to enable the Verisign root for testing purposes, at which point sites like paypal, britishairways, ebay, and schwab should show the intended visual treatment.  If this is the intent of the report, then I will mark it WORKSFORME.

In the same bug, Kai described a platform-specific issue where his identity box was being styled grey, instead of green.  This occurs on Vista as well, and is a result of the changes made in bug 395248 which style the button as a toolbox.  We can morph this bug into one which tracks that problem, but I'd like to know which problem you're describing, first.

Comment 4

[Bill Gianopoulos [:WG9s]]

(In reply to comment #3)
> So Bill, in your initial comments, you describe a solution that would turn the
> address bar green.  We're not doing that, so if that is the intent of this bug,
> then I can mark it WONTFIX.
> 
> In your reply to reed, you indicate that you meant to refer to the identity
> box, which already does turn Green, with Company Name (Country) text on EV
> sites.  This code exists and works (though see below) on current trunk, except
> that there are currently no EV roots in our store.

In my initial comments I suggested a css fix to turn the identity portion of the address  bar green leaving the URL portion of it yellow.  My feeling is this indicates that identity has been verified but does not turn the URL portion green as the existance of an EV cert says nothing about the safety of the site.

In current trunk builds this does not happen.  The only thing that appears green is the larry information but you have to click on something to see that.

If it is the intention that nothing is green unless you click someplace, then I guess you can wontfix, if you are actually seeing something green with out having to click anywhere then you must have some patch in your tree that is not in CVS, or changes in your userChrome.css file.

Comment 5

[Johnathan Nightingale [:johnath]]

It sounds like you are describing my third scenario, where the styling from bug 395248 causes you not to see the styling.  Are you on a windows/linux platform?

Comment 6

[Bill Gianopoulos [:WG9s]]

Yes, this is on both windows and linux.

Comment 7

[Bill Gianopoulos [:WG9s]]

(In reply to comment #5)
> It sounds like you are describing my third scenario, where the styling from bug
> 395248 causes you not to see the styling.  Are you on a windows/linux platform?
> 

Yes that is exactly the issue.  That is why the css code I added both changed the background color and removes the styling for the case where it is an EV Cert.  I can find no simple way to make it look like a button and be green.

I left the styling for the non EV case so it is still more obvious that the icon is a clickable button.

I can make this into a patch for winstripe/gnomestripe, but I suppose you could do this too.  My personal opinion is that the color is more important than the styling here.

Comment 8

[Bill Gianopoulos [:WG9s]]

For windows how about something like this?

#identity-box.verifiedIdentity {
  background-color: #AFA !important;
  -moz-appearance: none !important;  
}

#identity-box.verifiedIdentity:hover {
  -moz-appearance: toolbox !important;
}

This makes it green but becomes button like when you hover.

Comment 9

[Bill Gianopoulos [:WG9s]]

For Linux, which uses different styling, it appears all that is required is this:

#identity-box.verifiedIdentity {
  background-color: #AFA !important;
}

Comment 10

[Bill Gianopoulos [:WG9s]]

Attachment: patch v1

This patch implements the CSS from comment #8 and comment #9.

Comment 11

[Bill Gianopoulos [:WG9s]]

Attachment: Linux Screenshot

Screenshots to show effect of this patch.

Comment 12

[Bill Gianopoulos [:WG9s]]

Attachment: Windows screenshot

Comment 13

[Bill Gianopoulos [:WG9s]]

Attachment: Windows screenshot before patch v2

Comment 14

[Bill Gianopoulos [:WG9s]]

(In reply to comment #10)
> This patch implements the CSS from comment #8 and comment #9.
> 
I meant to mention that in looking at the pinstripe theme I noticed that the green background color was changed from #AFA to #BFA, so I echoed that change in the patch.

Sorry for all the bugspam.

Comment 15

[Bill Gianopoulos [:WG9s]]

Attachment: patch v2

This patch creates the button appearance using standard CSS elements (inset/outset borders).

This has the following advantages:

 1.  You can make the button be greeen.
 2.  The button background in the text area is a solid color making
     the text easier to read if we go to the 90%+bold font as suggested
     in bug 406612.
 3.  The look can be the same cross platform (Johnathan was keen to have this)

Disadvantage:

 1.  The button does not look as pretty.

This patch only affects Linux and Windows.  I made no atempt to modify pinstripe as I do not have a Mac.

Comment 16

[Bill Gianopoulos [:WG9s]]

Attachment: windows screenshot with patch v2

Comment 17

[Michael Monreal [:micmon]]

Attachment: windows screenshot with patch v2

Patch v2 looks bad on windows and horrible on linux as you can see. The former patch looked more natural and overall much better.

Comment 18

[Dão Gottwald [:dao]]

Attachment: counterdraft (screenshot WinXP/Luna)

(In reply to comment #7)
> (In reply to comment #5)
> > It sounds like you are describing my third scenario, where the styling from bug
> > 395248 causes you not to see the styling.  Are you on a windows/linux platform?
> > 
> 
> Yes that is exactly the issue.  That is why the css code I added both changed
> the background color and removes the styling for the case where it is an EV
> Cert.  I can find no simple way to make it look like a button and be green.

Here it is, using |background-color: rgba(0, 255, 0, 0.25)|.

Comment 19

[Michael Monreal [:micmon]]

Attachment: counterdraft / linux

For reference, the same on linux.

Comment 20

[Bill Gianopoulos [:WG9s]]

(In reply to comment #17)
> Created an attachment (id=291659) [details]
>  windows screenshot with patch v2  
> 
> Patch v2 looks bad on windows and horrible on linux as you can see. The former
> patch looked more natural and overall much better.
> 

Well, that was my opinion as well, but the Firefox UI folk seemed kind of sold on the make it look like a button idea.

Comment 21

[Michael Monreal [:micmon]]

(In reply to comment #20)
> Well, that was my opinion as well, but the Firefox UI folk seemed kind of sold
> on the make it look like a button idea.

I'm sure they didn't realize it would look like that... Worst thing is that it doesn't even look like a "button" with the inset/outset stuff

Comment 22

[Michael Monreal [:micmon]]

If you really need to make it more obvious that you can click on it to get the identity menu, you could add a \/ arrow like search has. Then again, you could also add this to the feeds button...

Comment 23

[Bill Gianopoulos [:WG9s]]

Attachment: patch v3

Well I guess I should never assume anything.

I did not think a 1px outset border would appear clickable.  It does, and is far less ugly.

Comment 24

[Bill Gianopoulos [:WG9s]]

(In reply to comment #22)
> If you really need to make it more obvious that you can click on it to get the
> identity menu, you could add a \/ arrow like search has. Then again, you could
> also add this to the feeds button...
> 
Well, that might make sense if it produced an identity menu.  Since it does not the \/ is not the correct solution.

Comment 25

[Bill Gianopoulos [:WG9s]]

Attachment: Windows screenshot w patch v3 and EV Cert

Comment 26

[Bill Gianopoulos [:WG9s]]

Attachment: Windows screenshot w patch v3 and no EV Cert

Comment 27

[Michael Monreal [:micmon]]

This won't work for linux and mac, as the url bar is rounded here and putting a square into that looks bad. 

If you compare an actual button on the toolbar with the identity "button", there is no telling that it is supposed to be a button, even on windows, as you screenshots show.

Comment 28

[Dão Gottwald [:dao]]

And your "button" doesn't match the search engine button.

Comment 29

[Dão Gottwald [:dao]]

Attachment: alternative approach

This patch doesn't regress the appearance implemented in bug 395248 and bug 398020. See attachment 291661 [details].

Comment 30

[Bill Gianopoulos [:WG9s]]

(In reply to comment #29)
> Created an attachment (id=291722) [details]
> alternative approach
> 
> This patch doesn't regress the appearance implemented in bug 395248 and bug
> 398020. See attachment 291661 [details].
> 

I'm with you on this.  I am now running windows and Linux using your patch an like the way it looks much better.

I am just not 100% sure it resolves the issues in bug 395248, especially under Linux where, for some reason, it seems to look less like a button.

In any event, I unassigned the bug if you want to take it.

Comment 31

[Dão Gottwald [:dao]]

(In reply to comment #30)
> I'm with you on this.  I am now running windows and Linux using your patch an
> like the way it looks much better.

> In any event, I unassigned the bug if you want to take it.

OK. In this case, I'll cancel your ui review request as well as mine, which shouldn't be required as I'm only bringing back the green without modifying the current appearance. Some sort of feedback from beltzner would be nice, but I prefer to move on with this and potentially have to change the appearance later on.

> I am just not 100% sure it resolves the issues in bug 395248, especially under
> Linux where, for some reason, it seems to look less like a button.

This could be addressed by adding a custom gradient. The same applies to Classic Windows themes.

Comment 32

[Reed Loden [:reed]]

Checking in browser/base/content/browser.xul;
/cvsroot/mozilla/browser/base/content/browser.xul,v  <--  browser.xul
new revision: 1.399; previous revision: 1.398
done
Checking in browser/themes/gnomestripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/gnomestripe/browser/browser.css,v  <--  browser.css
new revision: 1.144; previous revision: 1.143
done
Checking in browser/themes/pinstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/pinstripe/browser/browser.css,v  <--  browser.css
new revision: 1.100; previous revision: 1.99
done
Checking in browser/themes/winstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/browser.css,v  <--  browser.css
new revision: 1.140; previous revision: 1.139
done

Comment 33

[pal-moz]

Attachment: screenshot

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007121103 Minefield/3.0b2pre ID:2007121103

not green.
see screenshot

Comment 34

[Dão Gottwald [:dao]]

Comment on attachment 292576 [details]
screenshot

EV isn't activated yet.

Comment 35

[Johnathan Nightingale [:johnath]]

This has the effect of restoring the identity label on SSL sites, which was not intended, bug 406612 is looking at options there.  I think this is because of Dao's changes to the structure of the identity-box in browser.xul.  He fixed the various theme-based browser.css selectors, but not the one in browser/base/content/ - which is how mconnor originally hid the label.  So that selector no longer matches.

I'll attach a patch in a couple minutes that fixes that selector, which we should land immediately, imo.

Comment 36

[Alfred Kayser]

Why is there now an extra hbox introduced? I think it is not needed, and just impacts Ts/Txul/Tp needlessly.
So, can the emergency patch (also) undo this hbox addition?

Comment 37

[Johnathan Nightingale [:johnath]]

Attachment: Re-hide larry

The main patch added an hbox in browser.xul, and caught all the selectors but this one, I think.

Comment 38

[Johnathan Nightingale [:johnath]]

(In reply to comment #36)
> Why is there now an extra hbox introduced? I think it is not needed, and just
> impacts Ts/Txul/Tp needlessly.
> So, can the emergency patch (also) undo this hbox addition?

The emergency patch is just a question of making the patch as-designed work properly - what you are describing sounds more like changing the approach of the patch.  By all means, if there are perf regressions, re-open or file a follow-up to fix that, I support that, but I don't want to force through a structural change on a patch that's already been reviewed and approved as part of fixing an omission.  

Comment 39

[Mike Connor [:mconnor]]

Comment on attachment 292591 [details] [diff] [review]
Re-hide larry

r+a=mconnor

Comment 40

[Johnathan Nightingale [:johnath]]

Checking in browser/base/content/browser.css;
/cvsroot/mozilla/browser/base/content/browser.css,v  <--  browser.css
new revision: 1.43; previous revision: 1.42
done

Comment 41

[Dão Gottwald [:dao]]

Alfred, the inner box is there to overlaying/colorize the background (background-color:-moz-dialog, -moz-appearance:toolbox, whatever).

Johnathan, thanks for the quick fix!

Comment 42

[Alfred Kayser]

But this could also be done directly on the <box id="identity-box" ?
I did this for my themes and this works perfectly.

Comment 43

[Dão Gottwald [:dao]]

(In reply to comment #42)
> But this could also be done directly on the <box id="identity-box" ?

#identity-box can't have two background colors or the toolbox appearance and a background color.

Comment 44

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using:

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2008012504 Minefield/3.0b3pre

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2008012504 Minefield/3.0b3pre (Proto theme)
-and-

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b3pre) Gecko/2008012504 Minefield/3.0b3pre

Litmus testcase:

https://litmus.mozilla.org/show_test.cgi?id=5151 (we'll update it once bug 406612 lands)