Closed Bug 1072751 Opened 5 years ago Closed 5 years ago

Switch SeaMonkey from xpinstall.whitelist.add to using a default permissions file

Categories

(SeaMonkey :: Passwords & Permissions, defect, major)

defect
Not set
major

Tracking

(seamonkey2.35+ fixed, seamonkey2.36 fixed)

RESOLVED FIXED
seamonkey2.36
Tracking Status
seamonkey2.35 + fixed
seamonkey2.36 --- fixed

People

(Reporter: MattN, Assigned: philip.chee)

References

()

Details

(Keywords: addon-compat)

Attachments

(1 file, 1 obsolete file)

Bug 1050080 needs to be ported to Seamonkey to unbreak add-on/LWT installation (perhaps just in new profiles).

Bug 506446 hardcoded a path of "resource://app/chrome/browser/default_permissions" for the default location but I don't know if that works for Seamonkey so may need to be changed or overridden via the permissions.manager.defaultsUrl preference.

MXR search: https://mxr.mozilla.org/comm-central/search?string=whitelist.add
Summary: Switch from xpinstall.whitelist.add to using a default permissions file → Switch SeaMonkey from xpinstall.whitelist.add to using a default permissions file
Attached patch WIP Patch v0.1 (obsolete) — Splinter Review
> +JS_PREFERENCE_FILES += [
> +    'permissions',

Currently I'm using moz.build + JS_PREFERENCE_FILES so it goes into resource:///defaults/pref/permissions.
I think it should go into resource:///defaults/permissions but I don't know the correct makefile incantation to do that.

I stuck the permissions file in suite/app/ since this corresponds to where Firefox puts it but of course we can put it anywhere in /suite/.

> +host	install	1	downloads.mozdev.org
This isn't in Firefox but I'm adding it BECAUSE mozdev.
Assignee: nobody → philip.chee
Status: NEW → ASSIGNED
Attachment #8564761 - Flags: feedback?(neil)
Attachment #8564761 - Flags: feedback?(mnyromyr)
Attachment #8564761 - Flags: feedback?(iann_bugzilla)
Note that will use resource://app/defaults/permissions once bug 1073095 is merged.
(In reply to Magnus Melin from comment #2)
> Note that will use resource://app/defaults/permissions once bug 1073095 is
> merged.

Don't use resource://app/ instead use resource:///
What is the advantage?
(In reply to Magnus Melin from comment #4)
> What is the advantage?
See Bug 555715 - Replace resource://app/ with resource:///
http://hg.mozilla.org/mozilla-central/rev/bab28d839f5e
Bug 620931 part 4 - Fix resource://app/ to always point to the same as resource:///
(In reply to Philip Chee from comment #1)
> I think it should go into resource:///defaults/permissions but I don't know
> the correct makefile incantation to do that.
Seems to be something like
DEFAULTS_FILES := permissions
DEFAULTS_DEST := $(FINAL_TARGET)/defaults
DEFAULTS_TIER := misc
INSTALL_TARGETS += DEFAULTS
Comment on attachment 8564761 [details] [diff] [review]
WIP Patch v0.1

>+@RESPATH@/@PREF_DIR@/permissions
This path is wrong.
Attachment #8564761 - Flags: feedback?(neil)
This patch:
1. Uses FINAL_TARGET_FILES to put the file in dist/bin/defaults
2. package-manifest.in then packages the file in omni.ja!/defaults/permissions
3. Which is then accessible as resource:///defaults/permissions
Attachment #8564761 - Attachment is obsolete: true
Attachment #8564761 - Flags: feedback?(mnyromyr)
Attachment #8564761 - Flags: feedback?(iann_bugzilla)
Attachment #8567509 - Flags: review?(iann_bugzilla)
Component: General → Passwords & Permissions
Attachment #8567509 - Flags: review?(iann_bugzilla) → review+
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/509aed2cbd10
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.36
Comment on attachment 8567509 [details] [diff] [review]
Patch v2 use FINAL_TARGET_FILES instead

[Approval Request Comment]
Regression caused by (bug #): Bug 1050080
User impact if declined: broken add-on/LWT installation (perhaps just in new profiles).
Testing completed (on m-c, etc.): has been baked in comm-central for ages. 
Risk to taking this patch (and alternatives if risky): Breaks some tests[1]
String changes made by this patch: None.

[1] bug 1072748 comment 10
> The very odd thing here is that the test failure verifies things work in
> thunderbird.
> 
> Did some investigation and
> extensions/cookie/test/unit/test_permmanager_notifications.js seems to work
> by accident for firefox. pm.removeAll() calls
> nsPermissionManager::ImportDefaults in
> http://mxr.mozilla.org/comm-central/source/mozilla/extensions/cookie/
> nsPermissionManager.cpp?force=1#1089 so the default permissions are
> re-imported => the test that there aren't any permissions left should fail -
> http://mxr.mozilla.org/comm-central/source/mozilla/extensions/cookie/test/
> unit/test_permmanager_notifications.js#128
> 
> This turns out to be because the permissions.manager.defaultsUrl pref is not
> set. 
> This is a pref set in firefox.js. Even if I set
> permissions.manager.defaultsUrl to resource://app/defaults/permissions in
> the test it turns out that URL can't be read (ImportDefaults can't get an
> inputStream) - which isn't true for firefox itself though.
> In thunderbird the pref is set and the import succeeds, so the test fails.
> 
> So, are the tests supposed to be run with application prefs present or just
> as components?
> And how come that's different in thunderbird vs firefox?
Attachment #8567509 - Flags: approval-comm-release?
(In reply to Philip Chee from comment #11)
> Comment on attachment 8567509 [details] [diff] [review]
> Patch v2 use FINAL_TARGET_FILES instead
> 
> [Approval Request Comment]
> Regression caused by (bug #): Bug 1050080
> User impact if declined: broken add-on/LWT installation (perhaps just in new
> profiles).
> Testing completed (on m-c, etc.): has been baked in comm-central for ages. 
> Risk to taking this patch (and alternatives if risky): Breaks some tests[1]
> String changes made by this patch: None.

Flags: approval-comm-release?
Flags: needinfo?(iann_bugzilla)
Flags: needinfo?(iann_bugzilla)
Attachment #8567509 - Flags: approval-comm-release? → approval-comm-release+
Depends on: 1190233
You need to log in before you can comment on or make changes to this bug.