1422397 - Rename browser/components/preferences/permissions.{xul,js} to exceptions.{xul,js}

Status: RESOLVED WONTFIX

(Firefox :: Settings UI, enhancement, P5)

Description

[Johann Hofmann [:johannh]]

Any consumer for the permissions.xul window uses it to define exceptions for some global setting (cookies, TP, popups, passwords, addons):

https://searchfox.org/mozilla-central/search?q=permissions.xul&path=

While those exceptions are technically defined through permissions, this is easily confused with the new sitePermissions.xul dialog which was added to allow the user to manage actual permissions, from the user's point of view.

We should probably rename permissions.xul to exceptions.xul, and knowing that this is likely to be the only use case for this dialog there may be additional opportunities for cleanups.

Comment 1

[:prathiksha]

Attachment: Bug 1422397 - Rename browser/components/preferences/permissions.{xul,js} to exceptions.{xul,js}.

Review commit: https://reviewboard.mozilla.org/r/204814/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/204814/

Comment 2

[Johann Hofmann [:johannh]]

Comment on attachment 8933877 [details]
Bug 1422397 - Rename browser/components/preferences/permissions.{xul,js} to exceptions.{xul,js}.

https://reviewboard.mozilla.org/r/204814/#review210324

Thanks for picking this up!

Please use hg mv instead of simply moving the file. That preserves history. Please also rename references to these files in the code.

Did you get a chance to take a look at whether we can eliminate code from one of the files by assuming that we're only dealing with exceptions to things?

Comment 3

[:prathiksha]

Comment on attachment 8933877 [details]
Bug 1422397 - Rename browser/components/preferences/permissions.{xul,js} to exceptions.{xul,js}.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/204814/diff/1-2/

Comment 4

[:prathiksha]

(In reply to Johann Hofmann [:johannh] from comment #2)

> Did you get a chance to take a look at whether we can eliminate code from
> one of the files by assuming that we're only dealing with exceptions to
> things?

I took a look at permissions.js. I don't think we can remove any code from it as of now. Some code in permissions.js will become redundant if we choose to make cookie exceptions use sitePermissions.xul and sitePermissions.js. I think we should make the cookie exception dialog use sitePermissions.xul in the future because the cookie dialog looks more like the dialogs we use for Location, Microphone, etc than other "Exceptions" dialogs.

Comment 5

[:prathiksha]

I feel like I was not clear in my previous comment. I meant to say, it'd be more appropriate to use sitePermissions.xul with the menulist and all for the cookie exception dialog.

Comment 6

[Johann Hofmann [:johannh]]

Comment on attachment 8933877 [details]
Bug 1422397 - Rename browser/components/preferences/permissions.{xul,js} to exceptions.{xul,js}.

https://reviewboard.mozilla.org/r/204814/#review212796

Hmm looking at this code I notice there are a lot more occurrences of "permissions" in here. These would probably also have to be renamed at which point I'm not sure whether the whole renaming is still worth the effort. I mostly filed this bug as a random idea for cleanup more than a direct action item and we shouldn't make the code base more inconsistent for the sake of it.

With that said, if you feel like working on this I'm happy to continue to help!

Thank you for looking into this and sorry for the mess.