554174 - Make about:support information easily parseable

Status: RESOLVED FIXED

(Toolkit :: General, enhancement)

Description

[[:Cww]]

Right now when people click "Copy to clipboard" from about:support it just copies the page text.  The problem is the page contents are completely localized and thus difficult to parse reliably.  This means we can't automatically get data like "which extensions are most used by people with a given problem" or correlate certain prefs with certain crashes.

We should standardize the output across locales (and ideally make it easier to parse than a blob of text).

Comment 1

[Johnathan Nightingale [:johnath]]

What should it look like, instead?

Not trying to be dense, just want to make sure a would-be implementor has clear instructions, since this could be a great first bug for a community member.

Comment 2

[[:Cww]]

I'm not sure.  I guess JSON output of some sort would be easiest to work with.  I'm not super familiar with JSON data structures but it seems like you can do:

Application Data
   Name: Firefox
   Version: 3.6.3
   -- etc --
Extensions
   1:
       Name: Java Console
       Version: 6.0.15
       Enabled: true
       ID: {blahblahblah}
   2: 
   -- etc --
Modified Preferences
   keyword.URL: 'http://blahblahblah'
   -- etc --

Comment 3

[James Socol [:jsocol, :james]]

Speaking for the SUMO site, what would be fantastic is a way to get this data in a non-localized JSON blob without needing the user to copy and paste at all. If we had a JS API that, like the geolocation API, was asynchronous and had a confirmation step (and possibly white-listed SUMO) it would allow us to parse the information automatically and make things easier on users and contributors.

Cheng's proposed format seems pretty good to me:

{'application':
  {'name': 'Firefox',
   'version': '3.6.4',
   ...
  },
 'extensions':
  [
    {'extension-blob-1': 'info', ...},
    ...
  ],
 'preferences':
  [
    {'pref': 'value',
    ...
  ]
}

Comment 4

[James Socol [:jsocol, :james]]

Cheng tells me that plugins are not included in this data since there's another way to get them. If it's possible to add plugin information here, it simplify things for us.

AMO also has a use-case for an API like this.

Comment 5

[Collin]

If there was perhaps some way to "send this" to mozilla, like we do with crash IDs, it would be very helpful. Use case: User goes to about:support and clicks "Send this data to SUMO" and gets a case ID, we can enter that ID and pull up the info. That would make helping via livechat / twitter much easier.

Comment 6

[LegNeato]

Also, as a 1/2 step, can't you slurp in the localizations from the product (I'm sure they are stored somewhere and rarely if ever change), convert from localized -> english, and then have your parsing tool(s) deal with english-only?

Comment 7

[James Socol [:jsocol, :james]]

(In reply to comment #6)
> Also, as a 1/2 step, can't you slurp in the localizations from the product (I'm
> sure they are stored somewhere and rarely if ever change), convert from
> localized -> english, and then have your parsing tool(s) deal with
> english-only?

If we can have a JSON version in the relatively near future, I don't think this is worth it.

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I'd like to try doing this if someone could point me in the right direction (eg, where's the geolocation prompt stuff implemented, where should the JSON blob stuff be generated, etc).

Comment 9

[[:Cww]]

Limi? ^^

Comment 10

[Aakash Desai [:aakashd]]

The input and bugzilla teams would love to have this capability, so that users can have an option to include their about:support information alongside their bug forms. What would we need here?

Comment 11

[Nigel Babu [:nigelb]]

Could someone explain how to do this? I'm interested in taking this up.

Comment 12

[Johnathan Nightingale [:johnath]]

(In reply to Aakash Desai [:aakashd] from comment #10)
> The input and bugzilla teams would love to have this capability, so that
> users can have an option to include their about:support information
> alongside their bug forms. What would we need here?

A format. If we know what we should copy to the clipboard instead of page text, it's just a question of someone (perhaps nigelb!) doing the legwork. Cheng suggested JSON up above, which is fine, but has a fair bit of syntactic gorp that will make the text easier for computers but harder for humans, e.g.

Extensions
   AdBlockPlus version 1.23

becomes

"{\"extensions\":[{\"name\":\"AdBlock Plus\",\"version\":\"1.23\"}]}"

If we're okay with that trade off, then JSON it is. If not, we should clarify what we actually want.

Comment 13

[James Socol [:jsocol, :james]]

JSON would be a lot more useful for SUMO, but like I said in comment 3, an API to this that didn't require copy/paste at all would be best. Then we can leave the copy/paste as plain text.

Comment 14

[Nigel Babu [:nigelb]]

I'm taking this up.  For the first cut, I'll add a button with "Copy JSON to Clipboard" that should copy JSON data un-localized to clipboard.  I'll probably have to rewrite parts of the code because the code assumes there's only going to be a push into the HTML for about:support.

Comment 15

[Drew Willcoxon :adw]

I'm planning on making a JavaScript module that provides access to structured about:support data.  Whoever wants to consume that data can do so, whether it's an add-on, a button on about:support that copies the data to the clipboard, a new DOM API, etc.

Comment 16

[Drew Willcoxon :adw]

Attachment: patch

Felipe, would you mind reviewing this?

I don't really like the name AboutSupport for the JavaScript module, since there's no "about:" page necessarily involved.  But it's intended to be used by about:support, and I can't think of a better name.

Comment 17

[Drew Willcoxon :adw]

Attachment: example data

Here's some example data that was generated during a test run.

Comment 18

[:Felipe Gomes (needinfo for replies!)]

Yeah sure, I can look at it. Is it the plan to make about:support use this module in a separate bug? I imagine there would be quite a bit of overlapping/duplicated code otherwise

Comment 19

[Drew Willcoxon :adw]

Thanks.  Yeah, that's what I'm planning.  I'd like to land both at the same time.  I'll file the about:support rewrite bug once this one is reviewed.

Comment 20

[Dão Gottwald [:dao]]

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

>+  "browser.fixup.",
>+  "browser.history_expire_",
>+  "browser.link.open_newwindow",
>+  "browser.places.",
>+  "browser.privatebrowsing.",
>+  "browser.search.context.loadInBackground",
>+  "browser.search.log",
>+  "browser.search.openintab",
>+  "browser.search.param",
>+  "browser.search.searchEnginesURL",
>+  "browser.search.suggest.enabled",
>+  "browser.search.update",
>+  "browser.search.useDBForOrder",
>+  "browser.sessionstore.",
>+  "browser.startup.homepage",
>+  "browser.tabs.",
>+  "browser.zoom.",

Can we make this list extensible at run time or build time? Most of the above prefs are Firefox-specific, other applications will want a different set of prefs.

I'd suggest TroubleshootingData as the module name.

Comment 21

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

This looks awesome! I just skimmed this quickly, but one thing that came to mind: it might be a good idea to have snapshot() wrap calls to the dataProviders in a try/catch so that failure in one of them won't impact the rest of the data.

Related to that, it might make sense in the future to also make dataProviders extensible, but that's certainly not something we need to tackle with v1.

Comment 22

[:Felipe Gomes (needinfo for replies!)]

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

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

Everything looks good to me. It would be nice to add a test to verify if the prefs blacklist is working. Clearing the review request for some simple comments, + dao's and gavin's previous notes.

::: toolkit/content/AboutSupport.jsm
@@ +77,5 @@
> +   *
> +   * @param done A function that will be asynchronously called when the
> +   *             snapshot completes.  It will be passed the snapshot object.
> +   */
> +  snapshot: function snapshot(done) {

at least in this exported function, use aCallback for the parameter name to follow the most common convention

@@ +106,5 @@
> +      name: Services.appinfo.name,
> +      version: Services.appinfo.version,
> +      userAgent: Cc["@mozilla.org/network/protocol;1?name=http"].
> +                 getService(Ci.nsIHttpProtocolHandler).
> +                 userAgent,

missing the supportURL data that is retrieved in aboutSupport.js?

@@ +137,5 @@
> +      }));
> +    });
> +  },
> +
> +  modifiedPreferences: function modifiedPreferences(done) {

I imagine you didn't mean to include the pref values, only their names here? (and let the consumer retrieve the data?). One observation is that this is the only data provider that returns an array instead of an object; if you included the values as well you'd get that extra consistency benefit (and then we can define the API for the data providers as always returning an object)

@@ +257,5 @@
> +      }
> +      catch (e) {
> +        continue;
> +      }
> +      data[gfxInfoProps[prop] || prop] = val;

could just assign data[..] inside the try block to not need to call continue

Comment 23

[Drew Willcoxon :adw]

Attachment: patch

(In reply to Dão Gottwald [:dao] from comment #20)
> Can we make this list extensible at run time or build time? Most of the
> above prefs are Firefox-specific, other applications will want a different
> set of prefs.

Yeah, good point.  Do you have any good ideas about how to do that?  It would be nice if each app could drop a file somewhere that contained a list of names that would be picked up here on its behalf.

I talked to Gavin about it today and he doesn't think it should block the bug, especially since this just refactors what's already in aboutSupport.js.

> I'd suggest TroubleshootingData as the module name.

Sounds good.  I went with Troubleshoot to encourage people to add other functionality related to troubleshooting.

(In reply to :Felipe Gomes from comment #22)
> Comment on attachment 657529 [details] [diff] [review]
> patch
> 
> Review of attachment 657529 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Everything looks good to me. It would be nice to add a test to verify if the
> prefs blacklist is working.

Good point.  I added tests for the whitelist and blacklist.

> at least in this exported function, use aCallback for the parameter name to
> follow the most common convention

I dislike that convention, and it's been left behind in some new code like the social APIs, so I stuck with unprefixed names.

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/social/

> missing the supportURL data that is retrieved in aboutSupport.js?

Yes, thanks.

> I imagine you didn't mean to include the pref values, only their names here?

Yeah, but this patch includes the values, too.

(I was concerned about leaking users' privacy, since the point of this bug is to gather info that can be easily reported to someone else.  (e.g., I consider browser.startup.homepage, which is included in the data, to be private.)  I talked with Gavin about it today, and paraphrasing, this module only gathers the data, it doesn't do anything with it, and it doesn't provide access to anything that Services.prefs and the current "copy to clipboard" function in about:support don't, so the privacy implications depend on consumers.)

Comment 24

[Drew Willcoxon :adw]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2ec1b483d19b

Comment 25

[Drew Willcoxon :adw]

Follow-up fix for Windows-#ifdef'ed JS syntax error:

https://hg.mozilla.org/integration/mozilla-inbound/rev/94de0e883fdd

Comment 26

[Graeme McCutcheon [:graememcc]]

https://hg.mozilla.org/mozilla-central/rev/2ec1b483d19b
https://hg.mozilla.org/mozilla-central/rev/94de0e883fdd

Comment 27

[Graeme McCutcheon [:graememcc]]

https://hg.mozilla.org/mozilla-central/rev/2ec1b483d19b