76405 - Make it so Webclient works with the Mozilla trunk

Status: RESOLVED FIXED

(Core Graveyard :: Java APIs to WebShell, defect)

Description

[Ashu Kulkarni]

All Blackwood projects need to work with a Date-stamped snapshot of the Mozilla
Trunk, post 0.8.1. 

Currently, Webclient works with 0.8.1 on Win32 (Bug 74151). We need to get it to
work with the Trunk on Win32, Solaris and Linux.

Comment 1

[Ashu Kulkarni]

Attachment: cvs diff -u of patch - Works on Solaris

Comment 2

[Ashu Kulkarni]

Ed,
I have tested this patch on Solaris but not on Win32 or on linux. Everything
works fine except for Window->Close which causes a core dump due to some Null
ptr deref. Haven't investigated this yet. BTW, on Solaris, the call to
gdk_event_handler_set (handle_gdk_event, ..) is not necessary. I am able to
navigate and follow links fine without this. 
_Ashu

Comment 3

[edburns]

Depends on 75607.  This doesn't work on linux.

Comment 4

[edburns]

Ok, it's working on linux, but I had to modify the makefile:

Add -lxpcom to link line.

Remove -Ldist/bin/components -lwidget_gtk

.

Let me test these changes on Solaris and then I'll give review and approval.

I'm using webclient on linux to write this.

Comment 5

[Ashu Kulkarni]

Pls see the Makefile.in I have posted in the patch. It already has the -lxpcom 
in the link libraries line. I have also removed the lwidget_gtk. Did you need 
to change the Makefile apart from that?

Comment 6

[edburns]

Sorry for the reversal Ashu, but I can't approve this patch as it is.  The
sticking point is the change to Prompt.java.  Can you make it so the signatures
of Prompt.java don't change syntatically?  What we need to do is put pressure on
the embedding team to give us access to the realm string again.

I'll still try your patches on Solaris.

Comment 7

[edburns]

I'll accept the change if you introduce a Prompt2.java that contains the
modified promptUsernameAndPassword method().

Comment 8

[Ashu Kulkarni]

Here's what I suggest -
Create a new interface Prompt2.java with a corresponding 
Prompt2ActionEvents.cpp which implementes the new nsIPrompt interface. I will 
modify the EMWindow. java example to implement this interface from the Java 
side. This will keep us in sync with the Mozilla trunk (which we have decided 
we need).

In the meanwhile, we can push the Embedding@Mozilla folks to provide the realm 
string again in the nsIPrompt interface. 

If you disagree, let me know and we can discuss it.

Comment 9

[edburns]

Hi Ashu,

1. Please modify Makefile.in to remove the -Ldist/bin/components

2. Regarding Prompt.java

I think this change can be accomodated entirely in Java.  Let's stick
with your new modifications to the native side, for the most part.
Let's define Prompt2.java like this:

// code start

package org.mozilla.webclient;

import java.util.Properties;


/**

 * Extension to prompt. 

 */

public interface Prompt2 extends Prompt
{

public static final String CHK_VALUE_KEY = "chkValue";

/**

 * Puts up a username/password dialog with OK and Cancel buttons.

 * @param fillThis a pre-allocated properties object
 * that the callee fills in.

 * keys: userName, password
 
 * @return true for OK, false for Cancel

 */

public boolean promptUsernameAndPassword(String dialogTitle,
					 String text,
					 String chkMessage,
					 boolean chkValue,
					 Properties fillThis);
    
} // end of interface Prompt2

// code end

Then, in wsPromptUsernameAndPasswordEvent::handleEvent(), we check
whether the instance we have is a Prompt or a Prompt2.  If it's a
Prompt, we call the old promptUserNameAndPassword method, dummying up
strings as appropriate.  If it's a Prompt2 we just call the new method.

How does that sound?

Comment 10

[Ashu Kulkarni]

Agreed!

Comment 11

[edburns]

This works on Solaris "out of the box".  I'll wait for another rev of the 
patch.  

Comment 12

[Ashu Kulkarni]

Attachment: Second Revision of Patch - with Prompt2.java (tar.gz)

Comment 13

[Ashu Kulkarni]

This Second Revision contains a Prompt2.java which extends Prompt.java. The
application designer can choose which Interface he wishes to embed. The code for
recognizing the Implementing interface is in PromptActionEvents.cpp on the
Native side. CBrowserContainer is not made aware of the distinction between the
2 Java side interfaces.

Works on Solaris. Pls test on Win32 and linux.

Comment 14

[edburns]

Hi Ashu,

1. ALWAYS enclose if statements with curly braces.

Bad:

    if (env->IsInstanceOf(mPromptGlobalRef, clz))
        isPrompt2 = JNI_TRUE;  // it is a Prompt2
    else
        isPrompt2 = JNI_FALSE; // it is a Prompt

Better:

    if (env->IsInstanceOf(mPromptGlobalRef, clz)) {
        isPrompt2 = JNI_TRUE;  // it is a Prompt2
    } 
    else {
        isPrompt2 = JNI_FALSE; // it is a Prompt
    }  

2. You need to de-allocate the realmString.

Unfortunately, it appears basic auth is broken due to another bug, bug
76781.  Ashu, you can check this in and we'll re-address it when bug
76781 gets addressed.  Don't close this bug, though, until then.

Comment 15

[edburns]

Ashu, it turns out you have to implement nsIAuthPrompt.  I've added 
nsIAuthPrompt to the set of interfaces implemented by CBrowserContainer and 
I've found that PromptUserNameAndPassword does get called, and it has a realm 
string.  I suggest that we forget about the Prompt2 thing and map the methods 
from nsIAuthPrompt, and nsIPrompt into our Prompt.java.

Can you do that?

Comment 16

[Ashu Kulkarni]

Will do - I'll check it in by next week.

Comment 17

[Ashu Kulkarni]

Attachment: third iteration of patch (with nsIAuthPrompt) - cvs diff -u

Comment 18

[Ashu Kulkarni]

Attachment: third iteration of patch (with nsIAuthPrompt) - cvs diff -u

Comment 19

[Ashu Kulkarni]

Attachment: third iteration of patch (with nsIAuthPrompt) - cvs diff -u

Comment 20

[Ashu Kulkarni]

Attachment: third iteration (with nsIAuthPrompt) - cvs diff -u

Comment 21

[Ashu Kulkarni]

Thanks to Bugzilla's and my browser's sluggish behavior, I ended up mistakenly
attaching this diff 4 times. Pls disregard 
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=31594 (posted 10:22)
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=31595 (posted 10:25)
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=31596 (posted 10:25)
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=31597 (posted 10:31)

Correct diff for the patch is the last attachment.

Comment 22

[Ashu Kulkarni]

Attachment: final correct patch (cvs diff -u)

Comment 23

[edburns]

This looks good, but I can't test it until the mozilla cvs server is up, unless 
you attach a tar.gz of the modified files.

Comment 24

[edburns]

Ashu, yet more API changes: <http://bugzilla.mozilla.org/show_bug.cgi?id=46859>
"nsIPrompt's UniversalDialog needs to go away."  I'll produce another diff.

Comment 25

[edburns]

Ashu, you know I don't like commented out lines, and certainly not without an
explanation:

===================================================================
RCS file: /cvsroot/mozilla/java/webclient/src_moz/CBrowserContainer.h,v
retrieving revision 1.6
diff -u -r1.6 CBrowserContainer.h
--- CBrowserContainer.h	2001/04/02 22:48:30	1.6
+++ CBrowserContainer.h	2001/04/20 17:17:04
@@ -36,12 +36,13 @@
 #include "nsIWebShell.h"  // We still have to implement nsIWebShellContainer
                           // in order to receveive some DocumentLoaderObserver
                           // events.  edburns
-#include "nsIStreamObserver.h"
+// #include "nsIStreamObserver.h"

Comment 26

[edburns]

Ashu, thanks for the doing the hard part.  I'm going to re-assign this to me and
check it in after I've tested and got it working on Win32.

Right now I'm seeing that Copy doesn't work, and using bookmarks to go to a node
causes the app to hang.

Comment 27

[edburns]

reassign

Comment 28

[edburns]

a

Comment 29

[edburns]

Attachment: cvs diff -u of fix for this bug edburns first iteration, works with 25 April trunk on Linux.

Comment 30

[Ashu Kulkarni]

> Ashu, yet more API changes: <http://bugzilla.mozilla.org/show_bug.cgi?
> id=46859> "nsIPrompt's UniversalDialog needs to go away."  I'll produce 
> another diff.

This is the reason we need to decide on a Date tag to freeze on. I suggest we 
decide a date quickly if we dont want to keep playing tag with the underlying 
APIs.

Comment 31

[edburns]

Agreed.  Kevin Lee tells me the date tag is imminent.

Comment 32

[geetha.vaidyanaathan]

Changing QA contact for Webclient forthis bugid
Had already requested a change of QA contact for Webclient
Not sure what one needs to do to request for QA contact chg for all
the existing bugs
Can Alexi thake this as an action item?

-Geetha

Comment 33

[geetha.vaidyanaathan]

oops.. did actually chg it in the QA contact field last time

Comment 34

[edburns]

Works on win32, Solaris, and linux with 042701 0100 PDT trunk.