579547 - New About window for Firefox 4

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Alex Faaborg [:faaborg] (Firefox UX)]

This is a meta bug to track design and discussion of creating a new About window for Firefox 4.  Dependent bugs will include things like:

-a button for check for updates
-modifying the text to describe our mission and values (mentioning that this is open source software)
-including a link to our privacy policy
-general visual design, making sure the window has the Stephen Horlander level of quality :)

Comment 1

[Mike Beltzner [:beltzner, not reading bugmail]]

I'll take the design work on this, standing on the shoulders of giants (in this case, faaborg)

Comment 2

[Alex Faaborg [:faaborg] (Firefox UX)]

I would be interested to see if shorlander has any ideas as well.  The common approach of just using the app icon on white (a little too large for the window) works reasonably well, but we could probably create some better graphics (perhaps reference our in-content UI textures?)

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

Attachment: mockup v1

First pass at the revised design, taking all dependencies into account.

Major changes: 
 - no UA string anymore (can still be obtained from about:)
 - copyright notice removed
 - credits button/function removed [1]
 - language made more open sourcey
 - took a shot at a tagline
 - added help button

[1] we haven't been keeping it up to date, and about:contributors is a better way of doing this, I think, in the long run

Not final, need to tweak it a bunch I think.

@shorlander: given this set of content, can you perhaps propose a layout? I went with a slightly wider box, and left room for a graphical element, but would love your take on it

@faaborg: the tagline/language is close, but not quite right. Give me a tweak?

@margaret: if you want to start implementing, please note that the word "contributors" should link to about:contributors, "rights" to about:rights and "privacy" to the Firefox Privacy policy. To get the "Last Updated" date, use the buildID.

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

Attachment: mockup v1 (balsamiq)

Comment 5

[Stephen Horlander [:shorlander]]

(In reply to comment #3)
> @shorlander: given this set of content, can you perhaps propose a layout? I
> went with a slightly wider box, and left room for a graphical element, but
> would love your take on it

I will take a look at this over the weekend.

Comment 6

[Alex Faaborg [:faaborg] (Firefox UX)]

>@faaborg: the tagline/language is close, but not quite right. Give me a tweak?

I gave this (and two other areas of text) a lot of thought over the weekend.  I want to run it by the ux team and some people from engagement, but in general I think we might want to dial up the ideological rhetoric a bit, and say a few things that wouldn't sound right coming from our competitors.  For instance start with the line:

"Firefox exists to protect a free and open Web."

Comment 7

[:Margaret Leibovic]

We also need to incorporate the "licensing information" link somehow. Thoughts on where to put that?

Comment 8

[:Margaret Leibovic]

Attachment: basic about dialog (osx w/ firefox branding)

I have the basic functionality here. Now we just need to make it pretty!

Comment 9

[:Margaret Leibovic]

Attachment: basic about dialog (windows w/ minefield branding)

Comment 10

[:Margaret Leibovic]

Attachment: wip

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #7)
> We also need to incorporate the "licensing information" link somehow. Thoughts
> on where to put that?

Perhaps if the wee text were to read:

// Branded

Firefox and the Firefox logos are trademarks of the Mozilla Foundation. Some of the trademarks are used under license from The Charlton Company. The source code for Firefox is available under _several licenses_.

// Unbranded

The source code for Minefield is available under _several licenses_.

(In reply to comment #8)
> I have the basic functionality here. Now we just need to make it pretty!

Great work! Two comments: 
 - need to pull the version string / buildID apart and make it human readable
 - maybe we should use a link instead of a button for check for updates? the button is much more ... visible ... though.

Comment 12

[:Margaret Leibovic]

>  - need to pull the version string / buildID apart and make it human readable

Are you talking about the "version 4.0b5pre" string? I'm not sure how I would make that more human readable. Also, won't this naturally be more readable for official releases, since it will just have numbers?

Comment 13

[:Margaret Leibovic]

Attachment: wip-v2

Improved update functionality from last patch.

Comment 14

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #12)
> Are you talking about the "version 4.0b5pre" string? I'm not sure how I would
> make that more human readable. Also, won't this naturally be more readable for
> official releases, since it will just have numbers?

It will; I meant that I wanted to put emphasis on the Last Updated date, as that's more meaningful to people. I think you've got that mostly sorted in your soon-to-come update, though!

Comment 15

[:Margaret Leibovic]

Right now there's code that includes the distribution and distribution id if a
pref is set (http://mxr.mozilla.org/mozilla-central/source/browser/base/content/aboutDialog.js#45).

Kev, do you know if we need to keep this?

Comment 16

[:Margaret Leibovic]

Oops, forgot the cc. Kev, can you take a look at comment 15?

Comment 17

[Mike Beltzner [:beltzner, not reading bugmail]]

This isn't really a [meta] anymore, it's actually tracking the implementation which will take care of all of these dependencies. Also removing bug 591776 from the dependency list - it's related to, but not dependent on this fix, I think.

Comment 18

[Dave Garrett]

(In reply to comment #17)
> Also removing bug 591776 from the dependency list - it's related to,
> but not dependent on this fix, I think.

It's a soft one-way dependency: if the UA leaves the about dialog it should be put in a better user accessible place, but if it doesn't then it should still probably go to about:support too, but doesn't have to as much. These are both blocking beta6+ now so removal from the list is fine by me.

Comment 19

[Dave Garrett]

(In reply to comment #8)
> Now we just need to make it pretty!

Couple suggestions:
1) Show a little more tail. There's more room to show more of the big logo.
2) Maybe use some of the bluey background stuffs that's been showing up on various Mozilla.com sites, i.e. some variation of:
http://www.mozilla.com/img/tignish/template/background-feature.jpg

Comment 20

[Kev Needham [:kev]]

I would like to keep the content from comment #15 if we can, or find a simple way to get the "about" information from distribution.ini elsewhere to make it easy for an individual to see they're using a customized distribution without looking at http logs or the file directory.

Comment 21

[Axel Hecht [:Pike]]

Comment on attachment 470852 [details] [diff] [review]
wip-v2

+  if (um.updateCount) {
+    var update = um.getUpdateAt(0);
+    var lastUpdated = formatBuildDate(update.buildID);
+    var lastUpdatedLabel = document.documentElement.getAttribute("lastupdatedlabel");
+    document.getElementById("lastUpdated").setAttribute("value",
+                            "(" + lastUpdatedLabel + lastUpdated + ")");
+  }

This won't work for l10n, you'll need to probably make the formatting go through a .properties entry. 

Just me, if I read the code correct, "Last updated" refers to the build ID date? Confusing to me. I assume that the real build id is still accessible through about:support?

Comment 22

[:Margaret Leibovic]

(In reply to comment #21)
> Comment on attachment 470852 [details] [diff] [review]
> wip-v2
> 
> +  if (um.updateCount) {
> +    var update = um.getUpdateAt(0);
> +    var lastUpdated = formatBuildDate(update.buildID);
> +    var lastUpdatedLabel =
> document.documentElement.getAttribute("lastupdatedlabel");
> +    document.getElementById("lastUpdated").setAttribute("value",
> +                            "(" + lastUpdatedLabel + lastUpdated + ")");
> +  }
> 
> This won't work for l10n, you'll need to probably make the formatting go
> through a .properties entry. 

Okay, I'll fix that. It does look pretty hacky :)

> Just me, if I read the code correct, "Last updated" refers to the build ID
> date? Confusing to me. I assume that the real build id is still accessible
> through about:support?

Initially I implemented it as the the date when the last update was applied, but beltzner decided it would be more useful to have the build date for the last update, since that's more sensible for troubleshooting.

Comment 23

[:Margaret Leibovic]

Attachment: wip-v3 (localization fixes)

Comment 24

[Axel Hecht [:Pike]]

Comment on attachment 471308 [details] [diff] [review]
wip-v3 (localization fixes)

Looks good, just two comments for the sake of completeness.

I guess the slightly different use of trademarkInfo shouldn't kill us here, though you should probably update the localization note in http://mxr.mozilla.org/mozilla-central/source/other-licenses/branding/firefox/locales/en-US/brand.dtd#5.

I'm only a little concerned that the order of the link to about:rights and about:privacy is fixed, that could only cause an issue if there's a language in which beginning and end of a sentence have drastically different priorities than the little they have in English. Which wouldn't be all that bad, IMHO, as it's really not that much of a prioritization in English either way.

Comment 25

[:Margaret Leibovic]

Attachment: updated screenshot (osx w/ firefox branding)

Comment 26

[Alex Faaborg [:faaborg] (Firefox UX)]

Attachment: Potential about window text

Here's some different text, with the same layout as beltzner's first mockup and margaret's patch.

This text is more overtly ideological, attempting to position us relative to our competitors and accurately represent our unique perspective on critical issues such as end user rights, privacy, and net neutrality.

The alternate design is for if we absolutely must use the text "privacy policy" to link our privacy policy.

Comment 27

[Dave Garrett]

Comment on attachment 471331 [details]
Potential about window text

Both versions look beautiful! :D

I think the version with the 3 links on the bottom looks more clean as it has 2 fewer lines of text in the paragraph on the right.

Could the "Join Us" link be expanded to "Join the Mozilla team" or something, please? It looks out of place and sounds odd by itself.

Comment 28

[Alex Faaborg [:faaborg] (Firefox UX)]

Liz: regarding this attachment https://bugzilla.mozilla.org/attachment.cgi?id=471331 can you confirm if the hyperlink "ensure your rights and protect your privacy" is descriptive enough to comply with providing users access to our privacy policy?

I know Julie has some concerns, but I think this is very clearly aligned with our organization's overall values (even if in certain cases like feedback we do in fact allow users to provide information that they know will be made public).

Comment 29

[Julie Martin]

I am not sure if I'm looking at the right version but I see this:

"Firefox is free and open source software provided by the non-profit Mozilla Foundation to make the Web a better place, and made possible by our global community of contributors.
As a user you have certain rights, and are entitled to your privacy."

A few things here:  Firefox is provided by the corporation, not the foundation.  Saying users have rights and are entitled to privacy can set the wrong expectations.  Some things we don't share, some things are public and users expect that.  

We could say:

"Firefox is free and open source software provided by Mozilla Corporation to make the Web a better place and made possible by our global community of contributors.
As a user your privacy matters.  We value your privacy too.  Our approach to your privacy is explained here. [link to PP]"

Comment 30

[Alex Faaborg [:faaborg] (Firefox UX)]

Julie, my question was about this version of the text: https://bugzilla.mozilla.org/attachment.cgi?id=471331

Comment 31

[harvey anderson]

I like the version you just posted. Observations/suggestions:

-drop net neutrality language not b/c we don't support it, but b/c there's so much more complexity to it and it seems odd in this contex. Wouldn't this also be more appropriate for other content tools like first run or start page, website.  

-The messages regarding privacy and rights are somewhat obscure. If I were looking for licensing info or privacy policy, it doesn't immediately convey the message. thus, could you do something more simple like including the words/links at the bottom. 

[Licensing Information] [Privacy Policy] [About Rights]

-could you include something to the effect of: "The Mozilla Foundation is a non-profit, public interest organization committed to make the make the Web a better place." or some similar statement that got to the public interest nature of our project?

Comment 32

[Shawn Wilsher :sdwilsh]

(In reply to comment #29)
> A few things here:  Firefox is provided by the corporation, not the foundation.
>  Saying users have rights and are entitled to privacy can set the wrong
> expectations.  Some things we don't share, some things are public and users
> expect that.  
That's not always true though, right?  I'm thinking of situations where a Linux distribution ships Firefox or someone produces a branded build (no modifications).

Comment 33

[imradyurrad]

Is there really a need for the "..." after Check For Updates?

Comment 34

[Liz Compton [:liz]]

I don't have any brilliant ideas for the wording about our mission, but I agree with Harvey that the 3 links at the bottom are more helpful to users looking for those things.

What happened to the link to Help? I thought that was a good idea.

Comment 35

[imradyurrad]

(In reply to comment #34)

It's a submenu item in the FF button.
https://bug583386.bugzilla.mozilla.org/attachment.cgi?id=465479

Comment 36

[Mike Beltzner [:beltzner, not reading bugmail]]

Taz: yes, there is a need, as it launches another dialog.

Liz: we say in the about:rights infobar that Firefox is offered by the Mozilla Foundation, which is the sole owner of the Corporation and the owner of the trademark.

Alex: I think this language is overly aggressive. We need a middle ground.

Margaret & reviewers: please focus on the code for now, we'll work the strings separately :)

Comment 37

[:Margaret Leibovic]

(In reply to comment #34)
> What happened to the link to Help? I thought that was a good idea.

Faaborg, what did happen to help in the new mock-ups? See bug 591013.

Comment 38

[:Margaret Leibovic]

Do we need to have an "OK" button for windows? If so, the current design would have to change.

Comment 39

[Luke Iliffe (Harlequin99)]

(In reply to comment #22)
> Initially I implemented it as the the date when the last update was applied,
> but beltzner decided it would be more useful to have the build date for the
> last update, since that's more sensible for troubleshooting.

Would that change then necessitate a string change to avoid confusing a normal user? A user who knows they last update yesterday may be confused to see a potentially much earlier date. 

Perhaps "Firefox build date" instead?

Comment 40

[:Margaret Leibovic]

Attachment: wip-v4

Comment 41

[:Margaret Leibovic]

Attachment: wip-v4 screenshot (osx w/ firefox branding)

(Note: Don't pay too much attention to the exact language. It is still being worked out.) 

I decided not to put links in the text if they available at the bottom of the dialog because the redundancy seemed confusing.

Also, where should I get the large Firefox logo image? Should I be using an image for the "Firefox" in the upper left-hand corner, and if so, where should I get that?

Comment 42

[:Margaret Leibovic]

Attachment: patch

I did a lot of code clean-up here to get the patch into a more review-able state. Now all it should really need is some string/image/css love.

Comment 43

[Brian King [:kinger]]

Comment on attachment 471628 [details] [diff] [review]
patch

>diff --git a/browser/base/content/aboutDialog.xul b/browser/base/content/aboutDialog.xul

...

> #ifdef XP_MACOSX
>         inwindowmenu="false"
>-        buttons="extra2"
>         align="end"
> #else
>         title="&aboutDialog.title;"
>-        buttons="accept,extra2"

I know it is like that already, but I am curious why there is no window title on Mac.

Comment 44

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #43)
> I know it is like that already, but I am curious why there is no window title
> on Mac.

From a quick scan of my OSX apps, that just seems to be de rigeur.

Comment 45

[Liz Compton [:liz]]

Back to the help question - maybe a 4th button could be added at the bottom, either "Help" or "Technical Support."

Comment 46

[Alex Faaborg [:faaborg] (Firefox UX)]

Attachment: Revised text

revised text, first paragraph is about Firefox helping the world, second paragraph is about Firefox helping you personally:

--------------------
Firefox is created by Mozilla, a global community of contributors who are working together to build a better Internet.  Firefox exists to defend the open Web so that it remains a public resource for the free expression of ideas and unrestricted technological innovation.

As a non-profit organization we are leading a global movement to empower people everywhere with the tools necessary to take control of their online lives.  We believe in ensuring your rights and protecting your privacy.

Join Us
--------------------

Comment 47

[Liz Compton [:liz]]

My suggestion for a slightly modified version. I think it's more accurate to say Mozilla exists... rather than Firefox exists... I removed the last sentence. Since we're going to have the buttons at the bottom, I don't think it's needed and we had some concerns about the wording.

-------------------------------------------
Firefox is created by Mozilla, a global community of contributors who are
working together to build a better Internet.  

Mozilla exists to defend the open Web so that it remains a public resource for the free expression of ideas and unrestricted technological innovation. As a non-profit organization we are leading a global movement to empower people
everywhere with the tools necessary to take control of their online lives.

Join Us
----------------------------------

Comment 48

[Mike Beltzner [:beltzner, not reading bugmail]]

(Adding Chelsea; if we're starting to reference/link to mozilla.org here, we should get their lead messaging person in on the mix!)

Comments on the text, which I largely like in the form indicated in comment 47:

 - assuming "Mozilla" will link to www.mozilla.org
 - assuming "Join Us" will link to www.mozilla.org/contribute
 - doesn't mention that Firefox is free
 - doesn't mention that Firefox is open
 - doesn't match against mission perfectly

Addressing those comments, and drawing text from www.mozilla.org

-------------------------------------

Firefox is designed and built by _Mozilla_, a global community of contributors working together to make a the Internet better. Firefox is completely free and open. You can _use it_ in any way you want.

_Mozilla_ believes that the Internet should be open, public, and accessible by everyone without restriction. As a non-profit organization we define success as building a better communities and a better Internet, not benefiting our shareholders. 

Sound interesting? Why not _get involved_?

---------------------------------------

where,
  _Mozilla_ == www.mozilla.org
  _use it_ == about:rights
  _get involved_ == www.mozilla.org/contribute

I like the idea of having the links across the bottom as indicated in attachment 472890 [details] - let's definitely do that.

I also like the reduced visual weight (small text, lighter colour) for the trademark notification.

@Chelsea: you OK with this?

@Margaret: I think we can drop the requirement for a link to "Help" from this window (will update bug 591013) since this is already in the "Help" menu for most systems (OSX being the exception, what can you do) and it no longer really fits with the design.

Comment 49

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #48)
some edits, in the glaring light of the morning:

---------------------------

This software is completely free and open, you can _use it_ however you want.

Firefox is designed by _Mozilla_, a global community working together to make the Internet better. We believe that the Internet should be open, public, and accessible by everyone without restriction. 

Sound interesting? Why not _get involved_?

---------------------------

 - removed bit about shareholders, as it is misleading without the subsequent (in fact we don't have any shareholders!)
 - shortened, because brevity is the soul of wit, and German is the source of long strings
 - unburied the lede

As an added bonus, we can make the unbranded version the exact same as this one, and just drop the text about trademark attributions.

Comment 50

[Chelsea Novak [:chelsea]]

The links are pointing to the right place from the Foundation perspective (moz.org and /contribute) and the copy is fine as well (thank you for removing shareholders - there be dragons).

Comment 51

[Mike Beltzner [:beltzner, not reading bugmail]]

The first line here should probably match bug 571584, so adding that dependency.

Comment 52

[Chris J]

is there any reason why the About window needs to be a dialog box? Why couldn't it just open a mozilla.org webpage in a new tab, similar to the first run or start or Help page? 

That way there'd be much more space for content, it would fit with the rest of Firefox, and it would be part of the web.

Comment 53

[Mike Beltzner [:beltzner, not reading bugmail]]

Moving this from a dialog box to a tab is out of scope for this release.

Comment 54

[Mike Beltzner [:beltzner, not reading bugmail]]

Based on today's meeting, text for the left hand side of the dialog:

---------------------------

Firefox is designed by _Mozilla_, a global community working together to make
the Internet better. We believe that the Internet should be open, public, and
accessible by everyone without any restrictions.

Sound interesting? _Get involved_!

---------------------------

Short and sweet.

Comment 55

[:Margaret Leibovic]

Attachment: patch (with final strings)

Waiting on shorlander for design work, but this patch has the text from comment 54.

Comment 56

[Marco Bonardo [:mak]]

> Major changes: 
>  - credits button/function removed [1]

> [1] we haven't been keeping it up to date, and about:contributors is a better
> way of doing this, I think, in the long run

fwiw, if you are referring to about:credits, it is far less updated than the credits in the about dialog, just look around for some name like dietrich, me, unfocused, shawn, and so on

Comment 57

[Marco Bonardo [:mak]]

I've filed Bug 594788 for the about:credits issue.

Comment 58

[Stephen Horlander [:shorlander]]

Attachment: About Layout and Visual Style Ideas

(In reply to comment #55)
> Waiting on shorlander for design work

Did a few layouts and tried to incorporate our proposed in-content style (http://blog.stephenhorlander.com/2010/06/01/in-content-ui-visual-unification/) as suggested by faaborg in comment 2. Although basic white works as well.

I think the current layout is good I would just suggest a few tweaks.

General:
* Firefox/Minefield icon should go on the left. Aside from being mostly standard having it on the wrong side of the text alignment looks odd.

* It would be nice to get the trademark text out of the way at the bottom. Not sure if there is some legal requirement about having it in close proximity to the logo?

Specific:
A) I think this works the best. Large icon on the left, main text on the right, links along the bottom and the trademark text out of the way at the very bottom. Slightly shorter window.

B) Same as A with the trademark text under the smaller logo.

C) Stacked text and links vertically. I like the extra links in a horizontal space at the bottom but if they are being localized is that enough space?

D) Smaller window with a less dominant icon.

E) Cropped icon and segmented elements (icon/wordmark/update) (about text) (links) (trademark stuff)

F) Mostly a throw away :) Centered layout.

Comment 59

[Liz Compton [:liz]]

The trademark statement doesn't need to be in close proximity to the logo, but it does need to be legible, so shouldn't be too faint to easily read.

Comment 60

[Mike Beltzner [:beltzner, not reading bugmail]]

Nice work, Stephen. I think A or C work best. So if your preference is for A, let's do that. As Liz said, no requirement for the trademark statement to be near the logo, and I think there's enough room for localization.

Margaret: the only differences between branded and unbranded builds should be:

branded = use Firefox logo, include trademark statement
unbranded = use Minefield logo, do not include trademark statement

(of course, %brandShortName should take care of the Firefox/Minefield thing)

Comment 61

[Dave Garrett]

A+ for A! :)

Typos: v.4.06bpre used instead of v.4.0b6pre, not that it matters in a mockup.

Question however, do we really need or want the leading "v."?

Comment 62

[Vlado Valastiak [:wladow]]

(In reply to comment #61)
> Question however, do we really need or want the leading "v."?
If we do, please make it localizable. "v.4.0b6" doesn't work for us, but "v4.06" does.

Comment 63

[Dave Garrett]

I personally think the leading "v." is unnecessary and could confuse users who already have trouble understanding version numbers. I suggest dropping it please.

Comment 64

[Mike Beltzner [:beltzner, not reading bugmail]]

Happy to drop the "v", sure.

Comment 65

[Dave Garrett]

An explicit usage of the word "version" would make sense. Would probably want to bump the "last updated" parenthetical down to a new line to fit it cleanly, however, just the plain version number is probably also fine.

Comment 66

[:Margaret Leibovic]

Attachment: patch (with updated styling)

Comment 67

[:Margaret Leibovic]

Attachment: screenshot (firefox, osx)

Comment 68

[Liz Compton [:liz]]

Can you make the trademark statement a shade darker?

Comment 69

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #65)
> An explicit usage of the word "version" would make sense. Would probably want
> to bump the "last updated" parenthetical down to a new line to fit it cleanly,
> however, just the plain version number is probably also fine.

No, thank you. We'll drop the "v" and leave "Last Updated" where it is.

(In reply to comment #68)
> Can you make the trademark statement a shade darker?

If we must. Is that aesthetic advice or legal advice? I'm pretty happy with it as it is. Noticable, but not at all emphasized.

Comment 70

[Dave Garrett]

(In reply to comment #69)
> No, thank you. We'll drop the "v" and leave "Last Updated" where it is.

Sounds good to me.

> (In reply to comment #68)
> > Can you make the trademark statement a shade darker?
> 
> If we must. Is that aesthetic advice or legal advice? I'm pretty happy with it
> as it is. Noticable, but not at all emphasized.

It's basically required clutter; keeping it light is better looking, in my opinion.

Comment 71

[Phil Ringnalda (:philor)]

Margaret: sorry nobody answered it earlier, but the answer to your bug 585475 comment 12 question is pretty much "um, yeah, you're going to have to leave the menuitem around, in a Mac ifdef, and leave all the code around too."

The way the Widget: Cocoa menuitem moving works is that it finds the existing menuitem in the wrong menu, by id, hides it, remembers what the click action is, and then when the one it puts in the Firefox menu gets clicked, calls that action. So there has to be a menuitem with the id "checkForUpdates" or there will be no update menuitem in the Firefox menu, and there has to be a checkForUpdates() in scope for it to call.

Also, you're leaving some orphan CSS, in browser.css's #checkForUpdates[loading="true"].

Comment 72

[Alex Faaborg [:faaborg] (Firefox UX)]

We're being careful not to introduce a color profile right?  The screenshot has the firefox looking sunburned, but I'm assuming that's because the screenshot itself has the device profile of Margaret's laptop LCD.

Comment 73

[Stephen Horlander [:shorlander]]

The PNG in attachment 473676 [details] has a color profile. The supplied icon did not.

Comment 74

[Markus Amalthea Magnuson]

What are you opinions on the fact that the Apple HIG explicitly recommends against text links in about windows?

"If you want to give the user a convenient way to visit your website or contact your company from your About window, be sure to create buttons that accomplish these tasks. Do not provide a clickable URL or email address because it is not necessarily clear that there is an action associated with it."

http://developer.apple.com/library/mac/#documentation/UserExperience/Conceptual/AppleHIGuidelines/XHIGWindows/XHIGWindows.html#//apple_ref/doc/uid/20000961-DontLinkElementID_1812

Comment 75

[Dave Garrett]

(In reply to comment #74)
Standard styled hyperlinks in a browser's dialog are quite clear. ;)

Comment 76

[Gordon P. Hemsley [:GPHemsley]]

(In reply to comment #66)
> Created attachment 473674 [details] [diff] [review]
> patch (with updated styling)

I am of the opinion that the "Get Involved!" link should include the exclamation point.

I also think that "by" (after "accessible") should be changed to "to":
* open to everyone
* public to everyone
* accessible to everyone

And perhaps a comma after "everyone"?

(In reply to comment #69)
> (In reply to comment #65)
> > An explicit usage of the word "version" would make sense. Would probably want
> > to bump the "last updated" parenthetical down to a new line to fit it cleanly,
> > however, just the plain version number is probably also fine.
> 
> No, thank you. We'll drop the "v" and leave "Last Updated" where it is.

I second the proposal to move "Last Updated" down to its own line. I also think that it should lose the parentheses, gain a colon, and be more associated with the "Check for Updates..." button than the version number.

It might also be wise to include the build date/ID in there somewhere. (Unless I'm misinterpreting what the "Last Updated" date represents?)

Comment 77

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #76)
> I am of the opinion that the "Get Involved!" link should include the
> exclamation point.

I can't say that I hold a strong opinion either way; up to Margaret.

> I also think that "by" (after "accessible") should be changed to "to":
> * open to everyone
> * public to everyone
> * accessible to everyone

Good change; Margaret, please make it so.

> And perhaps a comma after "everyone"?

No, that would be a comma splice.

> I second the proposal to move "Last Updated" down to its own line. I also 

The answer was no before, and remains no.

However, on looking at it again, the "Last Updated" text isn't really needed. Margaret, sorry for the last minute change(s) but it should read:

Firefox
4.0.4 (released March 14, 2011)

> It might also be wise to include the build date/ID in there somewhere. (Unless
> I'm misinterpreting what the "Last Updated" date represents?)

The answer was no before, and remains no.

Comment 78

[Gordon P. Hemsley [:GPHemsley]]

(In reply to comment #77)
> > And perhaps a comma after "everyone"?
> 
> No, that would be a comma splice.

No it wouldn't. Comma splices join two otherwise complete sentences (independent clauses) with a comma, creating a run-on.

The comma in question here would properly delineate a parenthetical clause. (And I'm changing my suggestion from a question to a request.)

> > I second the proposal to move "Last Updated" down to its own line. I also 
> 
> The answer was no before, and remains no.
> 
> However, on looking at it again, the "Last Updated" text isn't really needed.
> Margaret, sorry for the last minute change(s) but it should read:
> 
> Firefox
> 4.0.4 (released March 14, 2011)
> 
> > It might also be wise to include the build date/ID in there somewhere. (Unless
> > I'm misinterpreting what the "Last Updated" date represents?)
> 
> The answer was no before, and remains no.

I do much like better the use of "released", especially with the lowercase "r". But I wonder: Why are the other suggested changes so steadfastly "no"?

Comment 79

[Luke Iliffe (Harlequin99)]

Are we going to stick with the string 'Last updated' even though it will display the build date, not the date the update was applied?

Comment 80

[Mike Beltzner [:beltzner, not reading bugmail]]

Luke, please read the previous bug comments (or at least the past two or three!) before commenting.

Comment 81

[:Margaret Leibovic]

Attachment: patch v4

I addressed comment 71 by putting back the "Check for updates..." help menuitem with a Mac ifdef around it. I also put the utility functions back in utilityOverlay.js.

I also responded to some of the string feedback (I'll attach a screenshot).

Comment 82

[:Margaret Leibovic]

Attachment: patch v4 screenshot

Comment 83

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 474736 [details]
patch v4 screenshot

This is fine from a UX perspective on strings and layout. No more changes needed or will be accepted.

Comment 84

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 474735 [details] [diff] [review]
patch v4

General comments:
- Also need to remove (credits.xhtml)
  - talked to beltzner, probably need to figure out some way to expose about:credits (seems to have gotten lost along the way in string changes)
- about-logo.png and about-wordmark.png need to be added to "unofficial" branding (need non-bomb versions of the minefield images)
- tabbing through links causes them to jump around (:active or :focused styling messed up?)
- the addition of "aboutDialog" prefix to all IDs seems unnecessary (and redundant)

>diff --git a/browser/base/content/aboutDialog.js b/browser/base/content/aboutDialog.js

> function init(aEvent)

>-  var prefs = Components.classes["@mozilla.org/preferences-service;1"]
>-                        .getService(Components.interfaces.nsIPrefBranch);
>-
>+  var prefs = Components.classes["@mozilla.org/preferences-service;1"].
>+              getService(Components.interfaces.nsIPrefBranch);

If you're going to change this, might as well change it to use Services.jsm.

> #ifdef XP_MACOSX
>   // it may not be sized at this point, and we need its width to calculate its position
>   window.sizeToContent();
>   window.moveTo((screen.availWidth / 2) - (window.outerWidth / 2), screen.availHeight / 5);
> #endif
>+
>+#ifdef MOZ_UPDATER
>+  initUpdates();
>+#endif

Seems like this should happen before the sizeToContent() call above.

>+function initUpdates()

Hmm, would be really nice to share this code with the code in buildHelpMenu that is was copied from... Can you refactor to make that possible?

>diff --git a/browser/base/content/aboutDialog.xul b/browser/base/content/aboutDialog.xul

> <dialog xmlns:html="http://www.w3.org/1999/xhtml"

> #ifdef XP_MACOSX

>         align="end"

I think this is no longer needed.

>+        buttons=",">

Hmm, this is a bit strange. I wonder whether this should actually be a <dialog> at all... Does buttons="" work (I think it should).

>+        <description id="aboutDialog_version">
>+#expand <label class="version-label" value="__MOZ_APP_VERSION__"/> <label class="version-label" id="aboutDialog_released"/>

Not sure what the l10n implications are of doing hardcoding this order. I think you should have a single "version" label that is programmatically set, and modify the string to include the version:

aboutVersion=%1$S (released %2$S) (include an L10N note explaining the values of the variables)

You can use Services.appinfo.version (I confirmed that it's always the same as __MOZ_APP_VERSION__), and remove the MOZ_APP_VERSION define in /browser/base/Makefile.in.

>+        <label id="aboutDialog_distribution"/>
>+        <label id="aboutDialog_distributionId"/>

Have you tested what things look like with these set?

>+#ifdef MOZ_UPDATER
>+        <hbox>
>+          <button id="aboutDialog_checkForUpdates" oncommand="checkForUpdates();" align="start"/>

This only works on Mac, because only it includes browserMountPoints (which includes utilityOverlay.js). I guess you could unconditionally include utilityOverlay, but maybe it would be cleaner to move the update-related code into a JS module that both can use.

>+        <description class="text-blurb">
>+          &contribute.start;<label class="text-link" href="http://www.mozilla.org/contribute">&contribute.getInvolvedLink;</label>
>+        </description>

Probably would be a good idea to provide a contribute.end (even if it's empty in en-US), in case some locales require the ability to put non-linked text after the link.

>+        <label class="text-link bottom-link" href="http://www.mozilla.com/legal/privacy/">&bottomLinks.privacy;</label>    

nit: remove trailing whitespace here

>+  <stringbundleset id="aboutDialog_stringbundleset">
>+    <stringbundle id="aboutDialog_browserBundle" src="chrome://browser/locale/browser.properties"/>
>+  </stringbundleset>

Just use Services.strings.createBundle in the JS (less XBL overhead - might not be relevant if you refactor the update code).

>diff --git a/browser/base/content/baseMenuOverlay.xul b/browser/base/content/baseMenuOverlay.xul

>+#ifdef XP_MACOSX
>         <menuseparator id="updateSeparator"/>
> #ifdef MOZ_UPDATER
>         <menuitem id="checkForUpdates"
>                   label="&updateCmd.label;"
>                   class="menuitem-iconic"
>                   oncommand="checkForUpdates();"/>
> #endif
>+#endif
>         <menuseparator id="aboutSeparator"/>

Looks like this has an existing bug (will show two separators when MOZ_UPDATER isn't defined) - fix it while you're here by moving the MOZ_UPDATER ifdef up one line?

Comment 85

[Axel Hecht [:Pike]]

(In reply to comment #84)
...
> >+        <description id="aboutDialog_version">
> >+#expand <label class="version-label" value="__MOZ_APP_VERSION__"/> <label class="version-label" id="aboutDialog_released"/>
> 
> Not sure what the l10n implications are of doing hardcoding this order. I think
> you should have a single "version" label that is programmatically set, and
> modify the string to include the version:
> 
> aboutVersion=%1$S (released %2$S) (include an L10N note explaining the values
> of the variables)
> 
> You can use Services.appinfo.version (I confirmed that it's always the same as
> __MOZ_APP_VERSION__), and remove the MOZ_APP_VERSION define in
> /browser/base/Makefile.in.

I looked at this before, and my take away was that this is not a regression. Given that version number and release date are styled differently, putting them into one string with markup may be worse.

Comment 86

[:Margaret Leibovic]

Attachment: patch v5

Comment 87

[:Margaret Leibovic]

(In reply to comment #84)

> - Also need to remove (credits.xhtml)
>   - talked to beltzner, probably need to figure out some way to expose
> about:credits (seems to have gotten lost along the way in string changes)

Should I file a new bug for this or try to get it in this patch?

> >diff --git a/browser/base/content/aboutDialog.xul b/browser/base/content/aboutDialog.xul
> 
> > <dialog xmlns:html="http://www.w3.org/1999/xhtml"
> 
> > #ifdef XP_MACOSX
> 
> >         align="end"
> 
> I think this is no longer needed.
> 
> >+        buttons=",">
> 
> Hmm, this is a bit strange. I wonder whether this should actually be a <dialog>
> at all... Does buttons="" work (I think it should).

It's weird, but you do have to do buttons=",". I'm not sure why, but it doesn't work otherwise.

> >+        <description id="aboutDialog_version">
> >+#expand <label class="version-label" value="__MOZ_APP_VERSION__"/> <label class="version-label" id="aboutDialog_released"/>
> 
> Not sure what the l10n implications are of doing hardcoding this order. I think
> you should have a single "version" label that is programmatically set, and
> modify the string to include the version:
> 
> aboutVersion=%1$S (released %2$S) (include an L10N note explaining the values
> of the variables)
> 
> You can use Services.appinfo.version (I confirmed that it's always the same as
> __MOZ_APP_VERSION__), and remove the MOZ_APP_VERSION define in
> /browser/base/Makefile.in.

I didn't change this because of Pike's comment (comment 85).

> >+        <label id="aboutDialog_distribution"/>
> >+        <label id="aboutDialog_distributionId"/>
> 
> Have you tested what things look like with these set?

I have now! I'll attach a screenshot :)

> >+#ifdef MOZ_UPDATER
> >+        <hbox>
> >+          <button id="aboutDialog_checkForUpdates" oncommand="checkForUpdates();" align="start"/>
> 
> This only works on Mac, because only it includes browserMountPoints (which
> includes utilityOverlay.js). I guess you could unconditionally include
> utilityOverlay, but maybe it would be cleaner to move the update-related code
> into a JS module that both can use.

I just included utilityOverlay.js for now.

Comment 88

[:Margaret Leibovic]

Attachment: screenshot with distro text

Comment 89

[:Margaret Leibovic]

Attachment: patch v6

Whoops, I forgot to add the new images to the jar file in browser/branding/unofficial/content.

Comment 90

[Marco Bonardo [:mak]]

(In reply to comment #87)
> (In reply to comment #84)

> It's weird, but you do have to do buttons=",". I'm not sure why, but it doesn't
> work otherwise.

http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/dialog.xml#289

if aButtons is empty it does not parse it, it should probably check if aButtons === null || aButtons === undefined. you could probably pass a not existing string like buttons="none" and I guess that would work around the ugliness.
PRobably this should be a panel rather than a dialog though.

Comment 91

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 474912 [details] [diff] [review]
patch v6

General comments:
- about-wordmark should actually be "Mozilla Developer Preview" for "unofficial" branding (not a big deal though, can file followup)
- have you tested RTL? ForceRTL can help with that: https://addons.mozilla.org/en-US/firefox/addon/7438/
- has this had any Linux/Windows testing? I can help with that tomorrow.

>diff --git a/browser/base/content/aboutDialog.js b/browser/base/content/aboutDialog.js

This doesn't explicitly import Services.jsm, but works because of utilityOverlay. I'd still kind of like to avoid the include of utilityOverlay by factoring out the update related code from it into a JS module (in a followup), so can you just do the explicit import() here to avoid needing to make that change later (and make it more obvious that it's happening)?

>+function initUpdates()

>+  var sdf =
>+      Components.classes["@mozilla.org/intl/scriptabledateformat;1"].
>+      getService(Components.interfaces.nsIScriptableDateFormat);

Hmm, I thought these were intended to be createInstance()d, but I suppose it doesn't matter since it doesn't seem to keep any persistent state (nsScriptableDateFormat.cpp).

>+    var update = um.getUpdateAt(0);
>+    var buildDate = sdf.FormatDateTime("", sdf.dateFormatLong,
>+                                       sdf.timeFormatNone,
>+                                       update.buildID.substring(0, 4),
>+                                       update.buildID.substring(4, 6),
>+                                       update.buildID.substring(6, 8),
>+                                       update.buildID.substring(8, 10),
>+                                       update.buildID.substring(10, 12),
>+                                       update.buildID.substring(12, 14));

nit: use let instead of var, and make the variable hold the buildID rather than the update object (updateBuildID)

>diff --git a/browser/base/content/aboutDialog.xul b/browser/base/content/aboutDialog.xul

> <dialog xmlns:html="http://www.w3.org/1999/xhtml"

>+        buttons=",">

We should just make this a <window> and omit this attribute. To maintain the Esc-to-close behavior, you can add a:

<keyset>
  <key key="VK_ESCAPE" oncommand="window.close();"/>
</keyset>

>diff --git a/browser/base/content/credits.xhtml b/browser/base/content/credits.xhtml

>-    <!ENTITY % creditsDTD SYSTEM "chrome://browser/locale/credits.dtd">
>-      <img src="chrome://branding/content/aboutCredits.png" />
>-      <img src="chrome://branding/content/aboutFooter.png" />

These are all unused now.

>diff --git a/browser/base/content/utilityOverlay.js b/browser/base/content/utilityOverlay.js

>+function setupCheckForUpdates(aElement)

Just keep the parameter name as "checkForUpdates" to reduce the diff churn.

>-#else
>-  // Needed by safebrowsing for inserting its menuitem so just hide it
>-  document.getElementById("updateSeparator").hidden = true;

I was wrong about this - safeBrowsing uses updateSeparator as an anchor for positioning its menu item, so we can't remove it for no-updater builds - need to undo that ifdef change I suggested.

>+    aElement.removeAttribute("loading");

I wonder whether we should have a throbber on the button to reflect the "loading" state... Have you tested actually going through an update cycle to make sure this UI handles it all correctly (i.e. looks OK while an update is downloading)? Rob Strong can probably help with that (he has update tests set up, IIRC), or we could just be sure to test it after it lands.

>diff --git a/browser/locales/en-US/chrome/browser/aboutDialog.dtd b/browser/locales/en-US/chrome/browser/aboutDialog.dtd

>+<!ENTITY bottomLinks.license        "Licensing Information">
>+<!ENTITY bottomLinks.rights         "End User Rights">
>+<!ENTITY bottomLinks.privacy        "Privacy Policy">

Might want an L10N note explaining that these are link titles, and mentioning their targets (about:license, about:rights, etc.).

Still a couple of things I want to check, so no r+ yet, but thought I'd post these right away so that you can respond to them.

Comment 92

[:Margaret Leibovic]

(In reply to comment #91)

> - have you tested RTL? ForceRTL can help with that:
> https://addons.mozilla.org/en-US/firefox/addon/7438/

It looks good, except for the fact that the wordmark is aligned to the left instead of the right. It's a background image, so I tried setting background-position: -moz-start, but that didn't do anything. Thoughts on how to fix this?

> - has this had any Linux/Windows testing? I can help with that tomorrow.

I did some Windows testing with an earlier patch, but I haven't tested the final patch on Linux or Windows.

> >-#else
> >-  // Needed by safebrowsing for inserting its menuitem so just hide it
> >-  document.getElementById("updateSeparator").hidden = true;
> 
> I was wrong about this - safeBrowsing uses updateSeparator as an anchor for
> positioning its menu item, so we can't remove it for no-updater builds - need
> to undo that ifdef change I suggested.

Should I also keep it outside of the Mac ifdef then? I can hide it in utilityOverlay.js, but it's kind of awkward because there are two ifdefs, and I would want to hide it in the else case for both of them.

> >+    aElement.removeAttribute("loading");
> 
> I wonder whether we should have a throbber on the button to reflect the
> "loading" state... Have you tested actually going through an update cycle to
> make sure this UI handles it all correctly (i.e. looks OK while an update is
> downloading)? Rob Strong can probably help with that (he has update tests set
> up, IIRC), or we could just be sure to test it after it lands.

I did test it with a test update from Rob, and it looks right. However, the test update downloads really quickly, so I didn't see the loading state. Clicking the button brings up the update dialog, though, and that lets the user know what's going on.

Comment 93

[:Margaret Leibovic]

Attachment: patch v7

Comment 94

[:Margaret Leibovic]

Attachment: patch v8

Oops, wrong comment syntax in dtd file.

Comment 95

[Marco Bonardo [:mak]]

Comment on attachment 475027 [details] [diff] [review]
patch v8

I've tested the latest patch on Windows 7, it works and looks correct.

Just a couple comments but it's personal taste mostly:
- I find the below grey part having a too subtle difference from the top white part. On my screen it's barely noticeable.
- I find a bit strange that each link opens a new separate window, I was expecting them to open tabs in my last window. But looks like we are not the only ones doing so.

>diff --git a/browser/base/content/aboutDialog.js b/browser/base/content/aboutDialog.js

>+// Services = object with smart getters for common XPCOM services
>+Components.utils.import("resource://gre/modules/Services.jsm");

the comment does not look useful

> function init(aEvent)

>-      var distroVersion = prefs.getCharPref("distribution.version");
>-      var distroAbout = prefs.getComplexValue("distribution.about",
>+      var distroVersion = Services.prefs.getCharPref("distribution.version");
>+      var distroAbout = Services.prefs.getComplexValue("distribution.about",
>         Components.interfaces.nsISupportsString);
>   

there is some trailing space here, could be worth removing them while touching this code

>       var distroField = document.getElementById("distribution");
>       distroField.value = distroAbout;
>       distroField.style.display = "block";
>     

and here as well

>+function initUpdates()
>+  var um =
>+      Components.classes["@mozilla.org/updates/update-manager;1"].
>+      getService(Components.interfaces.nsIUpdateManager);
>+  var sdf =
>+      Components.classes["@mozilla.org/intl/scriptabledateformat;1"].
>+      getService(Components.interfaces.nsIScriptableDateFormat);

the limit is 80 chars still, and looks like these would not go over it, so why nl after the "="? it looks ugly
(ps: ah I see this code has been copied from utilityOverlay, if Gavin is not fine with fixing ugliness in both places,
I'd suggest to at least avoid importing it in new code)

>diff --git a/browser/locales/en-US/chrome/browser/aboutDialog.dtd b/browser/locales/en-US/chrome/browser/aboutDialog.dtd

>+<!ENTITY contribute.start           "Sound interesting? ">

stupid question. I'm not English so it's really a question, should not this be "Sounds interesting?"

Comment 96

[Axel Hecht [:Pike]]

Comment on attachment 475027 [details] [diff] [review]
patch v8

To copy the discussion from irc:


One thing to watch out for when testing on non-mac:

The long dateformat comment in http://mxr.mozilla.org/mozilla-central/source/intl/locale/idl/nsIScriptableDateFormat.idl#87 indicates that this may leave us with the weekday showing in the OS locale (not the fx locale) on at least some platforms. I wouldn't mind to just parse numbers out of the buildID and to use those directly.

Comment 97

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #96)
> the fx locale) on at least some platforms. I wouldn't mind to just parse
> numbers out of the buildID and to use those directly.

But I would. Do we know which locales don't have matching OS locales? Can we do something where "if we know it'll work, show month names, if not, show fugly numbers?"

Comment 98

[Axel Hecht [:Pike]]

This is about using a German build on an English OS, or vice versa.

This has nothing to do with matchOS or something.

Also, it's unclear what non-mac platforms actually return using the current code, as the comment describing the date format indicates something that the mac doesn't return (i.e., it claims to include the weekday). If that's just for datetime and not for just date, no idea. We'll have to test and see what we get.

Comment 99

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 475027 [details] [diff] [review]
patch v8

- it seems like link focus styling is broken on windows/linux
- RTL fix (based on feedback from ehsan): http://pastebin.mozilla.org/789057

>diff --git a/browser/base/content/aboutDialog.js b/browser/base/content/aboutDialog.js

>+function initUpdates()

>+    let buildID = um.getUpdateAt(0).buildID;
>+    let buildDate = sdf.FormatDateTime("", sdf.dateFormatLong,
>+                                       sdf.timeFormatNone,
>+                                       buildID.substring(0, 4),
>+                                       buildID.substring(4, 6),
>+                                       buildID.substring(6, 8),
>+                                       buildID.substring(8, 10),
>+                                       buildID.substring(10, 12),
>+                                       buildID.substring(12, 14));

We'll need to figure out a better solution here... It really sucks that nsIScriptableDateFormat is so sucky :(

>diff --git a/browser/base/content/aboutDialog.xul b/browser/base/content/aboutDialog.xul

>+  <script type="application/javascript" src="chrome://browser/content/utilityOverlay.js"/>

>+    <key key="VK_ESCAPE" oncommand="window.close();"/>

Oops, mistake in my example code - this needs to use keycode="VK_ESCAPE" to work.

>diff --git a/browser/base/content/utilityOverlay.js b/browser/base/content/utilityOverlay.js

>+function setupCheckForUpdates(checkForUpdates)

This function tries to use document.getElementById("bundle_browser"), which fails on non-Mac. I guess you should refactor it to take a reference to a stringBundle, and have the utilityOverlay caller pass strings.stringBundle (will also require some changes to getStringWithUpdateName to use the nsIStringBundle API rather than the <stringbundle> one, e.g. s/getFormattedString/formatStringFromName/)

Comment 100

[:Margaret Leibovic]

Attachment: patch v9

This should (hopefully) address all the concerns. I can file a follow-up bug for the localized "released" date.

Comment 101

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 475180 [details] [diff] [review]
patch v9

>diff --git a/browser/base/content/baseMenuOverlay.xul b/browser/base/content/baseMenuOverlay.xul

>+#ifdef XP_MACOSX
>         <menuitem id="checkForUpdates"
>+#endif

This means that you can remove the #checkForUpdates[loading="true"] rules in browser.css for winstripe/gnomestripe.

>diff --git a/browser/base/content/utilityOverlay.js b/browser/base/content/utilityOverlay.js

>+  var browserBundle = Services.strings.
>+                      createBundle("chrome://browser/locale/browser.properties");

var browserBundle = document.getElementById("bundle_browser").stringBundle;

>diff --git a/browser/locales/en-US/chrome/browser/browser.properties b/browser/locales/en-US/chrome/browser/browser.properties

>+# About Firefox Dialog
>+# LOCALIZATION NOTE (aboutdialog.released): %1$S = year, %2$S = month, 
>+# %3$S = day

Mention that these are just numbers (rather than names), and that they're zero-padded (for day/month)?

Doing it this way now removes our ability to change this after string freeze - if you kept things as just "%s = date", we could potentially change the format later. Though I guess that could potentially have some l10n impact anyways? Get Axel to make the call, I guess?

>+aboutdialog.released=(released %2$S-%3$S-%1$S)

ISO 8601 is the one true date format! (grumble, I guess this is en-*US*...)

>diff --git a/browser/branding/unofficial/locales/en-US/brand.dtd b/browser/branding/unofficial/locales/en-US/brand.dtd

> <!-- LOCALIZATION NOTE (releaseBaseURL): The about: page appends __MOZ_APP_VERSION__.html, e.g. 2.0.html -->
> <!ENTITY  releaseBaseURL        "http://www.mozilla.org/projects/devpreview/releases/">

Hate to scope creep, but can you remove this while you're at it? It's unused (was forgotten in bug 348076).

r=me with those addressed. Yay!

Comment 102

[Axel Hecht [:Pike]]

I'm not a friend of re-doing this after b7 for fx4 wrt dates. Also, we should expose the order of the date parts to l10n. No idea how we'd do this better without l10n impact, too.

I'd suggest to go with this.

Comment 103

[Mike Beltzner [:beltzner, not reading bugmail]]

You'd suggest we go with ... what?

Comment 104

[:Margaret Leibovic]

Attachment: final patch

Comment 105

[imradyurrad]

(In reply to comment #103)> You'd suggest we go with ... what?Expose the date format to 1lon.

Comment 106

[Axel Hecht [:Pike]]

Comment on attachment 475211 [details] [diff] [review]
final patch

With one like this.

Possible nit, you could just strip a leading '0' for day and month to make it look more humane.

I'm open to follow up bugs for Firefox 4.next to make our date formatting not suck, too.

Comment 107

[Johnathan Nightingale [:johnath]]

Ship it. Margaret - accurate to say this is checkin-needed now?

Comment 108

[Nicolas Mandil (:NicolasWeb)]

Following my comment 7 on bug 571424. (sorry again for dup)
It could be nice to add a link on "We believe that the internet should be open, public and accessible by everyone without any restrictions." or on a following "Learn more..." link to a web page as an entrance door to the free software/open source world.
I first think about the Organic one : http://www.mozilla.com/en/firefox/organic/ (or maybe to http://www.mozilla-europe.org/en/firefox/organic/ as it more localized, aka,try french on both, only the last one work right).

Comment 109

[Nicolas Mandil (:NicolasWeb)]

(In reply to comment #108)
My previous proposal depend on the (ln10) string freeze, so we might consider it before this deadline. (just a late but important thought)

Comment 110

[Axel Hecht [:Pike]]

I'm not in favor of creeping the scope more, also, the "organic" messaging isn't working equally well around the globe.

Comment 111

[Julie Martin]

I'm also not comfortable with the language proposed in comment 108, I think it goes too far.

Comment 112

[:Margaret Leibovic]

There's no more room for changes in the language in this bug. I would say that anyone who wants to change the language should file a follow-up bug, but a lot of discussion went into reaching the current language, so I don't think it's going to change for Firefox 4.

Comment 113

[:Margaret Leibovic]

http://hg.mozilla.org/mozilla-central/rev/a1d900b2c5a4

Comment 114

[Justin Dolske [:Dolske]]

Bustage fix: http://hg.mozilla.org/mozilla-central/rev/9a2b165409b6

This removed "about.png" from the branding dir, which is what about:logo is tied to (see nsAboutRedirector.cpp). A handful of tests all refer to this (some seem to care about it's size, sigh), and I suppose we should avoid breaking other apps, so I just relanded the old about.png images.

Honestly, I'm not even sure why about:logo is around, we should probably just kill it off at some point. But not as a bustage fix. :)

Comment 115

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #95)
> - I find the below grey part having a too subtle difference from the top white
> part. On my screen it's barely noticeable.

Yeah, probably worth a followup.

> - I find a bit strange that each link opens a new separate window

That's bug 263433.

> stupid question. I'm not English so it's really a question, should not this be
> "Sounds interesting?"

It's a contraction of "does this sound interesting?", so no :)

Comment 116

[[e-mail]]

(In reply to comment #114)
> Honestly, I'm not even sure why about:logo is around, we should probably just
> kill it off at some point. But not as a bustage fix. :)
Thunderbird and SeaMonkey still use about:logo and Sunbird and Camino used to use it although I'm not sure whether they still do (they would if they used Toolkit's about: page for instance.)

Comment 117

[Nicolas Mandil (:NicolasWeb)]

Added a following bug 596535 : Add a link in About window to make Firefox
Mobile more discoverable for Firefox Desktop users.

Comment 118

[u88484]

Verified fixed.  Looks and works great!

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20100916 Firefox/4.0b7pre

Comment 119

[Jesse Ruderman]

Attachment: screenshot with checked-in patch

Beautiful!

Comment 120

[Dave Garrett]

(In reply to comment #119)
> Created attachment 476646 [details]
> screenshot with checked-in patch
> 
> Beautiful!

Indeed, quite a wonderful improvement. :)

However, it looks like Mac may be in the Linux camp with respect to the missing released date. If so, please note it in bug 597012.

Comment 121

[[not reading bugmail]]

The trademark text doesn't show up on Windows Minefield.

Comment 122

[Josh Tumath]

(In reply to comment #121)
> The trademark text doesn't show up on Windows Minefield.

That is because the Minefield logo is not trademarked. Trunk builds are meant to be completely free, and since the Firefox logo is copyrighted...

Comment 123

[Dave Garrett]

Which means that Jesse's screenshot is an odd combination of Firefox branding with a Minefield version, by the way. (and why I'm not 100% sure if Mac gets bug 597012 or not)

Comment 124

[[not reading bugmail]]

My point is that we regressed bug 569057 with this change since it was added back to About:Dialog recently and I would agree Minefield is also not a Charlton Trademark, but Firefox is.

Comment 125

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #124)
> My point is that we regressed bug 569057 with this change since it was added
> back to About:Dialog recently and I would agree Minefield is also not a
> Charlton Trademark, but Firefox is.

I don't understand that at all. "regressed bug 569057" would mean that the Charlton text doesn't appear in Firefox-branded builds. Is that what you're saying happens? It not being displayed in Minefield builds isn't a bug, it's intentional.

Comment 126

[[not reading bugmail]]

(In reply to comment #125)
> (In reply to comment #124)
> > My point is that we regressed bug 569057 with this change since it was added
> > back to About:Dialog recently and I would agree Minefield is also not a
> > Charlton Trademark, but Firefox is.
> 
> I don't understand that at all. "regressed bug 569057" would mean that the
> Charlton text doesn't appear in Firefox-branded builds. Is that what you're
> saying happens? It not being displayed in Minefield builds isn't a bug, it's
> intentional.

Well it was in Minefield till this bug was fixed, because it keeps getting taken out.  If it shows up in the next release build then we're good, but I cannot test that. None of the screenshots or comments mention it shouldn't be build for non branded builds as bug 569057 put it back in because it keeps getting taken out.

Can you confirm the patch that went in will show up in Firefox-branded builds?

Comment 127

[:Gavin Sharp [email: [e-mail]]]

I'm pretty sure that the Charlton text was never included for Minefield builds from the trunk. (It was included for branch unbranded builds given the hack we used to add it back there, though.)

My local branded build shows it just fine: http://grab.by/6t3E

(Email me if you want to discuss this further - we don't need to continue spamming all the nice people watching this bug.)

Comment 128

[[not reading bugmail]]

(In reply to comment #127)
> I'm pretty sure that the Charlton text was never included for Minefield builds
> from the trunk. (It was included for branch unbranded builds given the hack we
> used to add it back there, though.)
 
Just wanted to clear the air here, I double checked a few builds before and after that bug, your right, sorry, I was wrong, thinking I seen it in the dialog too before with the Minefield logo.

> My local branded build shows it just fine: http://grab.by/6t3E

Thanks for the update.

Comment 129

[Matthew Turnbull [Bluefang]]

(In reply to comment #122)
> (In reply to comment #121)
> > The trademark text doesn't show up on Windows Minefield.
> 
> That is because the Minefield logo is not trademarked. Trunk builds are meant
> to be completely free, and since the Firefox logo is copyrighted...

Then the trademark area of the dialog should be collapsed, either by making the "content" longer of making the whole dialog shorter. The big, empty space at the bottom of the dialog doesn't look good, and it gives the false impression that something should be there.

Comment 130

[Johnathan Nightingale [:johnath]]

(In reply to comment #129)
> (In reply to comment #122)
> > (In reply to comment #121)
> > > The trademark text doesn't show up on Windows Minefield.
> > 
> > That is because the Minefield logo is not trademarked. Trunk builds are meant
> > to be completely free, and since the Firefox logo is copyrighted...
> 
> Then the trademark area of the dialog should be collapsed, either by making the
> "content" longer of making the whole dialog shorter. The big, empty space at
> the bottom of the dialog doesn't look good, and it gives the false impression
> that something should be there.

We've wandered beyond the meat of fixing this bug - if you have follow ups to file, by all means, but let's stop spamming the 50 people on this cc list, please?

Comment 131

[Peter Henkel [:Terepin]]

If I may ask, why bottom strip is styled in general Windows style? It breaks consistency with Win7.

Comment 132

[Alex Faaborg [:faaborg] (Firefox UX)]

We'll fix that for windows (I think that's the OS X style being carried over at the moment)

Comment 133

[Pardal Freudenthal (:ShareBird)]

Shouldn't the style rules for the about window live in the skin provider instead of content? This makes things difficult for themers, as they have to specifically override those rules.

Comment 134

[Dave Garrett]

(In reply to comment #133)
> Shouldn't the style rules for the about window live in the skin provider
> instead of content? This makes things difficult for themers, as they have to
> specifically override those rules.

The about dialog isn't supposed to be re-themed much. In fact, AMO has a policy of telling theme and extension developers to leave the about dialog alone.

Comment 135

[Pardal Freudenthal (:ShareBird)]

(In reply to comment #134)
> 
> The about dialog isn't supposed to be re-themed much. In fact, AMO has a policy
> of telling theme and extension developers to leave the about dialog alone.

I can agree with this regarding to the branding images, but not for the general appearance of it. So I think we could maintain the rules for branding styles on content and move the other rules to the skin provider.

Notice that since this window imports the global rules, it indeed is themed and follow specifically rules from the used theme like, e.g., background and font colors.

Maybe I wouldn't touch it if it doesn't break things on my theme. Just as an example, I usually have rules for buttons where I specify padding. The aboutDialog.css overrides these rules and I need to create a new global rule just for the buttons on this window. I also needed to use the important flag since increasing the specificity using a child selector didn't work on this case. I guess other themers can also experience similar annoyances.

Also leaving this rules on content doesn't really avoid the window to be themed, just makes it more difficult and at risk of breaking things on other themes than the default.

Comment 136

[Peter Henkel [:Terepin]]

I have few suggestions:
1. Can we move about window into DH pop-up, please?
2. Ca we have acces directly from FXB? Something like question mark inside FXB on the left side.