Popup blocking preferences allow input when no data is in whitelist URL field

RESOLVED FIXED in Camino1.5

Status

defect
RESOLVED FIXED
13 years ago
12 years ago

People

(Reporter: bugzilla-graveyard, Assigned: stuart.morgan+bugzilla)

Tracking

({fixed1.8.1.1, polish})

Details

Attachments

(1 attachment)

fix
1.01 KB, patch
bugzilla-graveyard
: review+
mikepinkerton
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

13 years ago
On the 2006101622 trunk build...

1) Open Prefs -> Web Features.
2) Edit popup exceptions list.
3) Focus should be in "Site" field. If it isn't, put the cursor there.
4) Hit return.
5) Note "scheme:http" entry added to whitelist.

We should probably prevent this from happening, since it would potentially allow fairly basic users to accidentally whitelist ALL popups. I suggest that we should disable the "Add" button unless there's data in the "Site" field.

I'm filing this as UNCO as I'm currently unable to reproduce it. I have no idea what happened in the five minutes since I started filing the bug.
(Assignee)

Comment 1

13 years ago
WFM.  Add is enabled only when there's text.
STR:

1. Whitelist a site manually (including clicking Add)
2. Press return

It seems like we never re-validate the field after you've added one site (Add is *always* enabled), no matter what you do.

We do revalidate if I open the sheet, type a space, and then delete the space, but only if you've never hit Add in that sheet-session.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 3

13 years ago
2a) Add a site (example.com is safe) to the whitelist.

That'll get it to happen reliably.

cl
(Assignee)

Comment 4

13 years ago
Posted patch fixSplinter Review
Assignee: nobody → stuart.morgan
Status: NEW → ASSIGNED
Attachment #243113 - Flags: review?(bugzilla)
(Reporter)

Comment 5

13 years ago
Comment on attachment 243113 [details] [diff] [review]
fix

Looks good and fixes the problem. Not sure why diff decided on the change there in |mAddField setStringValue| but whatever.

cl
Attachment #243113 - Flags: review?(bugzilla) → review+
(Assignee)

Comment 6

13 years ago
Comment on attachment 243113 [details] [diff] [review]
fix

> Looks good and fixes the problem. Not sure why diff decided on the change there
> in |mAddField setStringValue| but whatever.

Because I removed trailing whitespace from that line.
Attachment #243113 - Flags: superreview?(mikepinkerton)
Comment on attachment 243113 [details] [diff] [review]
fix

sr=pink
Attachment #243113 - Flags: superreview?(mikepinkerton) → superreview+
Checked in on 1.8branch and trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Whiteboard: [needs checkin]

Updated

13 years ago
Keywords: fixed1.8.1.1

Updated

13 years ago
Keywords: fixed1.8.1
Did we decide to punt on validation for this field on this bug, e.g., preventing the ability to whitelist, say, " "?
Moving fixed "1.2" bugs to 1.1 where they were really fixed. Filter on CaminoFixed1.1 for bugmail purposes.
Target Milestone: Camino1.2 → Camino1.1
You need to log in before you can comment on or make changes to this bug.