Closed Bug 101669 Opened 24 years ago Closed 24 years ago

Form does not submit if insecure submission warning active

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Platform:
SGI
IRIX
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: nickb, Assigned: alexsavulov)

References

(
URL
)

Details

Attachments

(3 files)

security/manager/ssl/src/nsSecureBrowserUIImpl.cpp
659 bytes, patch
alexsavulov
: review+
Details | Diff | Splinter Review
security/manager/ssl/src/nsSecureBrowserUIImpl.cpp - improves comments
914 bytes, patch
Details | Diff | Splinter Review
proposed fix
623 bytes, patch
alexsavulov
: review+
attinasi
: superreview+
Details | Diff | Splinter Review
From Bugzilla Helper: User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.4) Gecko/20010913 BuildID: 2001092512 If a form is completed (eg www.google.com) and either enter or the submit button is pressed, the insecure submission dialog is shown, but the form is not submitted (regardless of whether continue or cancel is pressed, or the enter key is used -- providing the "Alert me whenever I submit..." check box is left checked). Reproducible: Always Steps to Reproduce: 1. Go to Edit->Preferences->Privacy&Security->SSL->Sending form data from an unencrypted page to an unencrypted page == checked. 2. Go to www.google.com 3. Enter a search string and press enter 4. Warning dialoge displayed Either 5a) Press Enter or Continue or 5b) Press Cancel Actual Results: if 5a then form should be submitted, if 5b nothing should happen (form reset?). However, nothing happens, the form is not submitted (or reset) and the search value remains. Expected Results: Submitted the form (in case of 5a) and returned the search results. Reset the form or do nothing (? not 100% sure which) in the case of 5b. Here is an example of a page which will not submit if the insecure submission warning is turned on. <html> <head> <title> Test Page </title> </head> <body> <h1> Test Page </h1> <form action=test1.html type=post> <input type=text name="test"> <input type=submit> </form> </body> </html>
I dont see this on windows and linux...
I get something similar using mozilla 0.94 on NetBSD, but not on my win98 machine. The first time a form is submitted after starting mozilla (not just a new browser window), nothing happens once the insecure submission dialog has gone after clicking the continue button. The difference for me is that submitting any form afterwards seems to work properly. The problem only occurs the first time submitting after the browser is run. The problem never occurs with the insecure submission dialog disabled. Going to the preferences and turning the warning on doesn't cause it to happen again for me.
I still get this same behaviour with 0.9.5 (on IRIX). Just to clarify, it ONLY occurs when the insecure submission dialog is shown.
The above patch will fix this problem. It is a simple variable initialisation problem.
Could someone please review this? Thanks
*** This bug has been confirmed by popular vote. ***
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: patch
reassigning
Assignee: rods → alexsavulov
who wants to take over this bug? i don't have a IRIX machine vladimire: can you ind a dev that may be taking over this IRIX bug? is working for me on W2000 / LINUX
Can someone please have a look at this. It is a single line patch, and is a quite annoying bug for new users. Thanks
Comment on attachment 54033 [details] [diff] [review] security/manager/ssl/src/nsSecureBrowserUIImpl.cpp r= alexsavulov initializing the value is cannot do harm, but please get a sr= from a super reviewer (i already sent a mail to reviewers@mozilla.org and cc'ed nickb@adacel.com.au) NICKB: (OPTIONAL) could you please find out what the "(or doesn't care)" part of the comment on the top of te method means? maybe you can rewrite that a little bit :-)
Attachment #54033 - Flags: review+
At an initial look, it seems as if they are refering to whether the user has selected to be prompted (warned) about submiting forms to insecure pages. I will confirm this, and create a patch with a better comment. Thanks :)
Improves the comments about what ConfirmPostToInsecure and ConfirmPostToInsecureFromSecure do
i still don't know about a dev that can do sr= for security stuff... waiting...
What's the mystery? Any super-reviewer can review any code change subject to super-review. Pick one and mail him, cc'ing reviewers@mozilla.org. I'll do this one right now. /be
The important point, and http://www.mozilla.org/hacking/reviewers.html makes this several ways, including by listing me as a default/catch-all reviewer, is to mail *one* person, cc'ing reviewers@mozilla.org, asking for super-review. Mailing to a bulk list is not going to get you any individual accountability. /be
Comment on attachment 54033 [details] [diff] [review] security/manager/ssl/src/nsSecureBrowserUIImpl.cpp The bug is not here, it's in the implementation of ConfirmDialog, called by ConfirmPostToInsecure, in nsNSSDialogs. Patch for that coming up. Don't spackle over root causes of problems like this; XPCOM says out params must be valid upon NS_OK return, so there should be no need to initialize result in the caller. /be
Attachment #54033 - Flags: needs-work+
This is a much better solution, thanks! :)
is it checked in already?
No, I don't think so.
Comment on attachment 58590 [details] [diff] [review] proposed fix r= alexsavulov shall I ask or a sr= and check it in guys?
Attachment #58590 - Flags: review+
Please do. Do you want to check the comments patch too? May aswell improve them while we are here too. Thanks
nick: have you tested the /be patch already? for the correctness of the procedure i will try to get another sr= (although brendan is sr himself) and then check the patch in this week.
Yes the patch is tested (and works perfectly!) :)
Comment on attachment 58590 [details] [diff] [review] proposed fix sr=attinasi
Attachment #58590 - Flags: superreview+
i will check the changed comments with the proposed patch since they do not affect the build/application.
fix checked into the trunk
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
oops... i'm not sure I checked in correctly
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
the checkin is on the trunk, but the checkin doesn't show up on the tinderboxes
I can pull the change via cvs, so it looks good.
Alexandru, is this ok to be closed now? as far as I can tell it all looks good. Thanks.
i'd say yes. if is on the trunk then is fixed. marking as fixed
Status: REOPENED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → FIXED
Verifying by reporter's coments (dont have an IRIX machine to test, if someone still sees this please reopen)
Status: RESOLVED → VERIFIED
Component: HTML: Form Submission → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: