201275 - [Webclient] move Prompt code from CBrowserContainer to PromptService

Status: RESOLVED WONTFIX

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

Description

[Kyle Yuan]

Comment 1

[Kyle Yuan]

WebClient should support its own prompt boxes by overriding the system default 
nsIPromptService instead of inheriting nsIPrompt in CBrowserContainer, because 
nsIPromptService is per app object but CBrowserContainer is per window.
See:
http://www.mozilla.org/projects/embedding/windowAPIs.html#advancedAPI
http://www.mozilla.org/projects/embedding/faq.html#3

Comment 2

[Kyle Yuan]

changing summary, Prompt is not a kind of event...

Comment 3

[Kyle Yuan]

Attachment: patch

The changes I've made:
1) made CBrowserContainer not inherit from nsIPrompt and nsIAuthPrompt, moved
those code into class PromptService which is registered as a service at
startup;
2) implemented Alert, AlertCheck, Confirm, ConfirmCheck and Prompt methods in
PromptService;
3) rewrite the UniversalDialog code, add a new class (UniversalDialogData) to
hold the all arguments to UniversalDialog;
4) removed some improper initialization code in
wsRealizeBrowserEvent::handleEvent().

Comment 4

[edburns]

r=edburns

Looks Great!

Comment 5

[edburns]

You provide a file by file by file breakdown of changes.  This is exactly what I
need to provide you prompt reviews.  Please follow this style for future review
requests.

Thanks,

Ed

Comment 6

[Kyle Yuan]

Ed, you mean I should split the patch by file and zip them together instead of
putting them all in a single text file?

Comment 7

[Kyle Yuan]

fix checked in!

Comment 8

[edburns]

I had to roll-back these patches since it turns out they break events from the
browser to the client.  Kyle, can you please rework the patch and make sure the
events still work?

Comment 9

[Kyle Yuan]

yes, I'm working on this right now.

Comment 10

[Kyle Yuan]

Attachment: add CBrowserContainer to gecko as a WebBrowserListener

also fix a crash bug in ConfirmEx.

Comment 11

[Kyle Yuan]

Comment on attachment 123257 [details] [diff] [review]
add CBrowserContainer to gecko as a WebBrowserListener

Ed, may I check this small fix in?

Comment 12

[edburns]

Comment on attachment 123257 [details] [diff] [review]
add CBrowserContainer to gecko as a WebBrowserListener

In the future, please explain what the patch does.

Not to be mean, but did you compile this patch?  After applying this
patch it doesn't compile as is.  I get this error:

     [exec] NativeEventThreadActionEvents.cpp: In method `void 
     [exec] *wsRealizeBrowserEvent::handleEvent ()':
     [exec] NativeEventThreadActionEvents.cpp:125: `browserContainer'
undeclared 
     [exec] (first use this function)
     [exec] NativeEventThreadActionEvents.cpp:125: (Each undeclared identifier
is 
     [exec] reported only once for each function it appears in.)
     [exec] make[1]: Leaving directory
`/home/edburns/Projects/mozilla/MOZILLA_1_3/mozilla/java/webclient/src_moz'
     [exec] make[1]: *** [NativeEventThreadActionEvents.o] Error 1

I had to change your patch to NativeEventThreadActionEvents.cpp to be:

Index: NativeEventThreadActionEvents.cpp
===================================================================
RCS file:
/cvsroot/mozilla/java/webclient/src_moz/NativeEventThreadActionEvents.cpp,v
retrieving revision 1.7
diff -u -r1.7 NativeEventThreadActionEvents.cpp
--- NativeEventThreadActionEvents.cpp	13 May 2003 15:22:00 -0000	1.7
+++ NativeEventThreadActionEvents.cpp	14 May 2003 15:43:30 -0000
@@ -102,8 +102,9 @@
     }

     // create our BrowserContainer, which implements many many things.
+    CBrowserContainer *browserContainer;

-    mInitContext->browserContainer = 
+    mInitContext->browserContainer = browserContainer = 
	 new CBrowserContainer(mInitContext->webBrowser, mInitContext->env, 
			       mInitContext);

@@ -121,6 +122,10 @@

     // set the docloaderobserver PENDING(edburns): how to we make our
     // presence as a nsIWebProgressListener know?n
+    nsWeakPtr weakling(
+		       
dont_AddRef(NS_GetWeakReference(NS_STATIC_CAST(nsIWebProgressListener*,
browserContainer))));
+    webBrowser->AddWebBrowserListener(weakling,
NS_GET_IID(nsIWebProgressListener));
+    

	printf("Creation Done.....\n");
     // Get the WebNavigation Object from the DocShell
===================================================================

to get it to compile.  After doing so, it runs fine, however.  To save
time, I have checked it in.  Please verify that it meets your
requirements.

I didn't check in the fix to PromptService since it has been CVS
removed.  You can still access it using cvs update -r.

New window still doesn't work for me.  Are you working on that?

Finally, in general, always use

cvs update -d -P

when updating your local copy of the tree.  This will cause files that
have been cvs removed to be removed from your workarea.  If you had done
this, you probably wouldn't have asked for review of a removed file.

Comment 13

[Kyle Yuan]

Ed, the new patch is a supplement of my previous patch, if you don't need the 
previous patch, you don't need this one either.

Sorry for not using the most recent source to do the patch...

BTW, I always compile & test my patchs before I provided them. New window works 
fine for me.

Comment 14

[Kyle Yuan]

Attachment: new patch (combined previous 2 patches, tar.gz format)

Comment 15

[Kyle Yuan]

mass give up webclient bugs

Comment 16

[Sylvestre Ledru [:Sylvestre]]

Java APIs to WebShell isn't a thing anymore. Closing.