557738 - Add user.js section to about:support

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Noah (NoahSUMO/SolidSnake) [:Noah]]

If a user.js file is present in a user's firefox profile, it should definitely be added to the about:support page, listing what prefs it has inside.

This would be a huge help in advising users on the Firefox support forums that they may need to remove this file in order to stop a certain pref from constantly not respecting their choice when they change it.

Also badware have used user.js to set homepage urls, keyword search urls, etc so that people will never be able to change these settings.

Comment 1

[Matthew N. [:MattN]]

Attachment: v.1 boolean in raw data, sandboxed iframe in page (for privacy)

Noah asked me to take a look at this.

The boolean is the raw data so support will do that a user.js exists.  I'm avoiding leaking the filesystem path in the data.  The UI shows an iframe with the contents of the user.js so that the user can easily inspect/copy the data.

Comment 2

[Matthew N. [:MattN]]

Attachment: v.1 Screenshot

Comment 3

[Drew Willcoxon :adw]

Comment on attachment 667756 [details] [diff] [review]
v.1 boolean in raw data, sandboxed iframe in page (for privacy)

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

I don't think including user.js inline is the right way to go: it looks strange, and it's inconsistent with the way the page handles other external information like the profile folder, about:plugins, and about:buildconfig.  I think it should be handled in a similar way, as a link or button that opens the file in the tab, plus your explanatory text.

::: toolkit/content/Troubleshoot.jsm
@@ +181,5 @@
> +    userJSFile.append("user.js");
> +    done({
> +      exists: userJSFile.exists() && userJSFile.fileSize > 0,
> +    });
> +  },

You need to add this userJS property to SNAPSHOT_SCHEMA in toolkit/content/tests/browser/browser_Troubleshoot.js.

::: toolkit/locales/en-US/chrome/global/aboutSupport.dtd
@@ +47,5 @@
>  <!ENTITY aboutSupport.modifiedPrefsValue "Value">
>  
> +<!-- LOCALIZATION NOTE (aboutSupport.userJSTitle): user.js is the name of the preference override file being checked. -->
> +<!ENTITY aboutSupport.userJSTitle "user.js preferences">
> +<!ENTITY aboutSupport.userJSDescription "A user.js file is present in your profile folder which may override preferences set within the application.">

The back half of this sentence is a non-restrictive relative clause, so "folder" needs a comma after it, i.e.,

A user.js file is present in your profile folder, which may override preferences set within the application.

http://en.wikipedia.org/wiki/Relative_clause#Restrictive_and_non-restrictive

Comment 4

[Matthew N. [:MattN]]

Attachment: v.2 link to view file URI of user.js, if present

Thanks for the quick review Drew.

Comment 5

[Matthew N. [:MattN]]

Attachment: v.2 screenshot

Noah, is this fine?  The link will display the contents in the tab.

Comment 6

[Noah (NoahSUMO/SolidSnake) [:Noah]]

Matt, thanks once again for taking this bug! Your work is perfect. It was exactly what I was looking for. The version pre-Drew's suggestion.

I'm not sure why Drew disagrees with it being inline on the about:support page. It's no more different or strange than the "Modified Preferences" area. And I'm not sure why he's comparing it to other about: pages. I also wouldn't say user.js is external information. It lives right inside the profile folder and directly affects prefs.js.

It also wouldn't show up on the about:support page for a majority of users. So only those advanced users who created the file & unsuspecting victims who suffered malicious user.js abuse would see it. But to be fair, in a few cases, legitimate addons use user.js.

Moving it to another tab loses the "Copy all to clipboard" feature, which makes the basic Firefox user more likely to not copy the information and paste it for us (SUMO & Mozillazine forum helpers, who need this feature to help others track down persistent about:config changes).

Many people are not computer savvy so adding the extra step of navigating to another tab and manually highlighting and copying the user.js contents complicates the quick retrieval of information we were looking for.

At support.mozilla.org, we have conditioned people to visit about:support, click "Copy all to clipboard", paste it in the provided field "Troubleshooting Information", and submit it to us. Please help us keep this as simple and as foolproof as possible.

Comment 7

[Drew Willcoxon :adw]

Noah, have you seen the "Copy raw data to clipboard" button in recent Nightlies?  Would it be OK to include the contents of user.js in that data while showing a link to user.js in the about:support page?

Comment 8

[Matthew N. [:MattN]]

(In reply to Noah (oldtimer) [:Noah] from comment #6)
> Moving it to another tab loses the "Copy all to clipboard" feature, which
> makes the basic Firefox user more likely to not copy the information and
> paste it for us (SUMO & Mozillazine forum helpers, who need this feature to
> help others track down persistent about:config changes).

The first version didn't copy the contents to the clipboard either, it only included whether the file exists and is non-empty.  I thought I mentioned to you on IRC that we cannot copy the contents to the clipboard because the data is not sanitized like the other prefs.  To implement the sanitization, we would have to re-parse user.js and we don't have APIs exposed to do that.  

> Many people are not computer savvy so adding the extra step of navigating to
> another tab and manually highlighting and copying the user.js contents
> complicates the quick retrieval of information we were looking for.

I think the cost/benefit ratio of having the contents is high since the file is for a user's customized preferences and if the user didn't put it there then they should send it to the trash (using the button to open the profile folder). If SUMO is curious what the contents are then they can ask the occasional more technical user to provide it by clicking the link and copying.

> At support.mozilla.org, we have conditioned people to visit about:support,
> click "Copy all to clipboard", paste it in the provided field
> "Troubleshooting Information", and submit it to us. Please help us keep this
> as simple and as foolproof as possible.

This training to blindly copy and submit the information to people is exactly why we shouldn't include the contents user.js on the clipboard. The copied data will indicate whether a user.js file exists (and is non-empty).

I think we should proceed with the current patch.

Comment 9

[Drew Willcoxon :adw]

Comment on attachment 668354 [details] [diff] [review]
v.2 link to view file URI of user.js, if present

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

The "user.js preferences" heading and text above the table of preferences make it look like the preferences in the table are part of user.js.  So I think the text should come after the table and make it clear that the preferences in user.js are in addition to those in the table.  Something like:

Additionally, your profile folder contains a user.js file, which includes preferences that may override those above and others set within the application.

And I'd make "user.js file" an inline link rather than having a separate "View file" link.

(Thanks for attaching screenshots.)

Comment 10

[Matthew N. [:MattN]]

Attachment: v.3 Inline link, move below table and change string

(In reply to Drew Willcoxon :adw from comment #9)
> And I'd make "user.js file" an inline link rather than having a separate
> "View file" link.

Good call, it didn't seem natural before.

Comment 11

[Drew Willcoxon :adw]

Comment on attachment 669329 [details] [diff] [review]
v.3 Inline link, move below table and change string

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

::: toolkit/content/aboutSupport.js
@@ +207,5 @@
> +    let userJSFile = Services.dirsvc.get("PrefD", Ci.nsIFile);
> +    userJSFile.append("user.js");
> +    let userJSLink = document.getElementById("prefs-user-js-link");
> +    userJSLink.href = Services.io.newFileURI(userJSFile).spec;
> +    let userJSSection = document.getElementById("prefs-user-js-section");

You can use $() instead of document.getElementById() if you'd like.  (Every other place does.)

::: toolkit/locales/en-US/chrome/global/aboutSupport.dtd
@@ +46,5 @@
>  <!ENTITY aboutSupport.modifiedPrefsName "Name">
>  <!ENTITY aboutSupport.modifiedPrefsValue "Value">
>  
> +<!-- LOCALIZATION NOTE (aboutSupport.userJSTitle, aboutSupport.userJSDescription): user.js is the name of the preference override file being checked. -->
> +<!ENTITY aboutSupport.userJSTitle "user.js preferences">

I guess "preferences" should be capitalized, since other headings in this page use title case.

Comment 12

[Matthew N. [:MattN]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4210c1f677e5

Try results: https://tbpl.mozilla.org/?tree=Try&rev=e57970061793

Comment 13

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4210c1f677e5