Allow editing of entries in cookie exceptions dialog

REOPENED
Assigned to

Status

()

--
enhancement
REOPENED
13 years ago
2 years ago

People

(Reporter: ispiked, Assigned: ytx)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

13 years ago
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.
(Reporter)

Comment 1

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

*** This bug has been marked as a duplicate of 87296 ***
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → DUPLICATE
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---

Comment 3

13 years ago
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.
(Assignee)

Comment 4

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

Updated

5 years ago
Assignee: nobody → ytxmoz
(Assignee)

Comment 5

5 years ago
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?
Flags: needinfo?
(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.
Flags: needinfo?
(Assignee)

Updated

5 years ago
Depends on: 956151
(Assignee)

Comment 7

5 years ago
Created attachment 8355764 [details] [diff] [review]
Patch
Attachment #8355764 - Flags: review?(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?
(Assignee)

Comment 9

5 years ago
(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 on attachment 8355764 [details] [diff] [review]
Patch

Clearing old review request. The patch probably needs to be rebased.
Attachment #8355764 - Flags: review?(dao+bmo)
You need to log in before you can comment on or make changes to this bug.