524757 - Add architecture and operating system to about:support

Status: RESOLVED FIXED

(Toolkit :: General, enhancement)

Description

[Jesse Ruderman]

Perhaps "Architecture: x86-32" and "Operating System: Mac OS X 10.5.8"

Comment 1

[Matthew N. [:MattN]]

Bug 591776 added the UA string to about:support which currently includes the OS and the architecture (or at least enough info to approximate it on a Mac using the OS X version, although we do allow 32-bit mode there)[1].

[1] https://developer.mozilla.org/en/Gecko_user_agent_string_reference#Windows

Comment 2

[Akilan Elango [:falcon]]

hey, I would like to work on this bug....., Get me started ...

Comment 3

[Matthew N. [:MattN]]

I'm not sure which section to put this in since this isn't about the application so it doesn't really fit in the first section of "Application Basics".

(In reply to Jesse Ruderman from comment #0)
> Perhaps "Architecture: x86-32" and "Operating System: Mac OS X 10.5.8"

Jesse, is the information in the UA string enough? I forget if we don't disclose some of this information on certain platforms:

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:41.0) Gecko/20100101 Firefox/41.0

Since Drew (the mentor) is out this week, it's not clear to me what still needs to be done from this old bug report, and it's not clear where the information should go, I think it would be good to find another bug until we sort this out. Sorry about that.

Comment 4

[Jesse Ruderman]

From that Mac UA string, it looks like the UA string doesn't include the minor OS version or the architecture's pointer size.

Comment 5

[Matthew N. [:MattN]]

(In reply to Jesse Ruderman from comment #4)
> From that Mac UA string, it looks like the UA string doesn't include the
> minor OS version

True but I'm not sure it's that useful.

> or the architecture's pointer size.

It's not necessary since 64-bit is the only option.

We seem to indicate "WOW64" or "Win64; x64;" on Windows and similar things on Linux. See http://www.useragentstring.com/pages/Firefox/

Comment 6

[Drew Willcoxon :adw]

Jesse, given Matt's most recent comment do you still think we should do this?  From what I understand, about:support is most often used by community members and SUMO to diagnose other people's problems.  They'll know to look in the UA string for OS info if they need it.  I can see the minor OS version being potentially helpful, though.

Comment 7

[Jesse Ruderman]

Ask some support people how often they need the minor OS version?

Comment 8

[Patrick]

Can you please provide me with the path file so i could do some work on it.

Comment 9

[Drew Willcoxon :adw]

Hi Patrick, thanks for volunteering.  There are a couple of files.  The first one is:

http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/Troubleshoot.jsm#180

That file generates the back-end data.  You'll want to add the OS info to the `application` data provider that I linked to.

The second file is:

http://mxr.mozilla.org/mozilla-central/source/toolkit/content/aboutSupport.js#37

That's the JS that builds the about:support XHTML page.  You can see there's an `application` part that uses the application object from Troubleshoot.jsm to build the "Application Basics" section of about:support.

If you need more help, let me know.

Comment 10

[Patrick]

Attachment: Bug___524757.diff

Hi Drew,

See patch for review

Comment 11

[Drew Willcoxon :adw]

Comment on attachment 8697970 [details] [diff] [review]
Bug___524757.diff

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

Hi Patrick, thanks for the patch.  It's not quite right, though.  We don't want to hard-code specific values.  That would mean that people using Windows or Linux would see "Mac OS X" instead of their actual OSes.  So instead we should get these values at runtime.

First though I would ask you to look at this page, especially the parts up to and including Step 1: https://developer.mozilla.org/en-US/docs/Introduction  That will tell you how to build Firefox and then run your build.  That way you can actually test the changes that you make before you ask for review.

Second, it looks like you might need some help with JS.  I recommend looking at this page: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide

I think your next steps should be: build and run Firefox on your computer so that you can test the patch you posted.  It should generate an error when you open about:support because your patch is not actually valid JS.  As practice, you might try spending a moment to make it valid (by turning those values into proper JS strings), even though we don't want to hard-code those values, as I said.

Once you've gotten that far, let me know here in the bug and I can walk you through getting the right values at runtime.

Comment 12

[Vince Tieu [:vtieu]]

Attachment: Bug___524757.diff

Hi Drew,

Here's my attempt at fixing this bug.

Comment 13

[Drew Willcoxon :adw]

Hi Vince, as I mentioned in my email, Patrick's already working on this, so let's let him try.

Comment 14

[Drew Willcoxon :adw]

Comment on attachment 8698343 [details] [diff] [review]
Bug___524757.diff

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

OK Vince, let's go with your patch.  You've updated Troubleshoot.jsm properly, but if you open about:support, you'll notice that your changes don't show up.  Did you try that?  The reason is that you need to update the page too; Troubleshoot.jsm is just the raw data.

The page lives in these xhtml and JS files:

http://mxr.mozilla.org/mozilla-central/source/toolkit/content/aboutSupport.xhtml
http://mxr.mozilla.org/mozilla-central/source/toolkit/content/aboutSupport.js

In the xhtml, you need to add a row to the application table for this new data.  I think we should combine osVersion and arch into one row, like "Darwin 14.5.0 x86-64", instead of showing them separately.  Let's add the new row after the "User Agent" row.

The first column in each row is a description, so you also need to modify the strings file that contains about:support's natural language strings:

http://mxr.mozilla.org/mozilla-central/source/toolkit/locales/en-US/chrome/global/aboutSupport.dtd

Let's use "OS" as the description.

After you add the new row to the xhtml, you need to modify the JS so the row is filled in.  The JS has comments and examples, but let me know if you need help with that part.

Finally, you need to update Troubleshoot.jsm's unit test to account for the new data.  There's a SNAPSHOT_SCHEMA object that defines what the snapshot data should look like.  You need to add entries for your new data.

http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/tests/browser/browser_Troubleshoot.js#91

Comment 15

[Vince Tieu [:vtieu]]

Attachment: Bug___524757_V2.diff

Thank you for the feedback. Here are the changes you suggested.

Comment 16

[Drew Willcoxon :adw]

Comment on attachment 8717695 [details] [diff] [review]
Bug___524757_V2.diff

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

Thanks, looks good.

Comment 17

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8f036992823c

Comment 18

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/88ce56957097

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/88ce56957097