110613 - File upload controls broken

Status: VERIFIED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

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

Comment 1

[Matthias Versen [:Matti]]

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

Comment 2

[John Keiser (jkeiser)]

I have an idea that may fix it.  If it works, patch and explanation are forthcoming.

Comment 3

[John Keiser (jkeiser)]

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.

Comment 4

[John Keiser (jkeiser)]

Attachment: Aforementioned (Doesn't Fix Problem) Patch

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

why doesn't this fix the problem?

Comment 6

[John Keiser (jkeiser)]

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.

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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?

Comment 8

[John Keiser (jkeiser)]

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.

Comment 9

[John Keiser (jkeiser)]

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.

Comment 10

[John Keiser (jkeiser)]

Attachment: 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.

Comment 11

[John Keiser (jkeiser)]

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 12

[Peter Van der Beken [:peterv]]

Comment on attachment 58261 [details] [diff] [review]
Patch That Fixes Problem

r=peterv

Comment 13

[Johnny Stenback (:jst)]

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.

Comment 14

[Alex Vincent [:WeirdAl]]

Not that it matters...
Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.5+) Gecko/20011116

Comment 15

[Ashley Bischoff (blog at handcoding.com)]

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

Comment 16

[Ashley Bischoff (blog at handcoding.com)]

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

Comment 17

[Dave Miller [:justdave]]

removing platform = PC.  duplicated on Mac OS X 2001111609

Comment 18

[Dave Miller [:justdave]]

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

Comment 19

[John Keiser (jkeiser)]

Fix was checked in, works for me now.

Comment 20

[Marc Boullet]

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. 

Comment 21

[Ashley Bischoff (blog at handcoding.com)]

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

Comment 22

[Marc Boullet]

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

Comment 23

[Thomas Swan]

Same problem happening on Win2K (Win32 Nightly Buid 2001120703)

Comment 24

[Darin Fisher]

probably not related to my patch for bug 15320 since that was only checked in
yesterday.

Comment 25

[Boris Zbarsky [:bzbarsky]]

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

Comment 26

[Boris Zbarsky [:bzbarsky]]

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

Comment 27

[Boris Zbarsky [:bzbarsky]]

Is this still a problem in current builds?

Comment 28

[Marc Boullet]

from Boris comment #27

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

Comment 29

[Boris Zbarsky [:bzbarsky]]

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

[Matthew Miller]

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

[Jonathan Baron]

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: [e-mail]
Of course, I'm also not sure that this is the right bug, but it looks
at least related.

Comment 32

[Marc Boullet]

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

[Marc Boullet]

Attachment: log file

Comment 34

[Marc Boullet]

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

[Dan Allen]

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...

Comment 36

[Boris Zbarsky [:bzbarsky]]

That is bug 116210.  See 0.9.7 release notes.

Comment 37

[John Keiser (jkeiser)]

Yes.  This is fixed.  Works for me now (Linux 2002011706).  Setting back to FIXED.

Comment 38

[Vladimir Ermakov]

verifying build 2002-02-22-03-trunk windows 98 and 2002-02-25-09-trunk linux redhat