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

RESOLVED FIXED in Camino1.5

Status

Camino Graveyard
Preferences
RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: Chris Lawson (gone), Assigned: Stuart Morgan)

Tracking

({fixed1.8.1.1, polish})

Trunk
Camino1.5
PowerPC
Mac OS X
fixed1.8.1.1, polish

Details

Attachments

(1 attachment)

fix
1.01 KB, patch
Chris Lawson (gone)
: review+
Mike Pinkerton (not reading bugmail)
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

11 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

11 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

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

That'll get it to happen reliably.

cl
(Assignee)

Comment 4

11 years ago
Created attachment 243113 [details] [diff] [review]
fix
Assignee: nobody → stuart.morgan
Status: NEW → ASSIGNED
Attachment #243113 - Flags: review?(bugzilla)
(Reporter)

Comment 5

11 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

11 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+
Whiteboard: [needs checkin]

Comment 8

11 years ago
Checked in on 1.8branch and trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Whiteboard: [needs checkin]

Updated

11 years ago
Keywords: fixed1.8.1.1

Updated

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