Closed
Bug 1050080
Opened 10 years ago
Closed 10 years ago
Re-whitelist domains for users of PermissionsUtils.importFromPrefs
Categories
(Firefox :: Settings UI, defect)
Firefox
Settings UI
Tracking
()
People
(Reporter: MattN, Assigned: markh)
References
(Depends on 1 open bug, )
Details
(Keywords: addon-compat)
Attachments
(1 file)
(deleted),
patch
|
MattN
:
review+
|
Details | Diff | Splinter Review |
Due to bug 771630 and bug 506446, users may have lost some of their default whitelists in the permission manager. This breaks things like UITour, LWT previews on AMO, and adds another warning for add-on and app installation. We should redo the whitelisting to help affected users who currently have broken experiences. We may want to do this soon (e.g. for the next UI Tour usage) and again once the two bugs are resolved.
This may be as simple as bumping the version number in the pref names to one in the future.
Of course this will affect users who explicitly removed the permissions but I don't see how we can detect that case. We should probably send an email to dev.platform and the enterprise mailing list to notify people if we do this.
MXR search: https://mxr.mozilla.org/mozilla-central/search?string=whitelist.add
Flags: firefox-backlog+
Updated•10 years ago
|
QA Whiteboard: [qa+]
Updated•10 years ago
|
Points: --- → 3
QA Whiteboard: [qa+]
Flags: qe-verify+
Updated•10 years ago
|
Assignee | ||
Comment 1•10 years ago
|
||
This patch previously had f+ in bug 506446, and that bug just landed on fx-team - assuming it makes it to central, this is the last piece of the puzzle to get these defaults back.
The try from bug 506446 - https://tbpl.mozilla.org/?tree=Try&rev=a3a9210acc3e - included this patch.
MattN, in that other bug you mentioned:
> Once this lands we should send an email to the enterprise mailing list
> to let organizations who currently remove our default permissions to
> know about the new way.
I'm not aware of that list - are you able to send that mail (or point me at the list) once this lands?
Assignee: nobody → mhammond
Attachment #8488427 -
Flags: review?(MattN+bmo)
Updated•10 years ago
|
Iteration: --- → 35.2
Updated•10 years ago
|
Status: NEW → ASSIGNED
Reporter | ||
Comment 2•10 years ago
|
||
Comment on attachment 8488427 [details] [diff] [review]
0001-Bug-1050080-add-default-permissions-for-uitour-and-a.patch
Review of attachment 8488427 [details] [diff] [review]:
-----------------------------------------------------------------
(In reply to Mark Hammond [:markh] from comment #1)
> MattN, in that other bug you mentioned:
>
> > Once this lands we should send an email to the enterprise mailing list
> > to let organizations who currently remove our default permissions to
> > know about the new way.
>
> I'm not aware of that list - are you able to send that mail (or point me at
> the list) once this lands?
https://mail.mozilla.org/listinfo/enterprise
CC'ing firefox-dev would probably be good too.
Attachment #8488427 -
Flags: review?(MattN+bmo) → review+
Updated•10 years ago
|
QA Contact: jbecerra
Assignee | ||
Comment 3•10 years ago
|
||
While drafting the email, I noticed default_permissions also needs the line:
host uitour 1 about:home
Given the patch to firefox.js:
-pref("browser.uitour.whitelist.add.340", "about:home");
I don't think this is worth re-review, but if you disagree, speak now or forever hold your peace ;)
Reporter | ||
Comment 4•10 years ago
|
||
(In reply to Mark Hammond [:markh] from comment #3)
> I don't think this is worth re-review, but if you disagree, speak now or
> forever hold your peace ;)
I agree but you can have r=me again regardless.
Assignee | ||
Comment 5•10 years ago
|
||
Thanks :)
https://hg.mozilla.org/integration/fx-team/rev/1e0d3035de88
Mail to enterprise@ and firefox-dev@ sent.
Comment 6•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Reporter | ||
Comment 7•10 years ago
|
||
Marking addon-compat in case an add-on used PermissionUtils (now removed) or needs to deal with the new behaviour for managing permissions.
Keywords: addon-compat
Reporter | ||
Comment 8•10 years ago
|
||
Mark, any objections to backing this out for causing bug 1072744, bug 1072748, and bug 1072751?
Will a backout cause any problems with the permissions due to the permissions prefs being cleared upon being previously imported? i.e. if the defaults are removed, do they also get lost from profiles that used this patch because the default permissions aren't stored in the database anymore? If this will cause the permissions to get lost until it re-lands then maybe we shouldn't backout fully and instead file a new bug to revert only PermissionsUtils.jsm and XPIProvider.jsm but make sure that reverting XPIProvider.jsm won't mess up desktop permissions. What do you think?
Flags: needinfo?(mhammond)
Assignee | ||
Comment 9•10 years ago
|
||
(In reply to Matthew N. [:MattN] from comment #8)
> Mark, any objections to backing this out for causing bug 1072744, bug
> 1072748, and bug 1072751?
It's a shame, but no.
> Will a backout cause any problems with the permissions due to the
> permissions prefs being cleared upon being previously imported? i.e. if the
> defaults are removed, do they also get lost from profiles that used this
> patch because the default permissions aren't stored in the database anymore?
This shouldn't be a problem - we never removed those old defaults from the database - so if a user still had the defaults written by the pref system they should remain.
> If this will cause the permissions to get lost until it re-lands then maybe
> we shouldn't backout fully and instead file a new bug to revert only
> PermissionsUtils.jsm and XPIProvider.jsm but make sure that reverting
> XPIProvider.jsm won't mess up desktop permissions. What do you think?
That would certainly make relanding this less of a priority and leave us with working permissions for uitour in the scenarios that caused us to take this approach in the first place. OTOH though, if we felt these problems were solvable in the next iteration I think it would be fine to back out fully and re-land.
Flags: needinfo?(mhammond)
Reporter | ||
Updated•10 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•