Closed Bug 357597 Opened 18 years ago Closed 18 years ago

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

Categories

(Camino Graveyard :: Preferences, defect)

PowerPC
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Camino1.5

People

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

Details

(Keywords: fixed1.8.1.1, polish)

Attachments

(1 file)

1.01 KB, patch
bugzilla-graveyard
: review+
mikepinkerton
: superreview+
Details | Diff | Splinter Review
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.
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
2a) Add a site (example.com is safe) to the whitelist.

That'll get it to happen reliably.

cl
Attached patch fixSplinter Review
Assignee: nobody → stuart.morgan
Status: NEW → ASSIGNED
Attachment #243113 - Flags: review?(bugzilla)
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+
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
Closed: 18 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Whiteboard: [needs checkin]
Keywords: fixed1.8.1.1
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.

Attachment

General

Creator:
Created:
Updated:
Size: