File upload controls broken

VERIFIED FIXED

Status

()

--
major
VERIFIED FIXED
17 years ago
17 years ago

People

(Reporter: bzbarsky, Assigned: john)

Tracking

({regression})

Trunk
regression
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

17 years ago
BUILD: 2001-11-15-21 and thereafter
NOT PRESENT IN: 2001-11-15-06

STEPS TO REPRODUCE:

1)  Load http://www.zbarsky.org:8000/~bzbarsky/test.html or
    http://bugzilla.mozilla.org/show_bug.cgi?id=98702
2)  Attempt to upload a file (add an attachment)

EXPECTED RESULTS:

Uploads the file (the cgi for the www.zbarsky.org page will dump out all the
POST data) or attaches an attachment

ACTUAL RESULTS:

No file uploaded.  Bugzilla complains that no file was selected.

ccing darin on the off chance that his url parsing rewrite is relevant
(Reporter)

Updated

17 years ago
Severity: normal → major
Keywords: regression
*** Bug 110615 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 2

17 years ago
I have an idea that may fix it.  If it works, patch and explanation are forthcoming.
(Assignee)

Comment 3

17 years ago
Well, I'll attach this patch, which really *should* be a problem in general even
if it's not the problem we're seeing now, but something even stranger is going
on.  SetValueGuaranteed() is being called by text controls but not file
controls.  I thought file controls used text frames as their primaries?

The problem I was guessing at was, we recently changed text frame to call normal
SetValue() on the content when they are destroyed.  SetValue(), unfortunately,
does security checks for input type=file and will not normally allow you to set
the value.  The soon-to-be-attached patch (should) fix that.

I will be back in a few hours, I will look into this more than.
Status: NEW → ASSIGNED
(Assignee)

Comment 4

17 years ago
Created attachment 58239 [details] [diff] [review]
Aforementioned (Doesn't Fix Problem) Patch
why doesn't this fix the problem?
(Assignee)

Comment 6

17 years ago
I am not sure yet.  I just noticed my printf() in the patch doesn't have a \n at
the end, so maybe it was showing up and I wasn't noticing it ...

We should consider that this may not be the problem at all--specifically, the
frame Destroy tends to happen lazily and probably would not have happened before
the submit.  This *is* a problem with input type=file though, and should be part
of this patch.

I'm back and investigating.
It's perfectly acceptable behaviour for type=file to reject the ::SetValue 
call, so we have to call some other setter (which used to be SetValueInternal).

It shouldn't matter that the frames are destroyed lazily since as long as the 
frame is around we'll get the value from it rather then the content. 
Unless ::GetPrimaryFrame stops returning the frame before the frame actually 
goes away?
(Assignee)

Comment 8

17 years ago
OK, more info ... it appears that the primary frame for files is not in fact the
text frame, but is instead the little file button.  This means that the content
now believes that the frame does not own the value.  Three solutions exist:

- change the primary frame to be the text frame (unknown ramifications but
definitely cleanest-looking solution)
- implement nsIGfxTextControlFrame in the file button (and delegate all calls to
the text frame) (unknown ramifications again)
- change the content to assume the frame owns the value *unless* it is a text
frame exists.  (This is quick, but it seems bad since the file control frame
uses a text control frame internally, and that frame may relinquish the value.)

Other suggestions welcome.  I'm going to try #3 and see if it works.  I think it
will.

sicking, if the frame is destroyed lazily that's fine, I agree ... that's just
an explanation for why the initial fix didn't work.
(Assignee)

Comment 9

17 years ago
Reading that I don't think I was very clear: the solution I am trying out is:
1. If the primary frame does not exist, content owns the value.
2. If the primary frame is a text frame, OwnsValue() reports whether it owns the
value.
3. If the primary frame is *not* a text frame, the frame owns the value.

This is essentially how we had it implemented before the recent change.
(Assignee)

Comment 10

17 years ago
Created attachment 58261 [details] [diff] [review]
Patch That Fixes Problem

This patch does #3 above (mimick previous behavior; if it's any kind of form
control frame at all, assume it owns the value--unless it's a text control, in
which case *ask* if it owns the value).

Better fix probably needed; discussion definitely needed.
Attachment #58239 - Attachment is obsolete: true
(Assignee)

Comment 11

17 years ago
BTW, I posted that patch with the patched build.  I haven't noticed any other
problems and I can't think of any new ones that would be caused by this, so I'll
check this in if we get r=/sr=.
Comment on attachment 58261 [details] [diff] [review]
Patch That Fixes Problem

r=peterv
Attachment #58261 - Flags: review+
Comment on attachment 58261 [details] [diff] [review]
Patch That Fixes Problem

sr=jst

I'll check this in to avoid having a blocker bug because of this tomorrow.
Attachment #58261 - Flags: superreview+
Not that it matters...
Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.5+) Gecko/20011116
OS: Linux → All
*** Bug 110774 has been marked as a duplicate of this bug. ***
*** Bug 110733 has been marked as a duplicate of this bug. ***
removing platform = PC.  duplicated on Mac OS X 2001111609
Hardware: PC → All
*** Bug 110259 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 19

17 years ago
Fix was checked in, works for me now.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 20

17 years ago
Reopening since I've seen the same behaviour in 2001120208 NT4

I tried to add an attachment to bug 112398 and I got twice "file selection
missing" or something like that.
The third time I submitted, it worked. 
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
*** Bug 113255 has been marked as a duplicate of this bug. ***

Comment 22

17 years ago
See also bug #113297 comment #1.
this reporter uses 20011203

Comment 23

17 years ago
Same problem happening on Win2K (Win32 Nightly Buid 2001120703)

Comment 24

17 years ago
probably not related to my patch for bug 15320 since that was only checked in
yesterday.
(Reporter)

Comment 25

17 years ago
*** Bug 116056 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 26

17 years ago
*** Bug 116272 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 27

17 years ago
Is this still a problem in current builds?

Comment 28

17 years ago
from Boris comment #27

yes. yesterday (20/12) same problem adding a testcase
with 2001121703 NT4
(Reporter)

Comment 29

17 years ago
Marc, could set NSPR_LOG_MODULES to nsHttp:5, then run Mozilla and capture the 
output from trying to attach a small file so we can see what it is that Mozilla 
actually sends to the server?

Comment 30

17 years ago
Is bug #116210 related to this? In some ways, it seems to be the opposite
problem -- was it perhaps caused by something attempting to fix this one?

Comment 31

17 years ago
I tried to upload a file to NSF Fastlane with release 0.9.7 (Linux).
I could do it with 0.9.6 (but Fastlane had other problems, now solved.)
But it simply does not upload now.  There is apparently an error message,
but I can't read it.  The window is mostly blank.  Fastlane is
http://www.fastlane.nsf.gov  It is what you MUST use now to submit
grant proposals to the National Science Foundation.  (They say not to
use "Netscape 6" and they don't mention Mozilla.)
If someone wants to try this and doesn't want to create a new password,
write me and I'll give you mine: baron@psych.upenn.edu
Of course, I'm also not sure that this is the right bug, but it looks
at least related.

Comment 32

17 years ago
I set the environment variables as per Boris comment #29.
I tried to download a small HTML file.

Here are the steps:
1. Clicked "Create a New Attachment" in this bug
2. Was asked for a login, password that Mozilla autofilled. Clicked login
3. Filled the file upload form with a file name (I browsed to get rid of
syntax errors) and selected the file type "text/html".
4. Clicked submit
5. Was asked again for login password. This page doesn't succeed to
load completly (the tab was still spinning). I had to stop it.
6. Back to the file upload submission page
7. reclicked submit
8. Was asked again for login password. this time the page loaded
successfully, the pw/login was autofilled and i clicked login
9. Then I got the red message: "You didn't specify a file to attach".
10. Retried two more times: same message.
11. At this time, I copied the log file to another file which
I will now try to attach to the bug.

For info, I'm behind a SQUID proxy with authentication. I noticed also
that the login/password authentication to Mozilla had not a constant behavior
(some times you're asked twice, some times once).

Comment 33

17 years ago
Created attachment 62750 [details]
log file

Comment 34

17 years ago
I had to use NS4.7 to post the file and
succeeded only the second time (is it a mozilla specific problem ?).

Notice that I didn't empty the cache before running the test and
that I changed, in the log file, server/file names with xxxxxx, yyyyyyy.

Comment 35

17 years ago
Now there is a new problem that I am wondering if anyone else notices...If you
try to submit a form with a file upload form field and it is empty (like you
don't to put in a file) mozilla just hangs and hangs and hangs...and let me tell
you lots of forms out there make this field optional!?!?  yahoo for example...
(Reporter)

Comment 36

17 years ago
That is bug 116210.  See 0.9.7 release notes.
(Assignee)

Comment 37

17 years ago
Yes.  This is fixed.  Works for me now (Linux 2002011706).  Setting back to FIXED.
Status: REOPENED → RESOLVED
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED

Comment 38

17 years ago
verifying build 2002-02-22-03-trunk windows 98 and 2002-02-25-09-trunk linux redhat
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.