Add a globally shared location for plugins

RESOLVED FIXED in mozilla1.9.1b2

Status

()

Core
XPCOM
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

(Depends on: 1 bug, {dev-doc-complete})

Trunk
mozilla1.9.1b2
All
Linux
dev-doc-complete
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

9 years ago
Created attachment 325804 [details] [diff] [review]
patch

In the same way /usr/share/mozilla/extensions and /usr/lib/mozilla/extensions are globally shared locations for extensions, the attached patch adds a globally shared location for plugins.
Attachment #325804 - Flags: review?(benjamin)
Comment on attachment 325804 [details] [diff] [review]
patch

I could sworn we already have such a location, but jst is the right person to deal with plugins.
Attachment #325804 - Flags: review?(benjamin) → review?(jst)
Comment on attachment 325804 [details] [diff] [review]
patch

+#elif XP_UNIX
+#define NS_SYSTEM_PLUGINS_DIR       "SysPlugins"
 #endif

- In nsAppFileLocationProvider::GetFiles():

 #else
-        static const char* keys[] = { nsnull, NS_USER_PLUGINS_DIR, NS_APP_PLUGINS_DIR, nsnull };
+        static const char* keys[] = { nsnull, NS_USER_PLUGINS_DIR, NS_APP_PLUGINS_DIR, NS_SYSTEM_PLUGINS_DIR, nsnull };

Here we're in the else case of an #if XP_MACOSX, which means XP_UNIX NS_SYSTEM_PLUGINS_DIR is not defined. This needs to be in an #ifdef XP_UNIX block, with the else case remaining unchanged. r- based on that, other than that this looks good.
Attachment #325804 - Flags: review?(jst) → review-
(Assignee)

Comment 3

9 years ago
Created attachment 338664 [details] [diff] [review]
patch v2
[Checkin: Comment 9]

Sorry for the delay
Attachment #325804 - Attachment is obsolete: true
Attachment #338664 - Flags: review?(jst)

Updated

9 years ago
Attachment #338664 - Flags: superreview+
Attachment #338664 - Flags: review?(jst)
Attachment #338664 - Flags: review?(joshmoz)
Comment on attachment 338664 [details] [diff] [review]
patch v2
[Checkin: Comment 9]

sr=jst. Josh, does this look good to you?

Comment 5

9 years ago
Comment on attachment 338664 [details] [diff] [review]
patch v2
[Checkin: Comment 9]

Why put the plugins under a directory labeled "mozilla"? As in:

"/usr/lib/mozilla/plugins"

Wouldn't it be better to set a standard for plugins targeted at any browser? Like:

"/usr/lib/webbrowserplugins"

That way webkit and opera browsers can feel first-class using it and installs could be simplified.
(Assignee)

Comment 6

9 years ago
(In reply to comment #5)
> (From update of attachment 338664 [details] [diff] [review])
> Why put the plugins under a directory labeled "mozilla"? As in:
> 
> "/usr/lib/mozilla/plugins"
> 
> Wouldn't it be better to set a standard for plugins targeted at any browser?
> Like:
> 
> "/usr/lib/webbrowserplugins"
> 
> That way webkit and opera browsers can feel first-class using it and installs
> could be simplified.

While /usr/lib/webbrowserplugins would be nice, /usr/lib/mozilla/plugins is a place where other browsers already look plugins for, and where I guess most distros put plugins already.
(In reply to comment #5)
> Wouldn't it be better to set a standard for plugins targeted at any browser?
> Like:
> 
> "/usr/lib/webbrowserplugins"

I wouldn't have anything against us looking there too, but from Mike's comment above it sounds like other browsers already look for plugins, and thus some plugin installers might install plugins there (whether we'll find them or not). So I'd say we go with this patch, and another patch (or both rolled into one here would be fine too) that adds the above path as well.

Updated

9 years ago
Attachment #338664 - Flags: review?(joshmoz) → review+

Comment 8

9 years ago
Comment on attachment 338664 [details] [diff] [review]
patch v2
[Checkin: Comment 9]

I'll take your word for it that there are already dependencies on this location, seems odd though given that we don't use it ourselves.
Assignee: nobody → mh+mozilla
Keywords: checkin-needed
Comment on attachment 338664 [details] [diff] [review]
patch v2
[Checkin: Comment 9]

http://hg.mozilla.org/mozilla-central/rev/dda85c54599d
Attachment #338664 - Attachment description: patch v2 → patch v2 [Checkin: Comment 9]
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Flags: in-testsuite-
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b2
Keywords: dev-doc-needed

Updated

9 years ago
Depends on: 467751
These constants aren't documented anywhere at all yet.  Any opinions on where they should be covered?
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.