76932 - Acrobat Forms fails to submit on Mac

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect, P1)

Description

[shrirang khanzode]

filing as a seperate problem for mac only from bug 53347.


steps:

Go to this URL and type any thing you want. Press submit.
Observe that nothing happens 

Expected:
You will know everything is working if you see something like
this populate the large grey box on the left:

%FDF-1.2   1 0 obj <<  /FDF << /Fields [ << /V (You may type something in here 
if you wish)/T (Gibberish)>>  << /T (Submit)>> ]  /F 
(http://access.adobe.com/browser/pingform.pdf)/ID [ 
<160bbb62aaa6ebe41da6613b185c3724><843579f805b9c66041e13a566488878f> ] >>  >>  
endobj trailer << /Root 1 0 R   >> %%EOF 

(This is FDF -- a second file format associated with PDF and Acrobat forms.)

Comment 1

[Peter Lubczynski]

Mac only.

Comment 2

[av (gone)]

I must have done something not pure xp when doing files but cannot immediately 
see what. Peter, if you put a break point in 
nsPluginHostImpl::NewPluginURLStream on if(aPostData) you should be able to 
quickly figure it out.

Comment 3

[Peter Lubczynski]

We are passing "Mac HD/temp00000" as filename for NS_NewPostDataStream which is 
probably wrong. Mac paths are seperated by colons (::). I'll set a breakpoint 
where NS_NewPostDataStream is used elsewhere and cc:ing Pollmann.

Comment 4

[av (gone)]

Peter, as far as I understand this path we should pass to NS_NewPostDataStream 
is supposed to come from the plugin itself, so it is supposed to be just right 
on every platform. Could you somehow check what exactly we get from the plugin 
(I beleive the breakpoint should be set in _posturl (ns4xPlugin.cpp))

Comment 5

[Peter Lubczynski]

NS_NewPostDataStream fails if ":" aren't used as path seperators on the Mac. I'm 
creating a testing a patch now to replace "/" with ":"

Comment 6

[Eric Pollmann]

Is "/" a valid character in a filename on Mac?  If so, might replacing "/" with
":" be dangerous?

If you do make this change, please try to make it as specific to plugins and Mac
as possible - NS_NewPostDataStream is used by layout too, and we don't have
problems with Mac path delimiters there.

Comment 7

[Peter Lubczynski]

Attachment: patch to replace '/' with ':' on Mac filenames

Comment 8

[Peter Lubczynski]

This patch is a Mac plugins only solution. I don't see another way to fix this 
without doing the replace. Comments?

Comment 9

[Peter Lubczynski]

Andrei,

In _posturl, buf (the filename) comes in as an url: file://Mac HD/filename

Before you call NS_NewPostDataStream, you strip off the file:// which makes it 
look like UNIX path. Guess this works in Windows, but Mac likes ':' folder 
seperators.

Comment 10

[av (gone)]

Oh, I see, it is not a file path but rather a url. (Man when you are on 
vacation you forget things so quickly.) Shouldn't we have some general way to 
convert file type urls into the native path? Does nsIFile provide something like 
this? I think that would be a right way to do this. Otherwise as Eric said we 
should be very careful. I wonder, why it works fine on Windows then?

What if to create a nsILocalFile object using this url and then just do GetPath 
off it (or what ever it has)? Maybe it will do the right thing for all 
platforms? You can steal the code from the place where I delete this file.

Comment 11

[Peter Lubczynski]

            nsCOMPtr<nsILocalFile> file;
            rv = NS_NewLocalFile(aPostData, PR_TRUE, 
getter_AddRefs(file));
            if (NS_FAILED(rv)) return rv;
            file->Normalize();  // doesn't help
            file->GetPath(&filename);

Unless I'm missing something, filename comes out the same way 
aPostData came in: file:///Mac HD/filename

I'll see how layout handles it.

Comment 12

[Eric Pollmann]

> I'll see how layout handles it.

Layout's input is a platform-native path, not a url, so it doesn't really have
to do any conversion - we take the value of the file input control (e.g.
Mac HD:filename), and  just directly use that.

Sorry, I don't know of anything generic that does this conversion, but there
might be something in the file: protocol handler in netwerk/protocol/file?

Comment 13

[Eric Pollmann]

Don't know if this is what you are looking for, but in nsFileChannel::Init() a
URL (mURI) looks to be converted into a localfile (localFile):

    // if we support the nsIURL interface then use it to get just
    // the file path with no other garbage!
    nsCOMPtr<nsIFileURL> fileURL = do_QueryInterface(mURI, &rv);
    if (NS_FAILED(rv)) {
        // this URL doesn't denote a file
        return NS_ERROR_MALFORMED_URI;
    }

    rv = fileURL->GetFile(getter_AddRefs(mFile));
    if (NS_FAILED(rv)) return rv;

    nsCOMPtr<nsILocalFile> localFile = do_QueryInterface(mFile);
    if (localFile)
        localFile->SetFollowLinks(PR_TRUE);

Comment 14

[Eric Pollmann]

Attachment: patch, convert url to native path using above logic

Comment 15

[Eric Pollmann]

The above patch worked okay on Windows with the test URL, does it also do the
trick on Mac?

Comment 16

[Peter Lubczynski]

I was just about to attach a patch similar to yours.

I tested yours on Mac, it's good, it works. I'll see if I can get reviews to get 
it in for mozilla 0.9.

Comment 17

[av (gone)]

Peter, do you think that now once we get this nsILocalFile object at the very 
beginning it would make sense to cache _it_ rather that the native path, so when 
the time to delete the file comes we don't need to convert the path into the 
nsILocalFile again. Unless this type of object is expensive in terms of 
footprint, I don't know.

Comment 18

[av (gone)]

It is good enough for me in its present form (and more likely to be accepted 
being smaller).

ra=av

Comment 19

[Marc Attinasi]

sr=attinasi

Comment 20

[Christopher Blizzard (:blizzard)]

a=blizzard for 0.9

Comment 21

[Peter Lubczynski]

Patch checked into the BRANCH:

/cvsroot/mozilla/modules/plugin/nglsrc/nsPluginHostImpl.cpp,v  <--  
nsPluginHostImpl.cpp
new revision: 1.233.4.1; previous revision: 1.233

Comment 22

[Peter Lubczynski]

Patch checked into the TRUNK. Marking FIXED.

/cvsroot/mozilla/modules/plugin/nglsrc/nsPluginHostImpl.cpp,v  <--  
nsPluginHostImpl.cpp
new revision: 1.238; previous revision: 1.237

Comment 23

[shrirang khanzode]

verification blocked due to bug 78741

Comment 24

[shrirang khanzode]

crashing for me after I press the SUBMIT button on mac trunk 0516. Reopening...

Comment 25

[Peter Lubczynski]

Can you please attach a stack trace? Thanks!

Comment 26

[shrirang khanzode]

Looking at the trace, it seems like the same one as in 64645 that suresh posted.
Will recheck this today with Peter's fix for SetUpStreamListener

stack:

 Call Stack:    (Signature = free() 794e41bf) 
     
   free() 
                                                     [nsAllocatorManager.cp]
     
   PL_strfree() 
                                                     [strdup.c, line 44]
     
   nsPluginStreamListenerPeer::SetUpStreamListener() 
                                                     [nsPluginHostImpl.cpp, line 
1622]
     
   nsPluginStreamListenerPeer::OnStartRequest() 
                                                     [nsPluginHostImpl.cpp, line 
1413]
     
   nsStreamListenerTee::OnStartRequest() 
                                                     [nsStreamListenerTee.cpp, 
line 13]
     
   nsHttpChannel::ProcessNormal() 
                                                     [nsHttpChannel.cpp, line 
431]
     
   nsHttpChannel::ProcessResponse() 
                                                     [nsHttpChannel.cpp, line 
363]
     
   nsHttpChannel::OnStartRequest() 
                                                     [nsHttpChannel.cpp, line 
1974]
     
   nsOnStartRequestEvent::HandleEvent() 
                                                     
[nsRequestObserverProxy.cpp, line 108]
     
   nsARequestObserverEvent::HandlePLEvent() 
                                                     
[nsRequestObserverProxy.cpp, line 63]
     
   XPCOM_DLL + 0x6ecd8 (0x16ab38d8) 
                                                      
     
   XPCOM_DLL + 0x6eb54 (0x16ab3754) 
                                                      
     
   XPCOM_DLL + 0x1c0a0 (0x16a60ca0) 
                                                      
     
   WIDGET_DLL + 0x5a40 (0x16dd43e0) 
                                                      
     
   WIDGET_DLL + 0x5804 (0x16dd41a4) 
                                                      
     
   WIDGET_DLL + 0x2d9b0 (0x16dfc350) 
                                                      
     
   WIDGET_DLL + 0x19ce0 (0x16de8680) 
                                                      
     
   WIDGET_DLL + 0x195e0 (0x16de7f80) 
                                                      
     
   WIDGET_DLL + 0x18bf8 (0x16de7598) 
                                                      
     
   nsAppShellService::Run() 
                                                     [nsAppShellService.cpp, 
line 416]
     
   Netscape 6 + 0x469c (0x174bd1bc) 
                                                      

Comment 27

[Peter Lubczynski]

Marking FIXED (again). I'm pretty sure that my patch fixes the crash in that 
stack. Please re-open if not, but verify it's the same stack or a different one.

Comment 28

[shrirang khanzode]

yeah, I see the exact same results on mac as that on windows now. The submitted 
data result appears inside the textarea now. Verifed on mac 2001051704 build.