Alex Faaborg [:faaborg] (Firefox UX) Reporter
	Description • 17 years ago

We need to theme the identity information contextual dialog for XP.  Here is a set of potential styles:

http://people.mozilla.com/~faaborg/files/granParadisoUI/contextualDialogStyleXP_i1.png

Of these, we think we are going to go with the "menu" style for XP.

Note that bug 403157 is tracking the appearance of the bookmarks contextual dialog, which will use the same visual style.

	u88484
	Comment 1 • 17 years ago

I vote for "menu" style without the transparency (alpha).  No need to see through it and it becomes a distraction and a possible accessibility issue.

	Mike Beltzner [:beltzner, not reading bugmail]
	Comment 2 • 17 years ago

The /\ part of the design isn't essential, but actually styling it nicely is. Dao, have any cycles for this? mcdavis?

	Dão Gottwald [:dao]
	Comment 3 • 17 years ago

see attachment 300901 [details]

	Alex Faaborg [:faaborg] (Firefox UX) Reporter
	Comment 4 • 17 years ago

Here is a spec, this shows the bookmarking interface, but the same style applies to the identity contextual dialog.  Only difference is that it should hang to the right instead of left.

	Alex Faaborg [:faaborg] (Firefox UX) Reporter
	Comment 5 • 17 years ago

Note that the current dialog does not support high contrast mode in windows.

	Alex Faaborg [:faaborg] (Firefox UX) Reporter
	Comment 6 • 17 years ago

Right, but none of those options control the small number of things we want to use hard coded colors for.  For instance (unfortunately) there is no hyperlink color in that set of preferences, because the windows was designed in 1995.

We aren't breaking support for any of those controls, just adding to the set of things that can have a certain color.

	mcdavis941 (sporadically reading bugmail)
	Comment 7 • 17 years ago

(In reply to comment #2)
> The /\ part of the design isn't essential, but actually styling it nicely is.
> Dao, have any cycles for this? mcdavis?
> 
Sorry, I missed this comment the first time around and just noticed it a couple days ago.  I'll follow up with some userChrome.css (the format requested by Alex) and screenshots in a few minute.

	mcdavis941 (sporadically reading bugmail)
	Comment 8 • 17 years ago

As userChrome.css rather than patch, as requested by Alex.

	mcdavis941 (sporadically reading bugmail)
	Comment 10 • 17 years ago

Any thoughts on the above?

Some comments:

- the button hanging way out to the right is creating eye flow problems in the SSL and EV cases.  maybe worth trying to tuck it in under the word "eavesdropping".
- v1 unbolds the string "(unknown)" in the SSL case.  to me, when it was bolded, it was even more prominent than the site domain name (mozilla.com in the screenshot) in the popup, and made the popup hard to read.  might also be worth trying (XUL change) to present it on the same line as its label, so it reads "which is run by: (unknown)".
- it looks like we have to choose between either drop shadows or rounded corners, so v1 went with dropshadows and square corners.  this seems OK since (for XP anyway) this is consistent with other things that drop down from the toolbox. plus, the rounded corners were more important with the door hanger/raised tab concept, which isn't used here.
- TODO: RTL
- TODO: test with larger fonts

	mcdavis941 (sporadically reading bugmail)
	Comment 12 • 17 years ago

v2, adding RTL support

	Dão Gottwald [:dao]
	Comment 13 • 17 years ago

What we have right now looks better to me than attachment 314016 [details].

	Johnathan Nightingale [:johnath] Assignee
	Comment 14 • 17 years ago

Great work so far mcdavis, I think you're close to having this one solved, a couple points:

- Bug 414389 regressed us a little here, since it replaced the 18x18 icon with a 24x24 icon without adjusting the offset padding here:

http://mxr.mozilla.org/mozilla/source/browser/themes/winstripe/browser/browser.css#1879

- My preference (and I think Mike and Alex's too) is not to play with indenting the text, and to reduce the multiplicity of bolding/size combinations to just 2 - text we emphasize (domain, owner), and text we don't (everything else).  The net effect of this is to make everything left-aligned (at least, in LTR), and to give "(unknown)" the same weight as "mozilla.org" (as it is in EV, when (unknown) has actual details instead).

- One nuance I noticed in attachment 314040 [details] that I liked was that the panel's top right corner (in pic 1 of that attachment) is aligned with the inside edge of the curve, not the far left of the button as it is on trunk.  This is much nicer looking.

Do you have time to put together a patch that does things this way, based on the work you've already done?  I don't want to steal your bug if you have time to work on it, but I would like to see this make it if we can get a clean, low-risk patch together.

	Johnathan Nightingale [:johnath] Assignee
	Comment 15 • 17 years ago

This is a patch form of the changes I think we should take.  Will request review tomorrow, once I have ui-r on the outcome.

	Dão Gottwald [:dao]
	Comment 16 • 17 years ago

Comment on attachment 315852 [details] [diff] [review]
Changes as patch

>+/* Match the curvature of the #identity-box */
>+#identity-popup {
>+  -moz-margin-start: 6px;
>+}

Currently, the curve is only present in LTR mode.

	Johnathan Nightingale [:johnath] Assignee
	Comment 17 • 17 years ago

Thanks Dao.  Selecting for chromedir requires a line of XUL change as well, updated patch attached.  Comparison graphics incoming momentarily.

	Johnathan Nightingale [:johnath] Assignee
	Comment 18 • 17 years ago

Getting confirmation that this is the intended look and feel, before requesting review.

	Johnathan Nightingale [:johnath] Assignee
	Comment 19 • 17 years ago

Salient differences from current behaviour:

 - Use system menu colouring instead of hard coded white
 - Drop emphasized text to 1.2em (was 140%)
 - No emphasis for "(unknown)"
 - Better RTL handling in CSS
 - Box corners align with site button curvature (for LTR, RTL doesn't curve)
 - Thinner border on panel
 - Padlock alignment regression fixed
 - "More information" button offset slightly more from encryption text

	Johnathan Nightingale [:johnath] Assignee
	Comment 20 • 17 years ago

Comment on attachment 316016 [details] [diff] [review]
Only adjust margin on LTR, change font weight for emphasized text

Tagging gavin for review now that beltzner's ui-r'd, but would certainly appreciate any feedback from Dao or mcdavis, if they're around to see it.

	Johnathan Nightingale [:johnath] Assignee
	Comment 21 • 17 years ago

Comment on attachment 316016 [details] [diff] [review]
Only adjust margin on LTR, change font weight for emphasized text

This is marked wanted, not blocking, but beltzner asked me to see about fixing it, so I feel like he probably wants to approve the fix?

	Johnathan Nightingale [:johnath] Assignee
	Comment 23 • 17 years ago

Checking in browser/themes/winstripe/browser/browser-aero.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/browser-aero.css,v  <--  browser-aero.css
new revision: 1.2; previous revision: 1.1
done
Checking in browser/themes/winstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/browser.css,v  <--  browser.css
new revision: 1.201; previous revision: 1.200
done
Checking in browser/base/content/browser.xul;
/cvsroot/mozilla/browser/base/content/browser.xul,v  <--  browser.xul
new revision: 1.461; previous revision: 1.460
done

	Kai Liu
	Comment 24 • 17 years ago

Sorry for this late comment.  I didn't notice this bug until I used today's nightly.  I'm curious, was there any particular reason to not use -moz-appearance: menupopup for #identity-popup?  It would have produced the same* results with fewer lines of CSS.  :-)

* Well, almost the same.  The main difference would be that in classic, the borders are a classic 2px bevel, but that's the correct rendering for classic, and the reason I even decided to peek at this bug was because the 1px border on classic kinda stood out in today's build.

	Kai Liu
	Comment 25 • 17 years ago

And the screenshots of what the patch above would do.  As you can see, it will look exactly the same.  Except that the borders in classic would be, well, classic.  :)

A few things that I forgot to mention:

* Since the popup box's child container specifies a background and color, there would be no need to do that in the popup box itself, which is why I removed it in the patch above.

* Would you reconsider the margin-top: 1px?  I personally think that having the borders blend in looks pretty nice (see the 5th pane at the bottom of this attached screenshot collage)

	Johnathan Nightingale [:johnath] Assignee
	Comment 26 • 17 years ago

Kai - looks good to me!  If you can get review for it, I think we should take it.

	Kai Liu
	Comment 27 • 16 years ago

Comment on attachment 316297 [details] [diff] [review]
menupopup moz-appearance

I've posted a new patch in bug 413060 comment 10 that includes and supersedes patch that I posted in this bug--Alex wanted to try the tooltip appearance for Vista, which I think looks marvelous...