remove the loadFromJars/jarURIs prefs

RESOLVED FIXED in Firefox 43

Status

()

defect
P1
normal
Rank:
7
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: florian, Assigned: florian)

Tracking

Trunk
Firefox 43
Points:
---
Dependency tree / graph
Bug Flags:
firefox-backlog +

Firefox Tracking Flags

(firefox41 affected, firefox43 fixed)

Details

(Whiteboard: [fxsearch][hijacking])

Attachments

(1 attachment, 1 obsolete attachment)

There is no good reason for this behavior to be pref-controlled.
Posted patch Patch (obsolete) — Splinter Review
I think this works. I'm not requesting review now because I wrote this above other patches in my queue that I want to land first, but that are not finished yet.
Whiteboard: [fxsearch][hijacking]
Rank: 7
Flags: firefox-backlog+
Priority: -- → P1
Comment on attachment 8656024 [details] [diff] [review]
Patch v2

Try seems green; requesting review.
Attachment #8656024 - Flags: review?(markh)
Comment on attachment 8656024 [details] [diff] [review]
Patch v2

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

Looks good to me!

::: toolkit/components/search/nsSearchService.js
@@ +935,5 @@
> +                                   Services.scriptSecurityManager.getSystemPrincipal(),
> +                                   null, // aTriggeringPrincipal
> +                                   Ci.nsILoadInfo.SEC_NORMAL,
> +                                   Ci.nsIContentPolicy.TYPE_OTHER);
> +  } catch (ex) { }

This would be simplified with NetUtil.newChannel

@@ +4232,4 @@
>    _findJAREngines: function SRCH_SVC_findJAREngines() {
>      LOG("_findJAREngines: looking for engines in JARs")
>  
> +    let chan = makeChannel("resource://search-plugins/list.txt");

Can we make "resource://search-plugins/" a constant up the top and add a comment explaining that we look there for app search plugins.

::: xpcom/io/nsAppFileLocationProvider.cpp
@@ +583,5 @@
>      NS_ADDREF(*aResult);
>      rv = NS_OK;
>    }
>    if (!nsCRT::strcmp(aProp, NS_APP_SEARCH_DIR_LIST)) {
> +    static const char* keys[] = { nullptr, NS_APP_USER_SEARCH_DIR, nullptr };

Looks like Thunderbird and Seamonkey still use this. Any harm in leaving it here or do we need to file bugs to have them update?
Attachment #8656024 - Flags: review?(markh) → review+
Thanks for the review!

(In reply to Dave Townsend [:mossop] from comment #4)

> ::: xpcom/io/nsAppFileLocationProvider.cpp
> @@ +583,5 @@
> >      NS_ADDREF(*aResult);
> >      rv = NS_OK;
> >    }
> >    if (!nsCRT::strcmp(aProp, NS_APP_SEARCH_DIR_LIST)) {
> > +    static const char* keys[] = { nullptr, NS_APP_USER_SEARCH_DIR, nullptr };
> 
> Looks like Thunderbird and Seamonkey still use this. Any harm in leaving it
> here or do we need to file bugs to have them update?

They both override NS_APP_SEARCH_DIR_LIST:
http://mxr.mozilla.org/comm-central/source/mail/components/shell/DirectoryProvider.cpp#194
http://mxr.mozilla.org/comm-central/source/suite/profile/nsSuiteDirectoryProvider.cpp#66

Instantbird may need to start packaging search plugins in omni.ja.

Adding the NS_APP_DISTRIBUTION_SEARCH_DIR_LIST empty enumerator here was actually so that other xul apps don't have to provide that themselves, but it seems at this point Thunderbird has already been updated to support it: http://mxr.mozilla.org/comm-central/source/mail/components/shell/DirectoryProvider.cpp#182
May still benefit SeaMonkey or Instantbird.
https://hg.mozilla.org/mozilla-central/rev/925475a10f08
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
You need to log in before you can comment on or make changes to this bug.