1050080 - Re-whitelist domains for users of PermissionsUtils.importFromPrefs

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect)

Description

[Matthew N. [:MattN]]

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

Comment 1

[Mark Hammond [:markh] [:mhammond]]

Attachment: 0001-Bug-1050080-add-default-permissions-for-uitour-and-a.patch

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?

Comment 2

[Matthew N. [:MattN]]

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.

Comment 3

[Mark Hammond [:markh] [:mhammond]]

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 ;)

Comment 4

[Matthew N. [:MattN]]

(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.

Comment 5

[Mark Hammond [:markh] [:mhammond]]

Thanks :)

https://hg.mozilla.org/integration/fx-team/rev/1e0d3035de88

Mail to enterprise@ and firefox-dev@ sent.

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/1e0d3035de88

Comment 7

[Matthew N. [:MattN]]

Marking addon-compat in case an add-on used PermissionUtils (now removed) or needs to deal with the new behaviour for managing permissions.

Comment 8

[Matthew N. [:MattN]]

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?

Comment 9

[Mark Hammond [:markh] [:mhammond]]

(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.