As discussed at <http://groups.google.com/group/mozilla.dev.apps.firefox/browse_frm/thread/93d5c6843e14c2fd>...
Most users don't provide enough information about their setup, and we need to fish it out of them. In many cases, we need to know what extensions are installed, and there's no way to simply copy and paste that info into a support post (They would need the info-lister extension). If I need a user to do something in their profile folder (eg. restore a bookmarks backup file), I can point them to a web page that lists the default locations, but the web page can't list the exact path for that specific user.
As a method of improving user support tools, it would be nice if we (support volunteers) could direct users to a single about: page, that listed all the info a user needs for user support; and create a "Copy my support info" button on that page.
Originally intended as a way improving the about: page, it may be better to put this on a new 'about' URL. (about:support, about:setup, etc.)
Filed under Firefox:General, but if this could be done in the Toolkit, and make it so Thunderbird users could also utilize a support info-lister, that would be twice as nice. :-)
Some info that it would include:
- version
- user-agent sting
- installed add-ons (and their version numbers)
- installed plug-ins (and their version numbers)
- profile location
- and maybe the cache location

Perhaps it's not even needed to display this stuff anywhere, for most support issues a "Copy my support info" button in the about-dialog or even a menu item with this text under Help could be enough. This would then just copy the info to the clipboard as plain text and maybe text/html.

This is something that'd be pretty simple to knock together and I tend to agree that just copying it to the clipboard in a simple text form is probably the best way to go, then users can just stick it onto pastebin or an email/forum.
I would add to the list of info the network settings including proxy.

I would still like an actual page. Using the example of teaching a user how to restore a bookmark backup, I can tell the user to the user to go to his/her profile folder, and direct them to the local support info page for the actual profile location, rather than a web site which lists the defaults. In such a case, copying/pasting text would just be an extra step.

I guess both should be provided:
- An easy "Copy support info" button in About Firefox or an item in the Help menu.
- A centralized informative page where all support related Firefox's details are displayed.
I think all customized preferences should be included as well. This covers comment 3.
Having a single page instead of several about:addons, about:plugins, about: buildconfig helps end users, while for developers its basically the same thing. Or are there extensions or any other kind of dependency on any of the currently available about:s?
This page should also have a printable format for the cases where a user can't access the web say at home, so s/he needs to take the information to a peer at work. Copied/printed sections are selectable.

Created attachment 252319[details]
About:support mockup
I dubbed it about:system, but about:support, about:setup, etc. are all good names.
As stated in my previous comment, it's intended for informative purposes and so it contains lots of information. A user can select what specific sections to share (copy/print). Includes customized preferences and build details. And simple graph of the profile size which may aid users with hard disk space management (not sure if it may grow big enough to really matter).
Plugins are clickable and expand to provided MIME details (not in this mockup).

In m.d.a.firefox, Gerv suggested about:diagnostics.
Bug 320141 looks like it is the same; but I don't want to ruin any momentum this bug may have, by resolving now. You know how Mozilla development is. Once a bug is forgotten, it's pretty tough to get it back in the limelight.

I really like Percy's mockup. Perhaps Dave's concern could be met by selecting only a few of the checkboxes at the top by default, and leave e.g. Customized Preferences and Profile deselected.
I am wondering if the full build info is really necessary. In most cases, just the build ID should be enough, right? In light of Bug 361734 we should make sure that the build ID listed on this page is actually the build ID and not a spoofed user-agent string (or list both).

In doing user support, I've never once needed to know build info, like build tools, and config arguments.
However, it is good to know when a user is spoofing their UA. That is often the cause of whatever problems they may be asking about. (For instance: bug 83376 )

Assuming we have some kind of agreement about what to include, and that this is something that would be taken then I'd be happy to take this on. Copying in Mike, perhaps he has some useful comments to bring.

I think that this is a great idea. Some initial thoughts ...
1) Something worries me in terms of both security/privacy and discoverability when I think about this being an about: page. Why not make it part of the Help dialog and actually pop a window which asks the user a minimal number of questions about the types of information they want to include, and for a destination output location (file? pastebin? copy buffer?).
2) It would be good to get information from people like Chris, Lucy, the crowd in #firefox, people on MozillaZine and other fora where Firefox Support is offered (apparently our Polish community is pretty kick-ass here) about:
- what information should always be provided
- what information should be provided on an opt-in basis
- how that information could best be packaged & shared with the troubleshooter

I'd find the following useful
-Build identifier
-Extensions, with versions and enabled/disabled status
-Plugin information
-IDs of the last X incidents sent via Talkback
-The names of the profiles, including info on which is currently in use
I don't see any particular need for customized prefs or build flags.

(In reply to comment #16)
> I'd find the following useful
> -Build identifier
> -Extensions, with versions and enabled/disabled status
> -Plugin information
> -IDs of the last X incidents sent via Talkback
> -The names of the profiles, including info on which is currently in use
>
> I don't see any particular need for customized prefs or build flags.
I agree with the above, and I would add the profile location(s).

I don't have much experience with support but I'd think that knowing if pipelining or fast page navigation or some other about:config preferences has been tweaked or not may be helpful. A user may have activated some of those settings and just forgot about it or just don't relate a problem to the change. It has happened to myself.

Such feature must be based on a dynamic mechanism. I thought it could be a good idea to have a set of "diagnostic modules" (addons, plugins, prefs, memory, ...).
Each module must provide at least a method to retrieve a "text plain report", a "short name" and a "rich view". They should be registered in a category (catManager speaking).
Then, each "diagnostic modules" could be displayed through different ways:
- a "diagnostic:" url which will display something like the mockup. And a specific diagnostic could be reached from a "diagnostic:moduleShortName" url (diagnostic:addons, diagnostic:profile, ...).
- something working like the prefPane. A diagnosticPane could host diagnostic modules through the overlay mechanism.
Then, any extensions could provide their own diagnostic module.
I think such feature must be provided through an extension first, and it's ok for me to work on it.
What do you think about such idea ?

Needs to be built-in to Firefox and available on any installation, in safe mode, without regard to any extensions installed.
Suggest:
about:profile provide live link to view profile directory in Firefox,
and a link (or text link) suitable such as for Windows Explorer
on a windows system. So user can actually work with Firefox customizations
and do their own backups in the manner that they prefer.
about:addons provide list of extensions and themes
As far as an extensions providing information:
"Infolister" extension, provides about:info (may be user customized)
"Extension Manager Extended" provides additional information and links
via context menus added within the Addons Manager.

Continuing my comment #23 for the about:profile part it should have a link
to get you into your profile so you can work with it within your system, a
working extension for Windows only, which works but is not listed as compatible is
"Open Profile Folder" extension opens the profile folder in Windows File Manager

I'm flagging this wanted-firefox3.6 because it will require string changes, but I don't think we should let it languish for another release. It's a huge force multiplier to spend a moderate amount of developer time up front in order to create a more effective support organization. I've also added it to the Firefox.next plan wiki discussion page.

From a support point of view in terms of copy pasteable info, I'd like the following:
Extensions (including if they've been disabled and version numbers)
CrashIds + Dates
In terms of helping us with basic troubleshooting, the following buttons would be really awesome:
1) [Restart Firefox in Safe Mode]
2) [Open my Profile folder]
It's also very important that it be accessible from a menu. Users have a LOT of trouble typing about:something into their location bar.

Some comments from beltzner:
I think that we might be able to find some goodness to get into Firefox 3.6 before Monday night's string freeze. It may mean landing the layout of about:support before it can actually pull in and format the data, but I think that's OK.
Looking at the list at https://wiki.mozilla.org/Support/Firefox.Next there's not a lot that I think we'd want to tackle for 3.6; all seems very string intensive, and very "self diagnostic" based which might be a little tough to do.
Instead, I think if we had about:support:
- link to SUMO
- list the installed plugins and add-ons in an easy to copy & paste textarea
- list any prefs that have been changed from default or are custom in an easy to copy & paste textarea
- list the installation history in an easy to copy & paste text area
- list the buildconfig info in an easy to copy & paste text area
we'd have a good start. Might also want some simple stats like:
- size of places DB
- size of profile directory
- age of profile directory
- number of profiles?

Created attachment 399858[details]
about:support screenshot showing extensions list
I've implemented a basic list of extensions. The styling is cribbed from about:plugins.
My primary focus for the next couple of days is figuring out which features we want for 3.6 and, importantly, what wording we want for those features. The latter is because string freeze for 3.6 is impending so we need to figure that out and finalize it as quickly as possible.
If you have opinions about layout and styling, I'd still like to hear them. However, I may not be able to act on them right away.

(In reply to comment #29)
Can I add:
Location of profile directory (don't need a link to open for now)
Also for
- list any prefs that have been changed from default or are custom in an easy to copy & paste textarea we'll need to sit down and decide what counts and what doesn't. We don't need to include all the prefs that are added by extensions or printing prefs or prefs that decide whether to show security or popups etc.

(In reply to comment #29)
>
> Instead, I think if we had about:support:
>
> - link to SUMO
> - list the installed plugins and add-ons in an easy to copy & paste textarea
Is there a benefit to making it a textarea? IMO I wouldn't want the user editing it, and if so, they can edit it wherever they're pasting it.
> - list any prefs that have been changed from default or are custom in an easy
> to copy & paste textarea
IMO just a link to about:config and/or to open prefs.js and user.js would do, esp since listing them would make this area quite noisy. I'm not sure the context that Cww is looking at in not wanting the prefs from extensions, printing etc. Depending on the problem we will be able to rule out lots of different prefs that we wouldn't want to rule out in other cases.
I would suggest that firefox itself include all the prefs for copying, and the filtering could be done at the support end. I.E. if the form the person is filling out has to do with a print problem, it would take all the prefs, but by default only display the print prefs.
> - list the installation history in an easy to copy & paste text area
> - list the buildconfig info in an easy to copy & paste text area
>
> we'd have a good start. Might also want some simple stats like:
>
I also want to add Location of profile. IMO this and listing add-ons in a copyable format are the most important inclusions. In a pinch we could probably use an "Open" button/link if we decide to implement that, but it'd be nicer to have "View profile" available.

(In reply to comment #32)
> (In reply to comment #29)
> > - list the installed plugins and add-ons in an easy to copy & paste textarea
>
> Is there a benefit to making it a textarea? IMO I wouldn't want the user
> editing it, and if so, they can edit it wherever they're pasting it.
Not to scope creep, but this page will be privileged, so I think an entirely legitimate follow up bug, once the basic page is there, is to request a button at the top that says "copy everything that matters here to my clipboard" - easier and less error-prone.
For now, though, getting a tightly constrained set of functionality in before string freeze is priority one, and thanks to everyone helping get there (and restraining what I know are your much longer wishlists. :)

> (In reply to comment #29)
> Can I add:
> Location of profile directory (don't need a link to open for now)
Good call. I missed an entire category of things around "how to help users find things on their system" like this.
> Also for
> - list any prefs that have been changed from default or are custom in an easy
> to copy & paste textarea we'll need to sit down and decide what counts and what
> doesn't. We don't need to include all the prefs that are added by extensions or
> printing prefs or prefs that decide whether to show security or popups etc.
I don't think we want to do that ahead of time. Better to include everything that's not "standard" and that way when someone copies it in for SUMO wizards to look at, they're seeing the entire list of "what makes this not like a fresh version of Firefox with a fresh profile."
(In reply to comment #33)
> Not to scope creep, but this page will be privileged, so I think an entirely
> legitimate follow up bug, once the basic page is there, is to request a button
> at the top that says "copy everything that matters here to my clipboard" -
> easier and less error-prone.
Yes, the point was to make it easy to select all and copy. I intended for it to be a read-only textarea, but copy to clipboard is better.
(In reply to comment #34)
> I still don't see the point in displaying the buildconfig info. The build ID is
> better.
We should have both. I was presuming that the buildconfig info will help SUMO folks understand if it's a standard build or something that was done custom for users.(In reply to comment #31)

Created attachment 399971[details][diff][review]
about:support v1 (WIP)
mconnor: If you don't have time to look at this patch, feel free to pass it on to someone else.
This patch is "work in progress". Normally I would not request formal review at this stage; however given the impending string freeze I want to identify any serious problems sooner rather than later.
Only some of the strings have been factored out into the DTD. I'll work on that more tomorrow. In the meantime, I'm concerned mostly about wording since we'll need to have that worked out before string freeze. I'm concerned about design/layout inasmuch as it effects wording.

It's great to see progress here!
I think the link to support should be broken out into its own top level introductory sentence, immediately below "about:support", basically saying:
This page contains technical information that might be useful when you're trying to solve a problem. If you are looking for answers to common questions about &brandShortName;, check out _support.mozilla.com_.
I bet that'll be wordsmithed.
I'd make the "Plugins" and "Build" labels slightly more descriptive ("Installed Plugins", "Build Configuration"). I like that you're linking to them for now, rather than blocking on getting all the pieces implemented here, for round 1.
The copy button being at the bottom is a layout thing that we can fix later, but we'll want to make sure that's easily findable for people.

(In reply to comment #35)
>I don't think we want to do that ahead of time. Better to include everything
>that's not "standard" and that way when someone copies it in for SUMO wizards
>to look at, they're seeing the entire list of "what makes this not like a fresh
>version of Firefox with a fresh profile."
I disagree... we don't know what extensions are putting into prefs.js and there can be sensitive information there. Knowing they have extensions should be enough, we don't need to know what those extension's settings are. We can simply say "X extension" is known to cause problems, please disable.

It is fantastic that this is being done.
Looking at the screen shot attachment from 09/11 (http://bugzilla.mozilla.org/attachment.cgi?id=399973), I have a suggestion about the top box. It can be smaller.
There is really no reason to have an entire line for each of the "Support Site", Plugins and Build URLs. Why take that much space to display a URL? Have a clickable link off to the side. People can see the URL by hovering over the link.
Thus, your top box would be half as tall. This would leave more room to have the other info be viewable more easily.

A menu item is definitely needed in order to make this easy to use. We could then use the title of that menu item as the title of the page as well (instead of "about:support").
I agree with Johnathan about moving the "copy to clipboard" button to the top, but we also need to pick a good format for how things should be copied to the clipboard in order to make this useful on SUMO. See my following attachment for how it should be formatted in order to be collapsible (so it doesn't occupy so much space by default).
(In reply to comment #38)
> cilias says on IRC that we want the support link to go to support.mozilla.COM,
> rather than support.mozilla.org.
Shouldn't we just be using the app.support.baseURL pref to get the support URL?

Created attachment 400097[details]
Formatted list of info for simple copy and paste on SUMO
This is the preferred format of the info. Headings are bold using "__" and list items start with a "*".
We will then put all this info under a collapsed-by-default box so it doesn't occupy that much space. Thanks!

Created attachment 400124[details][diff][review]
about:support v2 (WIP)
Notable changes in this patch:
Added subtitle/summary paragraph at the top with the link to support.mozilla.com, per Johnath's suggestion. Removed NYI sections for "Installed Plugins" and "Build Configuration" since that functionality is essentially available from the existing links to about:plugins and about:buildconfig.
Added the menu item Help >> Show Diagnostics, which opens the about:support page in a new tab.
I've now factored all the strings out into entity definitions in the DTD file.
I've also messed with the layout and styling some, but you shouldn't assume this is anywhere near final.

(In reply to comment #48)
> "Show Diagnostics" is definitely not indicative, because it implies that tests
> were run. This is more like "Profile information..."
I was thinking about "Technical Information..." but I'm not sure that's right either.

(In reply to comment #50)
> Created an attachment (id=400162) [details]
> about:support v3 (WIP)
>
> (In reply to comment #47)
>
>
> > I think "Show Diagnostics" is pretty jargony sounding, and would prefer
> > something like "Tech Support Information..." but I don't want to go too far
> > down the rabbit hole of wordsmithing. Either of those or some would-be third
> > candidate is better than not making string freeze.
>
>
> Maybe just "Technical Information..." But maybe that doesn't have as good
> discoverability.
Beltzner suggests "Troubleshooting Information..." since that's descriptive of what were trying to do for 3.6. If we turn it into a true diagnostic panel later, we can always rename it then.

(In reply to comment #51)
> Beltzner suggests "Troubleshooting Information..." since that's descriptive of
> what were trying to do for 3.6. If we turn it into a true diagnostic panel
> later, we can always rename it then.
+1

Created attachment 400219[details][diff][review]
about:support v4 (WIP)
The menu item is now called "Troubleshooting Information...".
The link to the support website now uses the "app.support.baseURL" pref.

Created attachment 400409[details][diff][review]
about:support v5 (WIP)
Notable changes in this patch:
* Implemented Non-default Preferences
* Moved the copy-to-clipboard button to the upper right corner of the page
* The page title is now "Troubleshooting Information" mirroring the Help
menu item. The page still appears as "about:support" in the address bar,
however.
* Did some CSS and JavaScript cleanup.
There may not be time to implement "Installation History" and "Update History" before string freeze. Should we throw together some static mocks in case there are a few more localized strings we'll want related to those features?

(In reply to comment #40)
> (In reply to comment #35)
> >I don't think we want to do that ahead of time. Better to include everything
> >that's not "standard" and that way when someone copies it in for SUMO wizards
> >to look at, they're seeing the entire list of "what makes this not like a fresh
> >version of Firefox with a fresh profile."
>
> I disagree... we don't know what extensions are putting into prefs.js and there
> can be sensitive information there. Knowing they have extensions should be
> enough, we don't need to know what those extension's settings are. We can
> simply say "X extension" is known to cause problems, please disable.
That makes sense specifically for SUMO's purposes, but IMO we should think beyond SUMO when making a user visable page. Add-on authors would certainly like to know if a specific pref from their extension is causing the problem so that they can fix that. Users would also rather be told to flip the pref rather than disable the add-on entirely. If this information is excluded at the app level rather than on the site level then those cases (extension authors troubleshooting) don't get to benefit.

(In reply to comment #58)
> This button looks misaligned with, and almost clips, the title of the page. I'd
> suggest just putting it left-aligned below the intro paragraph (and hence,
> above all the information it will be copying.
+1
Really great to see all the amazing progress on this!

Comment on attachment 400409[details][diff][review]
about:support v5 (WIP)
>+++ b/browser/base/content/aboutSupport.xhtml Sun Sep 13 17:23:35 2009 -0700
Hmm, would it make sense to put this into toolkit, so other apps like Thunderbird and SeaMonkey could also use it in some way?
>+ <style type="text/css"><![CDATA[
Ugh, can't you put this in a theme .css file so that it's accessible to theme authors, just like about: or about:plugins are?

(In reply to comment #60)
> (From update of attachment 400409[details][diff][review])
> >+++ b/browser/base/content/aboutSupport.xhtml Sun Sep 13 17:23:35 2009 -0700
>
> Hmm, would it make sense to put this into toolkit, so other apps like
> Thunderbird and SeaMonkey could also use it in some way?
In a follow up bug, absolutely - but right now toolkit is string frozen whereas browser is not. Swift action in browser gets this feature in for Firefox 3.6 users, where it will no doubt see certain evolutions. I definitely think we should move it, though - you want to file that bug?

(In reply to comment #61)
> In a follow up bug, absolutely - but right now toolkit is string frozen whereas
> browser is not.
The fact is that the only consumers of 1.9.2 toolkit are firefox 3.6 and fennec 1.0, none of them were already announced to be firmly string frozen, there's no opt-in thread in .l10n for these. So if string frozen toolkit is the only reason to land it to browser and put it into toolkit later, I'd suggest to put into toolkit just right now. This way you'll also avoid unnessecary l10n bussiness with moving files in 70+ localizations.
Pike?

Johnath, OK, I fully see the "get something in for Firefox 3.6" argument, I was mostly thinking in trunk terms here, where doing it right is more relevant than release schedules. And Thunderbird as well as SeaMonkey might just skip 1.9.2 and directly go 1.9.3 in any case. What Vlado says is still a valid concern, though.

Comment on attachment 400644[details][diff][review]
about:support v6 - strings only
>+<!ENTITY aboutSupport.nonDefaultPrefsTitle "Non-default Preferences">
^ NIT: "Modified Preferences" sounds a bit better to me, and will likely translate a little easier. You can just adjust that on check-in.
>+<!ENTITY aboutSupport.copyToClipboard.label "Copy this information to the clipboard">
^ NIT: I worry that this will be prohibitively long in German and other languages. Either change it to "Copy all to clipboard" or add a localization note that lets l10n know that they can shorten as they see fit. I think the former is easier, but leave it up to you.
Great work. r+uir+a192=beltzner

Created attachment 400662[details][diff][review]
about:support v7
(In reply to comment #68)
> (From update of attachment 400644[details][diff][review])
> >+<!ENTITY aboutSupport.nonDefaultPrefsTitle "Non-default Preferences">
>
> ^ NIT: "Modified Preferences" sounds a bit better to me, and will likely
> translate a little easier. You can just adjust that on check-in.
Done.
> >+<!ENTITY aboutSupport.copyToClipboard.label "Copy this information to the clipboard">
>
> ^ NIT: I worry that this will be prohibitively long in German and other
> languages. Either change it to "Copy all to clipboard" or add a localization
> note that lets l10n know that they can shorten as they see fit. I think the
> former is easier, but leave it up to you.
Done.
> Great work. r+uir+a192=beltzner

So we're not going to add the Default UA string as mentioned by comment 11 and comment 12 to this page and as requested to make "support easier" with bug 361734?
Has adding about:crashes been discussed? I would think a link at the bottom somewhere would be helpful for bugzilla support.

(In reply to comment #73)
> So we're not going to add the Default UA string as mentioned by comment 11 and
> comment 12 to this page and as requested to make "support easier" with bug
> 361734?
If the default UA string is modified by preferences, they'll show up. Further, there's a link to about:buildconfig which contains the UA string.
> Has adding about:crashes been discussed? I would think a link at the bottom
> somewhere would be helpful for bugzilla support.
That's not the purpose of this page. It's meant to be, in this first pass, a single place where someone can go to get the common diagnostic information that they're asked for when lodging a problem report. In the future it will hopefully contain self-diagnostics and repair tools.

about:buildconfig does not contain the UA string (at least in the newest 1.9.2 nightly it still doesn't) but from the info in about:support and about:buildconfig one can probably get all the information that the original UA string would give. The application name and version are the original ones or would they be broken when the UA string is forged (not sure what the fuelIApplication interface provides)?

(In reply to comment #74)
>
> If the default UA string is modified by preferences, they'll show up.
True.
> Further, there's a link to about:buildconfig which contains the UA string.
>
I don't see it on the latest nightly either in about:buildconfig, which is why I was asking. I was just thinking how to take care of a few birds with one stone (ie the mentioned requests), by having the original UA here might be of use [I was thinking like in () next the version number or something]. Though if it should be on about:buildconfig, where did it go and is there a bug # for this?
> > Has adding about:crashes been discussed? I would think a link at the bottom
> > somewhere would be helpful for bugzilla support.
>
> That's not the purpose of this page. It's meant to be, in this first pass, a
> single place where someone can go to get the common diagnostic information that
> they're asked for when lodging a problem report. In the future it will
> hopefully contain self-diagnostics and repair tools.
Sounds good. Thanks.
(In reply to comment #75)
> about:buildconfig does not contain the UA string (at least in the newest 1.9.2
> nightly it still doesn't)
I was just looking yesterday at bug numbers that show the original navigator.* and their corresponding override prefs to reference for your comment (though I don't know if their still valid prefs).
> but from the info in about:support and about:buildconfig one can probably get > all the information that the original UA string would give. The application
> name and version are the original ones or would they be broken when the UA
> string is forged (not sure what the fuelIApplication interface provides)?
Though I'd rather we just build and display the default UA somewhere and not try listing individual defaults or override prefs which already confuse people during the support process, hense the requests to always display the default UA in its entirety to make less work for everyone which is basically the purpose of bug 361734.

(In reply to comment #77)
> > <!ENTITY aboutSupport.extensionFirstRun "FirstRun">
> Can someone explain what this string means? I have no idea how to translate it.
"Indicates whether this is the extension's first run after install"
I think this is to facilitate extensions that want to show an introductory page the first time they run after they've been installed.
It's probably a legitimate question as to whether this is really useful on the about:support page. However, the data's there so I figured it wouldn't hurt to include it.

(In reply to comment #78)
> (In reply to comment #77)
> > > <!ENTITY aboutSupport.extensionFirstRun "FirstRun">
> > Can someone explain what this string means? I have no idea how to translate it.
>
> "Indicates whether this is the extension's first run after install"
>
> I think this is to facilitate extensions that want to show an introductory page
> the first time they run after they've been installed.
Axel, am I right in my belief that Curtis can add a localization note to this dtd when he does his final checkin without hurting string freeze/existing translations?

Yes, comments are fine.
I'm not sure if the information is conveyed by the UI string "FirstRun", btw. I kick myself for not commenting my nag about that string when I glanced at the patch. Not to say more than "removal of strings would be fine still, too" :-)

(In reply to comment #80)
> Yes, comments are fine.
>
> I'm not sure if the information is conveyed by the UI string "FirstRun", btw. I
> kick myself for not commenting my nag about that string when I glanced at the
> patch. Not to say more than "removal of strings would be fine still, too" :-)
In the case of "FirstRun", I think it's more important to be meaningful to our support people than that it's particularly meaningful to the user.

Comment on attachment 400662[details][diff][review]
about:support v7
This patch doesn't apply cleanly now, because of the string landings.
>diff -r 6f365b3614e6 browser/base/content/aboutSupport.xhtml
>--- /dev/null Thu Jan 01 00:00:00 1970 +0000
>+++ b/browser/base/content/aboutSupport.xhtml Mon Sep 14 19:54:41 2009 -0700
>@@ -0,0 +1,370 @@
>+<?xml version="1.0" encoding="UTF-8"?>
>+
>+# ***** BEGIN LICENSE BLOCK *****
># Version: MPL 1.1/GPL 2.0/LGPL 2.1
>#
># The contents of this file are subject to the Mozilla Public License Version
This patch is also a little odd because these comment lines aren't prefixed with +'s, confusing the heck out of hg.
>+
>+ // Update the other sections.
>+ populateExtensionsSection(Application.extensions.all)
>+ populatePreferencesSection(Application.prefs.all)
>+ }
Based on your reply, I am really concerned about the lag these introduce on initial load. People opening a Troubleshooting menu item are likely in no mood to have the browser seem to hang for 5 seconds. Can we do a deferred load here, instead? Basically, put a throbber placeholder in the tables initially, put these things in a setTimeout() and when they're done they can replace the throbber with their contents, that way the page itself comes up instantly and gives more visible indication that it's working very hard in the background. I would also disable the copy button until that timeout call completes. No need for new graphics resource, you can just use chrome://global/skin/icons/loading_16.png
>+ function populateExtensionsSection(extensions)
>+ {
>+ var trExtensions = [];
>+ for (var i = 0; i < extensions.length; i++) {
>+ var extension = extensions[i];
>+ var tr = createParentElement("tr", [
I don't think it really matters much in this case, where the entire function is ~10 lines, but as of javascript 1.7, variables intended to have block scope instead of function scope should be declared with 'let', not 'var'.
>+ function populatePreferencesSection(prefs)
>+ {
>+ var modifiedPrefs = [];
>+ for (var i = 0; i < prefs.length; i++) {
Ditto, particularly since you re-use i later on in this function. You re-initialize i, so there's no bug here, but 'let' is more representative of your intent. In fact, in this particular case, you should really just do:
modifiedPrefs = prefs.filter(function(pref) {return pref.modified});
>+ function copyContentsToClipboard()
>+ {
>+ var contentsDiv = document.getElementById("contents");
>+ var dataHtml = contentsDiv.innerHTML;
>+ var dataText = contentsDiv.textContent;
>+
>+ var supportsStringClass = Cc["@mozilla.org/supports-string;1"];
>+ var ssHtml = supportsStringClass.createInstance(Ci.nsISupportsString);
>+ var ssText = supportsStringClass.createInstance(Ci.nsISupportsString);
>+
>+ var transferable = Cc["@mozilla.org/widget/transferable;1"]
>+ .createInstance(Ci.nsITransferable);
>+
>+ transferable.addDataFlavor("text/html");
>+ ssHtml.data = dataHtml;
>+ transferable.setTransferData("text/html", ssHtml, dataHtml.length * 2);
>+
>+ transferable.addDataFlavor("text/unicode");
>+ ssText.data = dataText;
>+ transferable.setTransferData("text/unicode", ssText, dataText.length * 2);
>+
>+ var clipboard = Cc["@mozilla.org/widget/clipboard;1"]
>+ .getService(Ci.nsIClipboard);
>+ clipboard.setData(transferable, null, clipboard.kGlobalClipboard);
>+ }
I shouldn't review our clipboard code, having not used it before. Has someone like Enn or mconnor reviewed this?
>diff -r 6f365b3614e6 browser/components/about/AboutRedirector.cpp
> { "sessionrestore", "chrome://browser/content/aboutSessionRestore.xhtml",
>- nsIAboutModule::ALLOW_SCRIPT }
>+ nsIAboutModule::ALLOW_SCRIPT },
>+ { "support", "chrome://browser/content/aboutSupport.xhtml",
>+ nsIAboutModule::ALLOW_SCRIPT },
> };
Shouldn't have the terminal comma here
>diff -r 6f365b3614e6 browser/locales/en-US/chrome/browser/aboutSupport.dtd
>+<!ENTITY aboutSupport.extensionsTitle "Extensions">
>+<!ENTITY aboutSupport.extensionName "Name">
>+<!ENTITY aboutSupport.extensionEnabled "Enabled">
>+<!ENTITY aboutSupport.extensionVersion "Version">
>+<!ENTITY aboutSupport.extensionFirstRun "FirstRun">
>+<!ENTITY aboutSupport.extensionId "ID">
Add the localization note mentioned in bug discussion, to explain what FirstRun means. I do think it should be translated if everything else is, I just also wish that we had included a space, though, to make it look more Englishy, less Jargony. File a follow-up bug to do so on trunk if it's too late to get it in on branch?
r=me with the nits addressed, except for the clipboard bits, but I'd like to take a look at the delayed-load stuff before calling this done.
This patch doesn't include tests, but I'm not sure what they would look like - mostly you're exercising other APIs not implementing your own. With that said, though, please flag this with in-litmus? so that QA can put together a human "does the menu item do the thing, and does it look non-broken" test.

This hasn't landed yet on trunk nor branch?
We have the strings in on branch, so we should make a tough call on whether this should land for beta or after. Landing it now on branch might expose problems in the existing localizations. This consideration does not affect landing on central.

Please don't reveal the profile directory in this page. Giving it away defeats the defense-in-depth security feature of having a nonce. I think a "Reveal profile folder in Finder" button would be more useful, anyway.

> Please don't reveal the profile directory in this page. Giving it away defeats
> the defense-in-depth security feature of having a nonce. I think a "Reveal
> profile folder in Finder" button would be more useful, anyway.
How about showing the path where the profile folder is but not the name of it? (c:\users\jsmith\roaming profile\mozilla firefox\profiles) I think there may be permissions related issues that can be easily diagnosed knowing where the profile is located while still preserving the randomized name.(In reply to comment #85)

Created attachment 401794[details][diff][review]
about:support v8
[mconnor: can you take a look at the clipboard code?]
[johnath: note that I also added some code to make pasted text (as opposed to HTML) look better.]
(In reply to comment #83)
> (From update of attachment 400662[details][diff][review])
> This patch doesn't apply cleanly now, because of the string landings.
>
> >diff -r 6f365b3614e6 browser/base/content/aboutSupport.xhtml
> >--- /dev/null Thu Jan 01 00:00:00 1970 +0000
> >+++ b/browser/base/content/aboutSupport.xhtml Mon Sep 14 19:54:41 2009 -0700
> >@@ -0,0 +1,370 @@
> >+<?xml version="1.0" encoding="UTF-8"?>
> >+
> >+# ***** BEGIN LICENSE BLOCK *****
> ># Version: MPL 1.1/GPL 2.0/LGPL 2.1
> >#
> ># The contents of this file are subject to the Mozilla Public License Version
>
> This patch is also a little odd because these comment lines aren't prefixed
> with +'s, confusing the heck out of hg.
This was a line ending problem. I've fixed it, and I'll be more careful in the future.
> >+
> >+ // Update the other sections.
> >+ populateExtensionsSection(Application.extensions.all)
> >+ populatePreferencesSection(Application.prefs.all)
> >+ }
>
> Based on your reply, I am really concerned about the lag these introduce on
> initial load. People opening a Troubleshooting menu item are likely in no mood
> to have the browser seem to hang for 5 seconds. Can we do a deferred load here,
> instead? Basically, put a throbber placeholder in the tables initially, put
> these things in a setTimeout() and when they're done they can replace the
> throbber with their contents, that way the page itself comes up instantly and
> gives more visible indication that it's working very hard in the background. I
> would also disable the copy button until that timeout call completes. No need
> for new graphics resource, you can just use
> chrome://global/skin/icons/loading_16.png
I rewrote the code for reading the modified prefs to use the direct prefs API.
It now seems to take no more than a few hundred milliseconds to build the
modified prefs table. This is just as well since the throbber PNG wouldn't
animate while the older and slower prefs code was running.
> >+ function populateExtensionsSection(extensions)
> >+ {
> >+ var trExtensions = [];
> >+ for (var i = 0; i < extensions.length; i++) {
> >+ var extension = extensions[i];
> >+ var tr = createParentElement("tr", [
>
> I don't think it really matters much in this case, where the entire function is
> ~10 lines, but as of javascript 1.7, variables intended to have block scope
> instead of function scope should be declared with 'let', not 'var'.
Fixed.
>
> >+ function populatePreferencesSection(prefs)
> >+ {
> >+ var modifiedPrefs = [];
> >+ for (var i = 0; i < prefs.length; i++) {
>
> Ditto, particularly since you re-use i later on in this function. You
> re-initialize i, so there's no bug here, but 'let' is more representative of
> your intent. In fact, in this particular case, you should really just do:
>
> modifiedPrefs = prefs.filter(function(pref) {return pref.modified});
This code is quite a bit different now, but it is using let.
>
> >+ function copyContentsToClipboard()
> >+ {
> >+ var contentsDiv = document.getElementById("contents");
> >+ var dataHtml = contentsDiv.innerHTML;
> >+ var dataText = contentsDiv.textContent;
> >+
> >+ var supportsStringClass = Cc["@mozilla.org/supports-string;1"];
> >+ var ssHtml = supportsStringClass.createInstance(Ci.nsISupportsString);
> >+ var ssText = supportsStringClass.createInstance(Ci.nsISupportsString);
> >+
> >+ var transferable = Cc["@mozilla.org/widget/transferable;1"]
> >+ .createInstance(Ci.nsITransferable);
> >+
> >+ transferable.addDataFlavor("text/html");
> >+ ssHtml.data = dataHtml;
> >+ transferable.setTransferData("text/html", ssHtml, dataHtml.length * 2);
> >+
> >+ transferable.addDataFlavor("text/unicode");
> >+ ssText.data = dataText;
> >+ transferable.setTransferData("text/unicode", ssText, dataText.length * 2);
> >+
> >+ var clipboard = Cc["@mozilla.org/widget/clipboard;1"]
> >+ .getService(Ci.nsIClipboard);
> >+ clipboard.setData(transferable, null, clipboard.kGlobalClipboard);
> >+ }
>
> I shouldn't review our clipboard code, having not used it before. Has someone
> like Enn or mconnor reviewed this?
I'll track down a suitable person.
> >diff -r 6f365b3614e6 browser/components/about/AboutRedirector.cpp
> > { "sessionrestore", "chrome://browser/content/aboutSessionRestore.xhtml",
> >- nsIAboutModule::ALLOW_SCRIPT }
> >+ nsIAboutModule::ALLOW_SCRIPT },
> >+ { "support", "chrome://browser/content/aboutSupport.xhtml",
> >+ nsIAboutModule::ALLOW_SCRIPT },
> > };
>
> Shouldn't have the terminal comma here
I admit it's a little bit gauche, but we have a terminal comma in a number of
places in our source code. It's legal C++, it simplifies adding records at the
end, and it results in cleaner looking diffs when you do add another record to
the initializer list.
> >diff -r 6f365b3614e6 browser/locales/en-US/chrome/browser/aboutSupport.dtd
> >+<!ENTITY aboutSupport.extensionsTitle "Extensions">
> >+<!ENTITY aboutSupport.extensionName "Name">
> >+<!ENTITY aboutSupport.extensionEnabled "Enabled">
> >+<!ENTITY aboutSupport.extensionVersion "Version">
> >+<!ENTITY aboutSupport.extensionFirstRun "FirstRun">
> >+<!ENTITY aboutSupport.extensionId "ID">
>
> Add the localization note mentioned in bug discussion, to explain what FirstRun
> means. I do think it should be translated if everything else is, I just also
> wish that we had included a space, though, to make it look more Englishy, less
> Jargony. File a follow-up bug to do so on trunk if it's too late to get it in
> on branch?
Added the localization note, and also added that translations should feel free to add a space between the words. Can we just slipstream the space in? Its presence or absence shouldn't change the translations...
> r=me with the nits addressed, except for the clipboard bits, but I'd like to
> take a look at the delayed-load stuff before calling this done.
With the new optimizations, I think the load speed is no longer an issue.
> This patch doesn't include tests, but I'm not sure what they would look like -
> mostly you're exercising other APIs not implementing your own. With that said,
> though, please flag this with in-litmus? so that QA can put together a human
> "does the menu item do the thing, and does it look non-broken" test.
Noted.

Comment on attachment 401794[details][diff][review]
about:support v8
Only looked at the clipboard bits...
Don't use implicit global variables. Explicitly declare them, or don't use them at all. And if you're using a global variable, there's no need to pass it as an argument to functions... this threw me off at first.
Is there any point to |indent| at all now? You pass that through all over the place, but if you start with "" you're just adding unnecessary complexity...

(In reply to comment #90)
> Inherited indentation for JS seems to be the norm, at least for non-test .xhtml
> files. There wasn't enough inline CSS in these files to draw a conclusion for
> CSS though.
It may be the prevailing style, but most likely not for a good reason. So unless it makes sense to you, you don't need to copy it. I think not doing it makes it easier to handle the code, and I'm not doing it in bug 453063.
> There seems to be no real consensus about opening brace placement for
> functions, at least in the "browser/base/content/*.js" files.
Lots of legacy code and lots of different authors, including those who primarily deal with languages where this actually makes sense. We haven't tried to maintain this within these files and consequently don't need to do it in new files.

Comment on attachment 401977[details][diff][review]
about:support v8
(In reply to comment #90)
> > Don't use implicit global variables. Explicitly declare them, or don't use them
> > at all. And if you're using a global variable, there's no need to pass it as
> > an argument to functions... this threw me off at first.
>
> I've renamed "textFragments" to "textFragmentAccumulator" to make it explicit
> about how the results are being returned from the tree traversal. Using a
> receiver argument like this is a little bit unusual in JavaScript, but we do it
> all the time in C++ (especially with strings). I don't think it's an "implicit
> global variable".
...
>diff -r fb31c1bdf4dd browser/base/content/aboutSupport.xhtml
>+ // Return the plain text representation of an element. Do a little bit
>+ // of pretty-printing to make it human-readable.
>+ function createTextForElement(elem)
>+ {
>+ // Generate the initial text.
>+ textFragmentAccumulator = [];
textFragmentAccumulator is initialized without a 'var' keyword (function scope) or a 'let' keyword (block scope) and hence, implicitly, becomes part of the global scope.
mconnor is saying that if you intend for it to be a global variable, it should have an explicit declaration outside of the various function bodies in your script block and, being global, doesn't need to be passed as an argument to any function. If you intend, however, for it to be function-scoped and not appear in the global, long-lived namespace, then it should be "var textFragmentAccumulator..."
I suspect the latter is your intent.

Created attachment 402122[details][diff][review]
about:support v10
(In reply to comment #92)
> >diff -r fb31c1bdf4dd browser/base/content/aboutSupport.xhtml
> >+ // Return the plain text representation of an element. Do a little bit
> >+ // of pretty-printing to make it human-readable.
> >+ function createTextForElement(elem)
> >+ {
> >+ // Generate the initial text.
> >+ textFragmentAccumulator = [];
>
> textFragmentAccumulator is initialized without a 'var' keyword (function scope)
> or a 'let' keyword (block scope) and hence, implicitly, becomes part of the
> global scope.
>
> mconnor is saying that if you intend for it to be a global variable, it should
> have an explicit declaration outside of the various function bodies in your
> script block and, being global, doesn't need to be passed as an argument to any
> function. If you intend, however, for it to be function-scoped and not appear
> in the global, long-lived namespace, then it should be "var
> textFragmentAccumulator..."
>
> I suspect the latter is your intent.
Indeed. Fixed.

Comment on attachment 402122[details][diff][review]
about:support v10
This is fine for beta, for the clipboard bits at least, though johnath should weigh in as well.
We'll need a followup on:
* Not copy/pasting profile directory
* Fixing the plaintext copy/paste formatting (discussion on IRC)

Comment on attachment 402122[details][diff][review]
about:support v10
I agree we want the followups mconnor mentions filed, as well as the one I requested in comment 61 to move this to toolkit. I think the profile directory bug should block 3.6 final, but doesn't need to block this hitting the beta.
>+ let prefs = [Application.prefs.get(prefName)
>+ for each (prefName in prefNames)
>+ if (prefRootBranch.prefHasUserValue(prefName))];
Don't see a lot of array comprehension syntax in the code, but this is a nice use of it.
>+ <th>
>+ &aboutSupport.extensionFirstRun;
>+ </th>
As we've said in IRC: firstRun was mostly added because it was there, not because SUMO requested it, the string is confusing and requires a localization note, and the field itself is unlikely to be really diagnostic most of the time. I'd say we should just remove it, really.
r=me with the firstrun removal and follow up bugs filed and nominated for blocking in the one case.

Created attachment 402648[details][diff][review]
about:support v11
****************** mconnor ********************
(In reply to comment #95)
> (From update of attachment 402122[details][diff][review])
> This is fine for beta, for the clipboard bits at least, though johnath should
> weigh in as well.
>
> We'll need a followup on:
>
> * Not copy/pasting profile directoryBug 518601 - Troubleshooting Information page should not allow copy-and-paste of the profile directory.
> * Fixing the plaintext copy/paste formatting (discussion on IRC)Bug 518606 - Troubleshooting Information page should have better support for copy-and-paste to plaintext.
****************** johnath ********************
(In reply to comment #96)
> (From update of attachment 402122[details][diff][review])
> I agree we want the followups mconnor mentions filed, as well as the one I
> requested in comment 61 to move this to toolkit. I think the profile directory
> bug should block 3.6 final, but doesn't need to block this hitting the beta.Bug 518601 - Troubleshooting Information page should not allow copy-and-paste of the profile directory. (requested blocking)
Bug 518607 - Move the Troubleshooting Information page into toolkit so other apps like Thunderbird and SeaMonkey can use it.
> >+ let prefs = [Application.prefs.get(prefName)
> >+ for each (prefName in prefNames)
> >+ if (prefRootBranch.prefHasUserValue(prefName))];
>
> Don't see a lot of array comprehension syntax in the code, but this is a nice
> use of it.
>
> >+ <th>
> >+ &aboutSupport.extensionFirstRun;
> >+ </th>
>
> As we've said in IRC: firstRun was mostly added because it was there, not
> because SUMO requested it, the string is confusing and requires a localization
> note, and the field itself is unlikely to be really diagnostic most of the
> time. I'd say we should just remove it, really.
Done. Note that I changed the localization note in the DTD to just "no longer in use". Is there existing protocol for this sort of thing?
> r=me with the firstrun removal and follow up bugs filed and nominated for
> blocking in the one case.

(In reply to comment #97)
<...>
> > As we've said in IRC: firstRun was mostly added because it was there, not
> > because SUMO requested it, the string is confusing and requires a localization
> > note, and the field itself is unlikely to be really diagnostic most of the
> > time. I'd say we should just remove it, really.
>
>
> Done. Note that I changed the localization note in the DTD to just "no longer
> in use". Is there existing protocol for this sort of thing?
Just remove the string, that's fine for both central and 1.9.2 at this point in time. It will only yield obsolete strings for now, which don't pose any problem with shipping, and the localizations can still fix it for either Beta or RC.

Gnome's HIGs (http://library.gnome.org/devel/hig-book/stable/menus-design.html.en) are practically saying the same as Windows HIGs:
"Label the menu item with a trailing ellipsis ("...") only if the command requires further input from the user before it can be performed. Do not add an ellipsis to items that only present a confirmation dialog (such as Delete), or that do not require further input (such as Properties, Preferences or About)."
Apple's HIGs (http://developer.apple.com/mac/library/documentation/UserExperience/Conceptual/AppleHIGuidelines/XHIGText/XHIGText.html) say:
Use an ellipsis in the name of a button or menu item when the associated action:
* Requires specific input from the user.
* Is performed _by the user_ in a separate window or dialog.
* Always displays an alert that warns the user of a potentially dangerous outcome and offers an alternative.
BUT NOT when:
* Does not require specific input from the user.
* Is completed by the opening of a panel.
* Occasionally displays an alert that warns the user of a potentially dangerous outcome.
So the ellipsis is wrong on all the threee platforms. i'll file the followup.