310619 - Allow editing of entries in cookie exceptions dialog

Status: REOPENED

(Firefox :: Settings UI, enhancement)

Description

[Adam Guthrie]

When clicking on an entry in the cookie exceptions dialog the entires URL should
appear in the "Address of the website" input box and the "Block" "Allow for
session" and "Allow" should be enabled.

1. Tools > Options > Privacy > Cookies > Exceptions
2. Click on an entry in the tree.

Comment 1

[Adam Guthrie]

*** Bug 310207 has been marked as a duplicate of this bug. ***

Comment 2

[Asaf Romano (gone)]

*** This bug has been marked as a duplicate of 87296 ***

Comment 3

[maenneph]

This bug is NOT a duplicate of bug 87296, which is about editing the cookies itself, whereas this bug is about editing the whitelisting / blacklisting rules for cookes.

Clarifying the issue:

Steps to reproduce:

1. open Tools > Options > Privacy > Cookies > Exceptions
2. create a new exception rule (either allow, allow for session or block)
3. select some previously created entry in the listbox

Actual result: The Buttons 'block', 'allow' and 'allow for session' are greyed out and not operational. The only way to change the exception rule is to delete and newly create it. Right-clicking the listbox to change the properties also has no effect.

Expected result: The rules of selected item(s) of the listbox can be edited by clicking the provided buttons or at least be edited by right-clicking the item(s). If only one item is selected the URL should also be editable in the address box.

Note that deleting and newly creating exception entries is error-prone and is getting tiresome quickly with many exception rules. This is indeed an annoying bug, not only RFE.

Please change the severity accordingly.

Comment 4

[:ytx]

I'd like to work on this bug, could someone assign me please?

Comment 5

[:ytx]

While working on this bug, I noticed that the Permission object in http://dxr.mozilla.org/mozilla-central/source/browser/components/preferences/permissions.js#11 has 2 attributes which seem redundant, perm (which is the int permission type) and capability (which is a string representation of the permission type).

When an item in the array gets updated, only capability is also updated. This leads to a problem at http://dxr.mozilla.org/mozilla-central/source/browser/components/preferences/permissions.js#108, where when adding an entry, perm is accessed to check whether a permission already exists.

In my patch I would use a similar check to prevent updating an entry with the same permission, so this problem affects my patch too.

Should I file a bug about this? Should the test at line 108 be updated to check the capability attribute, and should the perm attribute be removed, or should the script be updated so that the perm attribute is also updated when changing a permission?

Comment 6

[Dão Gottwald [:dao]]

(In reply to :ytx from comment #5)
> When an item in the array gets updated, only capability is also updated.
> This leads to a problem at
> http://dxr.mozilla.org/mozilla-central/source/browser/components/preferences/
> permissions.js#108, where when adding an entry, perm is accessed to check
> whether a permission already exists.
> 
> In my patch I would use a similar check to prevent updating an entry with
> the same permission, so this problem affects my patch too.

Not sure how exactly this problem manifests in the UI.

> Should I file a bug about this?

Sounds like a separate bug, yes.

> Should the test at line 108 be updated to
> check the capability attribute, and should the perm attribute be removed, or
> should the script be updated so that the perm attribute is also updated when
> changing a permission?

I'm not entirely sure what will make more sense for permissions.js, but if the perm attribute is redundant, it sounds like it should be removed.

Comment 7

[:ytx]

Attachment: Patch

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 8355764 [details] [diff] [review]
Patch

I'm not sure double-click is the best choice here, or that we need the context menu.

How about updating the domain field whenever an item is selected (e.g. via single click) and getting rid of the context menu?

Comment 9

[:ytx]

(In reply to Dão Gottwald [:dao] from comment #8)

> How about updating the domain field whenever an item is selected (e.g. via
> single click)

Well, the first item in the list is selected by default, so this would cause text in the domain field when opening the window, which could be irritating to the user.
I'm not sure if there is a way to check for default selection, if there is, this is no problem, if not, the textbox could alternatively only be updated when the user actually single-clicks an entry.

> and getting rid of the context menu?

Updating the textbox doesn't work well with multiple entries, as copying all URLs in the textbox makes it quite unreadable and there'd have to be parsing to separate multiple URLs.

Multiple entries could be handled by using a context menu, or by enabling the buttons without updating the textbox, though this may be irritating as it's not clear that the buttons relate to the selected entries and not to adding a new entry.

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8355764 [details] [diff] [review]
Patch

Clearing old review request. The patch probably needs to be rebased.

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:jaws, could you have a look please?
For more information, please visit auto_nag documentation.