225236 - Entering an invalid URI for popup whitelist breaks OK button

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Licinio Alves]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5) Gecko/20031007 Firebird/0.7
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5) Gecko/20031007 Firebird/0.7

Tools->Options->Web Features->Block PopUp Windows

unable to enter a site to allow popups with a port
e.g. 123.45.67.1:8080

It allows me to Add/enter such a site, but then I can't click on OK to close the
options window.  I have to delete the site with the port number then I can close
the options window.  

So my only alternative now is to allow all popups to work on my application that
has a JavaScript popup window.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[[e-mail]]

most likely an uncaught exception in chrome, resulting from |new nsIURI|
failing... are there any errors reported in the js console?

mvl?

Comment 2

[Licinio Alves]

yes

Error: uncaught exception: [Exception... "Component returned failure code:
0x804b000a [nsIURI.spec]"  nsresult: "0x804b000a (<unknown>)"  location: "JS
frame :: chrome://browser/content/pref/pref-features.js :: onPopupPrefsOK ::
line 179"  data: no]

Comment 3

[[e-mail]]

the line is:

134       uri.spec = dataObject.permissions[p].host;

and the exception thrown is an NS_ERROR_MALFORMED_URI nsresult.

anyway we don't support ports anymore, so maybe we need code to strip the port
out of the text string the user enters?

Comment 4

[Licinio Alves]

Stripping out ports is fine, as long as I can add a site to allow popups from
and it allows popups from all ports on that site, which it corrently does not.

Comment 5

[[e-mail]]

that's because you're using fb0.7. get trunk :)

so fb's UI is fundamentally broken here, because it just adds the strings the
user enters into a list, and never processes them until the user hits OK. it
should at least try to convert them into an nsIURI object at entry time - and
throw an error dialog if an exception is caught. that's the only real mechanism
for detecting incorrect user input, and fb can't catch it.

seamonkey does better; we throw a dialog if the URI had a parse error.

mvl, btw, i found a hostPort!
http://lxr.mozilla.org/seamonkey/source/xpfe/communicator/resources/content/popupManager.js#331

Comment 6

[[e-mail]]

fyi: that hostPort isn't really critical - but it's inconsistent since if a user
enters foo.com:8080 then on pressing 'add' it will still read foo.com:8080,
which implies the port has meaning. nicer to rip off the port so it reads
foo.com, and then the user knows the port is meaningless ;)

mconnor, wanna patch?

Comment 7

[[e-mail]]

-> fb

Comment 8

[Jon Henry]

I imagine this will be fixed by the patch for this bug, but I noticed that when
entering a site with a port (for example, foo.com:8080) and clicking OK in the
Firebird options panel, everything seems to work; the site is added to the list.
However, the next time you load the options dialog the entry for that site is at
the top of the allowed popups list and simply reads "8080", the domain name has
been stripped.

Comment 9

[Mike Connor [:mconnor]]

-> me

easy enough to handle, and I should be able to make it for 0.8

Comment 10

[Mike Connor [:mconnor]]

Attachment: patch

basically uses a similar process to seamonkey for getting the valid URI out of
the entered string.  ports are stripped using uri.host (formerly known as
uri.hostPort).	There's also allowance for something completely failing to get
reduced to a URI, whereby there is an error dialog followed by the original
dialog for the user to correct the entered value.  We will rarely, if ever, hit
this, but it will eliminate the potential for the OK button bustage that
prompted this bug report.

Comment 11

[Licinio Alves]

Will this just fix the problem of adding a site with a port to the list of sites
allowed to display popups.

Will Firebird actually then allow all popups for the specified (or all ports) of
that site, e.g. 8080?
Originally, when I had entered the site, without the port 8080, it still blocked
port 8080 generated popups.

Comment 12

[Mike Connor [:mconnor]]

on trunk, port is no longer relevant for permissions.
so mozilla.org in the perms list will allow for mozilla.org:80, :8080, and 
actually given that its a root domain, bugzilla.mozilla.org too.

Comment 13

[Ben Goodger (use ben at mozilla dot org for email)]

Patch checked in. Thanks Mike!

Comment 14

[Mike Connor [:mconnor]]

Comment on attachment 135459 [details] [diff] [review]
patch

obsolete review request, patch checked in

Comment 15

[alanjstr]

Pre-release cleanup

Comment 16

[Mike Connor [:mconnor]]

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

Comment 17

[Mike Connor [:mconnor]]

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