Closed Bug 728981 Opened 8 years ago Closed 8 years ago

Add "browser.search" preferences to about:support

Categories

(Toolkit :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla14

People

(Reporter: cilias, Assigned: cilias)

References

()

Details

Attachments

(1 file, 4 obsolete files)

From discussion in SUMO community forum:
https://support.mozilla.org/en-US/forums/contributors/708157

These 3 items would be helpful in the about:support whitelist:
"browser.search."
"services.sync.engine."
"services.sync.lastSync"
Assignee: nobody → bmo2010
Attached patch patch1 (obsolete) — Splinter Review
Attachment #599012 - Flags: review?(paul)
Comment on attachment 599012 [details] [diff] [review]
patch1

Ooops. Browser.search isn't in the right spot (alphabetically).
Attachment #599012 - Attachment is obsolete: true
Attachment #599012 - Flags: review?(paul)
Sync is getting covered in bug 620465
Attached patch patch2 (obsolete) — Splinter Review
Attachment #599022 - Flags: review?(paul)
Sorry Dave. I didn't see your comment before attaching the second patch.
Should I wait until that patch is checked in?
Summary: Add "browser.search.", "services.sync.engine." and "services.sync.lastSync" to about:support → Add "browser.search." to about:support
Attachment #599022 - Attachment is obsolete: true
Attachment #599022 - Flags: review?(paul)
Status: NEW → ASSIGNED
(In reply to Chris Ilias [:cilias] from comment #6)
> Should I wait until that patch is checked in?

I don't think that's necessary
Attached patch patch3 (obsolete) — Splinter Review
Attachment #599858 - Flags: review?(paul)
Comment on attachment 599858 [details] [diff] [review]
patch3

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

I'm not sure how useful all of browser.search.* is, but I guess it's mostly no harm. For the most part, the prefs are really just used for defaults - most of the real customization is stored separately by the search service.

I have a small concern about privacy related to the search.selectedEngine... if somebody added a engine for a "sensitive" site and then left it selected it would show up. I guess that could be an indicator to answering "why does my computer only search for porn from the search box?".

Dave, what do you think?
Attachment #599858 - Flags: feedback?(dtownsend+bugmail)
(In reply to Paul O'Shannessy [:zpao] from comment #9)

> I have a small concern about privacy related to the search.selectedEngine...
> if somebody added a engine for a "sensitive" site and then left it selected
> it would show up. I guess that could be an indicator to answering "why does
> my computer only search for porn from the search box?".

Same general concern would be true for the prefs showing the ordering of engines.

Since this isn't sent anywhere unless the user cuts'n'pastes as part of a support request, I'm not worried about it.

Dave?
Comment on attachment 599858 [details] [diff] [review]
patch3

Gavin defined the criteria for inclusion here quite well in bug 620465 comment 7 and I think I have to agree that the currently active search engine is probably not quite there.

The referenced support discussion doesn't really say what we hope to find by including these prefs so for now I'm include to say we shouldn't include the selected engine or the ordering (the ordering is done in the DB these days so those aren't useful anyway).
Attachment #599858 - Flags: feedback?(dtownsend+bugmail) → feedback-
(In reply to Dave Townsend (:Mossop) from comment #11)
> Comment on attachment 599858 [details] [diff] [review]
> patch3
> 
> Gavin defined the criteria for inclusion here quite well in bug 620465
> comment 7 and I think I have to agree that the currently active search
> engine is probably not quite there.

I don't understand. Why wouldn't we be okay with a user posting that data?
(In reply to Chris Ilias [:cilias] from comment #12)
> (In reply to Dave Townsend (:Mossop) from comment #11)
> > Comment on attachment 599858 [details] [diff] [review]
> > patch3
> > 
> > Gavin defined the criteria for inclusion here quite well in bug 620465
> > comment 7 and I think I have to agree that the currently active search
> > engine is probably not quite there.
> 
> I don't understand. Why wouldn't we be okay with a user posting that data?

See comment 9
(In reply to Chris Ilias [:cilias] from comment #12)
> I don't understand. Why wouldn't we be okay with a user posting that data?

If Governor Schwarzenegger was searching for futuristic robot porn using an added search engine and then his Firefox wouldn't load bookmarks and he was asked to paste about:support into a forum, he may find himself in a compromising situation.
Are there any search engines whose mere existence pose a privacy concern? What about a porn extension being listed in the extensions section? Or the home page?
(In reply to Chris Ilias [:cilias] from comment #15)
> Are there any search engines whose mere existence pose a privacy concern?
> What about a porn extension being listed in the extensions section? Or the
> home page?

I would agree the extensions are concerning in this manner too, were I to review a patch adding those now I'd probably question it but might fall on the other side of the fence simply because unlike search plugins extensions can introduce all sorts of problems that can be causes for support issues.

What sorts of problems would we be able to solve if the selected search engine were included in the list and how often do they come up?
(In reply to Dave Townsend (:Mossop) from comment #16)

> What sorts of problems would we be able to solve if the selected search
> engine were included in the list and how often do they come up?

I originally asked that browser.search preferences be added to the "Modified Preferences" section of about:support based on the discussion here: https://support.mozilla.org/en-US/forums/contributors/708157  ("Ideally, that section should list as many modified prefs as possible without compromising privacy.") 

The reason being, I remembered seeing some support forum answers that mentioned  checking or resetting browser.search preferences.  Here are two support threads I found just now:

https://support.mozilla.org/en-US/questions/860649 How do I reset default search engine in location bar? (Resetting keyword.URL didn't help)

https://support.mozilla.org/en-US/questions/779047 My right click menu has changed from "Search Google for (selected text)" to Search Yahoo. How do I change it back?
I get the feeling that already existing use-cases are now required in order for a pref to be added. The main purpose of about:support was to help users provide as much info about their setup as possible (see bug 367596). Support volunteers should not only be able assess a problem by looking at the data, but in cases where the cause of the problem is not obvious, the info should help others replicate the user's setup and hopefully reproduce the issue.

For instance, just a couple of weeks ago in the support newsgroup, I was able to figure out that the cause of a user's problem was the combination of security.warn_leaving_secure;true and the noscript extension.
<http://groups.google.com/group/mozilla.support.firefox/msg/3089a9bc923bd181>
I didn't know that pref was useful until I reproduced the user's setup.
(In reply to Alice Wyman from comment #17)
> The reason being, I remembered seeing some support forum answers that
> mentioned  checking or resetting browser.search preferences.  Here are two
> support threads I found just now:
> 
> https://support.mozilla.org/en-US/questions/860649 How do I reset default
> search engine in location bar? (Resetting keyword.URL didn't help)
> 
> https://support.mozilla.org/en-US/questions/779047 My right click menu has
> changed from "Search Google for (selected text)" to Search Yahoo. How do I
> change it back?

It doesn't seem like in either of those cases having the value of browser.search.defaultenginename was helpful for diagnosing the problem.

(In reply to Chris Ilias [:cilias] from comment #18)
> I get the feeling that already existing use-cases are now required in order
> for a pref to be added. The main purpose of about:support was to help users
> provide as much info about their setup as possible (see bug 367596). Support
> volunteers should not only be able assess a problem by looking at the data,
> but in cases where the cause of the problem is not obvious, the info should
> help others replicate the user's setup and hopefully reproduce the issue.
> 
> For instance, just a couple of weeks ago in the support newsgroup, I was
> able to figure out that the cause of a user's problem was the combination of
> security.warn_leaving_secure;true and the noscript extension.
> <http://groups.google.com/group/mozilla.support.firefox/msg/3089a9bc923bd181>
> I didn't know that pref was useful until I reproduced the user's setup.

I'm all for including lots of data here, where it isn't a potential privacy concern. Here it isn't a big concern (not a password or username f.e.) but someone has raised what I think is a valid issue so I'm interested to hear how important it is we have this data. I'm struggling to think of the need, particularly when finding the selected search engine simply means asking the user to look at the search box (thus giving us the information willingly).

In case I didn't make it clear earlier, I'm fine with including all the browser.search.* prefs except for defaultenginename and the ordering prefs.
I think we may be going in circles here. I don't think the 'Governor Schwarzenegger' issue is valid, because I can't think of a porn search plugin. And I don't think usefulness of the pref should be questioned.

What can we do to convince you? What info do you need? :)
(In reply to Chris Ilias [:cilias] from comment #20)
> I think we may be going in circles here. I don't think the 'Governor
> Schwarzenegger' issue is valid, because I can't think of a porn search
> plugin.

I just made one. You can add it via the firefox search box at http://playground.zpao.com/mozilla/robotporn/search.html. Add it, then select it and check about config.
Sorry for the latency.
So how should I go about this? Add a bunch of prefs to the whitelist, or add "browser.search." to the whitelist and "browser.search.selectedEngine" and "browser.search.order." to the blacklist?
(In reply to Chris Ilias [:cilias] from comment #22)
> Sorry for the latency.
> So how should I go about this? Add a bunch of prefs to the whitelist, or add
> "browser.search." to the whitelist and "browser.search.selectedEngine" and
> "browser.search.order." to the blacklist?

I think adding to the whitelist makes the most sense.
Comment on attachment 599858 [details] [diff] [review]
patch3

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

r- based on what has been discussed since this patch was added.
Attachment #599858 - Flags: review?(paul) → review-
If I put:
  "browser.search.update",

(without the period at the end of "update")
Will that include the other search update prefs, or do I need to put:

  "browser.search.update",
  "browser.search.update.",
(In reply to Chris Ilias [:cilias] from comment #25)
> If I put:
>   "browser.search.update",
> 
> (without the period at the end of "update")
> Will that include the other search update prefs, or do I need to put:
> 
>   "browser.search.update",
>   "browser.search.update.",

Including the trailing period will result in browser.search.update not being added, but all of its children will. Without the trailing period, it will be added as well as all of its children. You shouldn't need to have both entries.
Attached patch patch4 (obsolete) — Splinter Review
Thanks.
Attachment #599858 - Attachment is obsolete: true
Attachment #611986 - Flags: review?(paul)
Comment on attachment 611986 [details] [diff] [review]
patch4

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

I think we want browser.search.context.loadInBackground as well (at least I see it in my about:config). Just one more pass and we should be good to go. Thanks!

::: toolkit/content/aboutSupport.js
@@ +62,5 @@
> +  "browser.search.log",
> +  "browser.search.openintab",
> +  "browser.search.param",
> +  "browser.search.searchEnginesURL",
> +  "browser.search.suggest",

Make this suggest.enabled. It's the only pref under suggest (correct me if I'm wrong), so we should be explicit (in case something compromising gets added under there)

@@ +63,5 @@
> +  "browser.search.openintab",
> +  "browser.search.param",
> +  "browser.search.searchEnginesURL",
> +  "browser.search.suggest",
> +  "browser.search.update",

Nit: let's keep these alphabetized as we are the rest of the prefs (just useDBForOrder is out of order).
Attachment #611986 - Flags: review?(paul) → feedback+
Attached patch patch5Splinter Review
Attachment #611986 - Attachment is obsolete: true
Attachment #612989 - Flags: review?(paul)
Comment on attachment 612989 [details] [diff] [review]
patch5

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

Thanks Chris. Do you need somebody to land this or can you take care of it?
Attachment #612989 - Flags: review?(paul) → review+
I need someone to land it.
https://hg.mozilla.org/integration/mozilla-inbound/rev/d4e09a9ed76c
Summary: Add "browser.search." to about:support → Add "browser.search" preferences to about:support
Target Milestone: --- → mozilla14
https://hg.mozilla.org/mozilla-central/rev/d4e09a9ed76c
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.