Closed
Bug 296430
Opened 19 years ago
Closed 19 years ago
Allow extensions to ship searchplugins
Categories
(Toolkit :: Startup and Profile System, defect, P2)
Toolkit
Startup and Profile System
Tracking
()
RESOLVED
FIXED
mozilla1.8beta3
People
(Reporter: mkaply, Assigned: benjamin)
References
Details
Attachments
(3 files, 2 obsolete files)
16.38 KB,
patch
|
mkaply
:
first-review+
asa
:
approval-aviary1.1a2+
|
Details | Diff | Splinter Review |
21.65 KB,
patch
|
darin.moz
:
first-review+
asa
:
approval-aviary1.1a2+
|
Details | Diff | Splinter Review |
19.18 KB,
patch
|
darin.moz
:
first-review+
asa
:
approval-aviary1.1a2+
|
Details | Diff | Splinter Review |
Similar to bug 295247, extensions should be able to ship searchplugins.
Reporter | ||
Comment 1•19 years ago
|
||
bsmedberg: we discussed this.
Assignee | ||
Comment 2•19 years ago
|
||
This allows directory service providers to be registered (in no particular order) in the xpcom-directory-providers category, and will be used after autoreg but before xpcom-startup or any real work is done. This means that they cannot add new component directories, but they can add pretty much anything else including chrome directories and our future searchplugin directories.
Assignee | ||
Comment 3•19 years ago
|
||
Assignee | ||
Comment 4•19 years ago
|
||
Comment on attachment 185289 [details] [diff] [review] Directory service changes, rev. 1 I should test before attaching.
Attachment #185289 -
Attachment is obsolete: true
Attachment #185289 -
Flags: first-review?(darin)
Assignee | ||
Comment 5•19 years ago
|
||
ok, this one works
Attachment #185300 -
Flags: first-review?(darin)
Assignee | ||
Comment 6•19 years ago
|
||
Comment on attachment 185298 [details] [diff] [review] Add a directory list for internetsearchservice, rev. 1 mkaply, I have no clue who a good reviewer for this is: I suggest one of your IBM minions.
Attachment #185298 -
Flags: first-review?(mozilla)
Comment 7•19 years ago
|
||
Comment on attachment 185289 [details] [diff] [review] Directory service changes, rev. 1 >Index: xpcom/build/nsXPComInit.cpp >- NS_IF_RELEASE(gDirectoryService); >+ NS_IF_RELEASE(nsDirectoryService::gService); this name change seems a bit gratuitous to me. personally, i don't mind the global gDirectoryService since it is unique enough (as unique as nsDirectoryService::gService at any rate). >Index: xpcom/io/nsDirectoryService.cpp > nsresult > nsDirectoryService::Init() > { >+ NS_NOTREACHED("Don't call me, I was for internal use only!"); >+ return NS_OK; >+} In fact, shouldn't this function be declared with NS_IMETHODIMP? It would seem that calling this function on Windows might crash. >+ NS_ADDREF(gService = self); Can this be used? self.swap(gService); >+ nsCOMPtr<nsIUTF8StringEnumerator> strings (do_QueryInterface(entries)); nit: kill space between variable name and open paran.
Attachment #185289 -
Attachment is obsolete: false
Attachment #185289 -
Flags: first-review+
Comment 8•19 years ago
|
||
Comment on attachment 185289 [details] [diff] [review] Directory service changes, rev. 1 whoops.. wrong patch
Attachment #185289 -
Attachment is obsolete: true
Attachment #185289 -
Flags: first-review+
Comment 9•19 years ago
|
||
Comment on attachment 185300 [details] [diff] [review] Directory service changes, rev. 1 Should the "xpcom-directory-providers" category be declared in some header file?
Attachment #185300 -
Flags: first-review?(darin) → first-review+
Assignee | ||
Comment 10•19 years ago
|
||
> this name change seems a bit gratuitous to me. personally, i don't
It's not as much a name change as a combining of nsXPCOMInit.cpp
gDirectoryService and (static) nsDirectoryService::mService.
I'm happy to add this category to a header file. Are we comfortable with
freezing it right now and putting it in nsXPCOMCID.h ?
Assignee | ||
Updated•19 years ago
|
Attachment #185300 -
Flags: approval-aviary1.1a2?
Assignee | ||
Comment 11•19 years ago
|
||
Darin, this adds a #define in nsXPCOM.h and also documents a few of our categories and observer topics that I know are frozen.
Attachment #185478 -
Flags: first-review?(darin)
Assignee | ||
Comment 12•19 years ago
|
||
Attachment #185489 -
Flags: first-review?(darin)
Assignee | ||
Updated•19 years ago
|
Attachment #185300 -
Attachment is obsolete: true
Attachment #185300 -
Flags: approval-aviary1.1a2?
Assignee | ||
Comment 13•19 years ago
|
||
I wanted to write this in JS, but that would have required throwing exceptions instead of setting Components.returnCode because of bug 287107. Blech.
Depends on: 287107
Reporter | ||
Comment 14•19 years ago
|
||
So is the first "add a directory list" patch obsolete? It has totally different content than the old one, touching different files...
Assignee | ||
Comment 15•19 years ago
|
||
Comment on attachment 185478 [details] [diff] [review] Directory service changes, rev. 2 [checked in] Whoops, no, attachment 185478 [details] [diff] [review] is a new version of attachment 185300 [details] [diff] [review].
Attachment #185478 -
Attachment description: Add a directory list for internetsearchservice, rev. 2 → Directory service changes, rev. 2
Assignee | ||
Updated•19 years ago
|
Priority: -- → P2
Reporter | ||
Comment 16•19 years ago
|
||
Comment on attachment 185289 [details] [diff] [review] Directory service changes, rev. 1 How come the mac code isn't needed anymore?
Assignee | ||
Comment 17•19 years ago
|
||
The XP_MAC code is OS/9 only.
Comment 18•19 years ago
|
||
Comment on attachment 185478 [details] [diff] [review] Directory service changes, rev. 2 [checked in] nsDirectoryService::RegisterCategoryProviders should use the new #defines that you added to nsXPCOM.h otherwise, this patch looks good to me. r=darin
Attachment #185478 -
Flags: first-review?(darin) → first-review+
Comment 19•19 years ago
|
||
Comment on attachment 185489 [details] [diff] [review] Use a Firefox-specific dirprovider to provide the searchplugins list, rev. 1 [checked in] >Index: toolkit/xre/nsXULAppAPI.h >+#define XRE_EXTENSIONS_DIR_LIST "ExtDL" Hopefully this never collides with something defined by XPCOM. Should we prefix with some kind of namespace? >Index: browser/components/dirprovider/nsBrowserDirectoryProvider.cpp >+ nsCOMPtr<nsIFile> defaults; >+ rv = NS_GetSpecialDirectory(NS_APP_PROFILE_DEFAULTS_50_DIR, >+ getter_AddRefs(defaults)); >+ if (NS_FAILED(rv)) >+ return; >+ >+ defaults->AppendNative(aLeafName); up above, you get the profile directory, and you clone it before using it. presumably in that case you did not need to clone it, or you need to clone it here. I think it's the case that you did not need to clone it up above. Too bad documentation on the directory service is so poor :( r=darin
Attachment #185489 -
Flags: first-review?(darin) → first-review+
Assignee | ||
Comment 20•19 years ago
|
||
Comment on attachment 185489 [details] [diff] [review] Use a Firefox-specific dirprovider to provide the searchplugins list, rev. 1 [checked in] I will rename the key to "XREExtDL". And the cloning is correct: I needed both the profile directory and the "target" file to pass to the Ensure... function.
Attachment #185489 -
Flags: approval-aviary1.1a2?
Assignee | ||
Updated•19 years ago
|
Attachment #185478 -
Flags: approval-aviary1.1a2?
Comment 21•19 years ago
|
||
so are those defines in nsXPCOM.h frozen? if so should you add an @status FROZEN to it?
Assignee | ||
Comment 22•19 years ago
|
||
AIUI everything in nsXPCOM.h is frozen. I can add explicit @status FROZEN if desired.
Comment 23•19 years ago
|
||
the functions in this file all seem to explicitly mention @status FROZEN.
Updated•19 years ago
|
Attachment #185478 -
Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
Updated•19 years ago
|
Attachment #185489 -
Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
Assignee | ||
Updated•19 years ago
|
Attachment #185478 -
Attachment description: Directory service changes, rev. 2 → Directory service changes, rev. 2 [checked in]
Assignee | ||
Updated•19 years ago
|
Attachment #185489 -
Attachment description: Use a Firefox-specific dirprovider to provide the searchplugins list, rev. 1 → Use a Firefox-specific dirprovider to provide the searchplugins list, rev. 1 [checked in]
Comment 24•19 years ago
|
||
http://lxr.mozilla.org/seamonkey/source/browser/components/dirprovider/nsBrowserDirectoryProvider.cpp#105 now claims that our bookmarks file is called bookmarks.rdf instead of .html :-/.
Assignee | ||
Comment 25•19 years ago
|
||
s/bookmarks.rdf/bookmarks.html/ checked in *blush*.
Reporter | ||
Comment 26•19 years ago
|
||
Comment on attachment 185298 [details] [diff] [review] Add a directory list for internetsearchservice, rev. 1 r=mkaply Next time do the Mac removal stuff in a separate patch :)
Attachment #185298 -
Flags: first-review?(mozilla) → first-review+
Assignee | ||
Updated•19 years ago
|
Attachment #185298 -
Flags: approval-aviary1.1a2?
Updated•19 years ago
|
Attachment #185298 -
Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
Comment 27•19 years ago
|
||
Can an extension that comes with search plugins overwrite existing search plugins to lead the use to a malicious site?
Assignee | ||
Comment 28•19 years ago
|
||
> Can an extension that comes with search plugins overwrite existing search
> plugins to lead the use to a malicious site?
No, the search plugins are additive. But since extensions can be as malicious as
they want (to the point of replacing the search box entirely), it's a mostly
moot point.
Assignee | ||
Comment 29•19 years ago
|
||
Fixed on trunk for 1.8b3
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.8beta3
Comment 30•19 years ago
|
||
is stuff like this documented anywhere? I mean how to add the search plugins should they just be in a subdir named "searchplugins" under my extension? Do I need to register them anywhere? In the install.rdf
Comment 31•19 years ago
|
||
see the bottom of http://developer-test.mozilla.org/en/docs/Bundles
Component: XRE Startup → Startup and Profile System
QA Contact: nobody → startup
You need to log in
before you can comment on or make changes to this bug.
Description
•