230462 - Options dialog can't be closed with OK button after opening "Cookies | Exceptions"

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P4)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7a) Gecko/20040108
Firebird/0.8.0+ (MozJF)

After opening "Cookies | Exceptions" the options dialog can't be closed with OK
button.

Steps to reproduce:
1. Open "Tools | Options"
2. Open "Cookies | Exceptions" and close it directly
3. Ok button doesn't react on user event

Comment 1

[Mike Connor [:mconnor]]

this is WFM on the most recent 0.8 branch build.  Does the javascript console
show any errors when this happens?  If so please copy/paste here.

Have you tested on a clean profile?

Comment 2

[Cedric Roijakkers]

The same thing happens when adding a site to the popup blocker whitelist.

- Open Tools -> Options
- Click on Web Features
- Click Add Site
- Enter Something
- Click OK
- Go to advanced and change a random option
- The OK button on the option screen doesn't react anymore

This problem still seems to be there in the 0.7+ versions.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Yes it works for me with a new profile. Within my regular profile the javascript
console is reporting following error message:

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

It appears only when I close the Exception dialog with OK. Cancel will not raise
an error.

After adding a new site to the popup blocker list i can normally close the
dialog. Cedric what says your javascript console?

Comment 4

[Ashley Smith]

Confirmed this using instructions in comment #2.  Repeated three times in a row.
 Similar problems to this one have been around for a while, but were difficult
to reproduce.

Checked for duplicates, found 229022, which should be closed as this report has
better details for reproducing the bug.

Tested using:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20040102
Firebird/0.7+

Comment 5

[Michael Hendy (Hendikins)]

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

Comment 6

[Mike Connor [:mconnor]]

the popup blocking bug should be fixed on post-12/18 builds, I wrote the patch

if this is working on a new profile, but not your existing profile, what's wrong
with your existing profile?  If there's malformed URIs in the permissions list,
that could certainly cause it.

Can you copy cookperm.txt out of your affected profile and either attach it here
or email it to me?  Other than having your permissions white/blacklist, there
isn't much to worry about from a privacy perspective.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I figured out which entry inside the cookieperm.txt is responsible for the bug:
"205.134.166.50:8080	0F" 

Seams to get in trouble by setting an entry with ip address and port. This entry
was not added manually instead with the dialog which popups for request to store
or not. Manually I can't add such an entry within the exceptions dialog, except
by adding "http://" in front of the ip. But after reopening this dialog you can
see that only "205.134.166.50" without the port was stored.

Using no ip but a domainname like "yahoo.com:8000" the data is also not stored
right after adding it manually. There was only "8000" stored - but where is the
domain?

Comment 8

[Mike Connor [:mconnor]]

I knew I should have really looked at cookie exceptions about this when I fixed
popup blocking.  Ports are irrelevant now, so it should never allow you to add
an entry with ports.  See bug 225236 for more info.

Was that entry added recently using a current build or is it pre-existing from
0.7?  If recently, can you give me a link to the site that created it so I can test?

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Sorry, but seems to be very old and I don't know which site has ask for
permission to set this cookie. 

Why ports are irrelevant? If I have a website laying on another port as 80 of my
server I want's to have a cookie directly for this VirtualHost. For example: You
have the same website one time for site.de (official) and for testing on
site.de:8080. I have the same cookie handling and store user data. So both sites
using the same cookie in that way? It shouldn't because we have different sites!

Comment 10

[Mike Connor [:mconnor]]

in the 1.6 cycle the cookie handling was changed to ignore ports.  This is a
backend change, and with the exception of testing purposes for your example, its
irrelevant for nearly everyone.  In any case, its far beyond the scope of this bug.

Comment 11

[Mike Connor [:mconnor]]

Attachment: patch including other fixes

in addition to validating on input instead of on OK to fix this bug, this also
includes support for session-only permissions, and allows for replacing
existing permissions without deleting the previous permission (syncing with the
Seamonkey impl in this regard).

ALLOW_SESSION_ONLY is a hack until dwitte makes it a publicly defined value,
the same const is used in seamonkey as well, and will be fixed treewide once
this is fixed.

Comment 12

[Mike Connor [:mconnor]]

Comment on attachment 138740 [details] [diff] [review]
patch including other fixes

no huge rush on this one, as long as we can get it in before the dialogs get
fixed to allow session permissions.  This certainly is a trunk-only patch

Comment 13

[Cedric Roijakkers]

I just installed a fresh 3 feb 2004 build of firebird (with a clean profile, and
without my .js config files).

Added a site to the popup blocker whitelist, then switched to another field in
the Options dialog box (as stated in my comment #2). Apparently the OK-button
not working bug is still there.

FYI: There is absolutely nothing in my Javascript Console.

Comment 14

[Peter Edstrom]

concerning comment #2 and comment #4, I find the bug can be reached by a simpler
process provided that sites are already listed. I have two sites that allow
popup's, both of which list a specific port.

- Open Tools -> Options
- Click on Web Features
- The OK button on the option screen doesn't react anymore

Comment 15

[Mike Connor [:mconnor]]

if you have invalid sites already in the list, its not fixed.  Those entries
should be removed.

Comment 16

[temcat]

The same bug is consistently reproduced on theme changing:

1) Choose Tools->Options->Themes
2) Click on the desired theme
3) Click OK. The theme will change, but the dialog won't close. Additional OK
won't do it, either -- you have to choose Cancel to close the window.

Sometimes (but not always) this happens when I change font settings.

Comment 17

[Mike Connor [:mconnor]]

Comment on attachment 138740 [details] [diff] [review]
patch including other fixes

moving this to blake

Comment 18

[Mike Connor [:mconnor]]

Attachment: updated to trunk

this patch also adds support for the allow for session cookie permission that's
exposed in the cookie dialogs.

Comment 19

[Mike Connor [:mconnor]]

Comment on attachment 146383 [details] [diff] [review]
updated to trunk

more stuff that'll sit in a queue until feature hell ends

Comment 20

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

Doesn't look like the pref UI bustage still occurs... what does the addt'l UI
look like, Mike?

Comment 21

[Mike Connor [:mconnor]]

damn,  this has l10n impact too.  plussing to get on my own priority list, need
to update to aviary tip what with the l10n stuff, then I can take screenshots :)

Comment 22

[Mike Connor [:mconnor]]

if this misses RC1, it misses everything, I should set these right :)

Comment 23

[Mike Connor [:mconnor]]

Comment on attachment 146383 [details] [diff] [review]
updated to trunk

hooray for bitrot

Comment 24

[Mike Connor [:mconnor]]

Attachment: updated to aviary tip

bitrot was an understatement.

Comment 25

[Mike Connor [:mconnor]]

Comment on attachment 155375 [details] [diff] [review]
updated to aviary tip

buncha stuff here: fix this bug, add support for the session perms (since the
cookie dialog does, multiple dupes of the weirdness there), remove some crufty
unused strings for themes/extensions, fix some oddness with button enabling in
the exceptions window (remove all never enabled once you added perms, unless
you then deleted some, and the block/allow buttons stayed active in certain
cases.	All stuff for PR1, so quick is good.

Comment 26

[Mike Connor [:mconnor]]

Attachment: big scary prefwindow patch

fixes 230462 and friends
fixes bug 241526 (for the prefpanel, at least)
fixes bug 252953

Comment 27

[Mike Connor [:mconnor]]

Comment on attachment 155402 [details] [diff] [review]
big scary prefwindow patch

blake, if you could take a look at this soon it'd be great

Comment 28

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

Mike, are the pref locking cases contained within this patch indicative of
special cases where disabling is needed etc - i.e. are all the other checkboxes,
fields etc throughout prefs disabled automatically by the window framework?

Comment 29

[Mike Connor [:mconnor]]

yes, the pref locking cases here are only those cases where we are doing manual
enabling/disabling.  In "normal" cases the prefwindow API handles it properly. 
I played with this a LOT and it works very consistently.

Comment 30

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

Comment on attachment 155402 [details] [diff] [review]
big scary prefwindow patch

r+a=[e-mail]

Comment 31

[chris hofmann]

did this one land yet?

Comment 32

[Simon Paquet [:sipaq]]

This has been checked into the Aviary-branch. The bug is left open for trunk
checkin.

Comment 33

[Henrik Skupin [:whimboo][⌚️UTC+2]]

v.