97380 - No user interaction: prompt before starting plugin

Status: RESOLVED WONTFIX

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

Description

[edburns]

This bug covers a subset of the material in bug 1785, namely, the part about 
simply telling the user the plugin is starting for the given mimetype.

Comment 1

[edburns]

Reassign.

Comment 2

[edburns]

reassign

Comment 3

[edburns]

accept

Comment 4

[edburns]

0.9.4

Comment 5

[edburns]

modules/oji/src/nsJVMManager.cpp
modules/oji/src/nsJVMManager.h
layout/html/base/src/nsObjectFrame.cpp
modules/plugin/base/src/nsIPluginInstanceOwner.h
modules/plugin/base/src/nsPluginHostImpl.cpp
modules/plugin/base/src/nsPluginHostImpl.h
modules/plugin/base/src/nsPluginViewer.cpp

Here's what I did to fix this bug.

I pestered Rick Potts about the problem of
nsIWebBrowserChrome->SetStatus() not being synchronous.  He fixed that
under bug 97227.

Taking advantage of Rick's fix to 97227, I modified
nsIPluginInstanceOwner to have new method, ShowStatus(const PRUnichar
*aStatusMsg).  This is necessary to allow for localized messages to be
posted from the plugin, such as "Starting plugin for type
application/x-java-vm" or "Beginnen steckbar für Art
application/x-java-vm".

I modified all the implementations of nsIPluginInstanceOwner: 

  layout/html/base/src/nsObjectFrame.cpp
  modules/plugin/base/src/nsPluginViewer.cpp

I inserted a call to nsIPluginInstanceOwner->ShowStatus() at the part
just before the big "whole damn app freezes when starting java" thing
happens.

I also modifed nsJVMManager.cpp ShowJavaConsole() to do the same.  These
are the two places I know of now that can start java.

Comment 6

[edburns]

Attachment: diff -u of fix for this bug, first iteration

Comment 7

[Peter Lubczynski]

Ed, I'm getting a build error even after applying the patch from the other bug.
Where did you get nsIWebBrowserChrome::STATUS_FLAG_SYNC from? I don't see it in
Rick's patch either but maybe that should land first anyway.

Comment 8

[edburns]

That's in Rick's patch for 97227.  Rick's patch MUST land before mine.

Comment 9

[edburns]

It turns out that Rick's patch no longer requires the STATUS_SYNC, since all 
calls to Chrome::SetStatus() are now synchronous, after his patch.

For the purposes of review, please take out that argument.

Once Rick checkes in his fix to 97227, I'll generate a new patch.  In the 
meantime, can you give r=?

Comment 10

[Peter Lubczynski]

looks good to me, r=peterl, but be sure it all compiles and works with other
tree changes before checking in.

Comment 11

[edburns]

Attachment: cvs diff -u of fix for this bug, second iteration

Comment 12

[edburns]

Attachment: cvs diff -u of fix for this bug, third iteration

Comment 13

[edburns]

Attachment: cvs diff -u of fix for this bug, fourth iteration (hopefull final)

Comment 14

[Chris Waterson]

1. Why do we need to keep ShowStatus(char*) at all? Couldn't we just convert
everything to use the PRUnichar* interface? I won't hold up this checkin for
that, but seems like we might want to file a new bug to eliminate ShowStatus(char*).

2. You shouldn't need to cast msg.get() to PRUnichar* when calling
aOwner->ShowStatus() in nsPluginHostImpl::PostStartupMessageForType(). Remove that.

3. Do |return ShowStatus(NS_ConvertUTF8toUCS2(aStatusMsg).get())| instead of
AssignWithConvesion(), etc. in pluginInstanceOwner::ShowStatus(char*):

+  nsAutoString  msg; msg.AssignWithConversion(aStatusMsg);
+  rv = this->ShowStatus(msg.get());
+  
+  return rv;

Same for nsPluginInstanceOwner in nsObjectFrame.cpp.

4. Shouln't need cast here:

+                    chrome->SetStatus(nsIWebBrowserChrome::STATUS_SCRIPT, 
+                                      (PRUnichar *) msg.get());

5. Use |msg.Truncate()| instead of |AssignWithConversion()| here:

+        msg.AssignWithConversion("");
+        chrome->SetStatus(nsIWebBrowserChrome::STATUS_SCRIPT, 
+                          msg.get());


Fix that, file a bug to remove ShowStatus(char*) (or convince me we need it),
and sr=waterson.

Comment 15

[Brendan Eich [:brendan]]

Oh good, waterson picked up the sr= ball here.

/be

Comment 16

[edburns]

I have filed http://bugzilla.mozilla.org/show_bug.cgi?id=97848 for item 1. in 
Waterson's list.

Comment 17

[edburns]

Attachment: cvs diff -u of fix for this bug, 5th and final iteration

Comment 18

[Asa Dotzler [:asa]]

Comment on attachment 47878 [details] [diff] [review]
cvs diff -u of fix for this bug, 5th and final iteration

a=asa on behalf of drivers

Comment 19

[edburns]

How to test this fix:

Have a mozilla trunk build from on or before noon PDT Friday 31 August 2001

Apply the 5th iteration patch to your tree

Clobber and Build the tree.  Please note that you may have to specifically re-
link gklayout.  

Install JDK1.3.1.

Start mozilla.

Go Tasks->Tools->Java Console.

See that the string "Starting plugin for type application/x-java-vm" is seen on 
the status bar before the browser freezes for java startup.

Close the browser

Start the browser

Visit a page with an applet

See that the string "Starting plugin for type application/x-java-vm" is seen on 
the status bar before the browser freezes for java startup.

Comment 20

[edburns]

Fix checked in.

Comment 21

[shrirang khanzode]

I installed the 1.3.1 jre from sun. On win trunk 0905, I don't get the "starting 
plugin...." message at all when I launch the java console. The only thing I see 
on the status bar *while loading an applet* is "Loading xx Applet" and "Applet 
xx Started". Is this and rpott's fix in the trunk ? I checked it and it is 
there...hmm..can anyone confirm , thx.

Comment 22

[edburns]

Did you make sure to rebuild your chrome in xpfe?  The message will not appear 
unless it's found in the string bundle, and it won't be found in the string 
bundle unless chrome is completely rebuilt.  After a clobber all build on win32 
today, I see this message.

Comment 23

[shrirang khanzode]

i ain't using a debug build,the commercial optimized build. How do I see it 
working 

Comment 24

[edburns]

Shrirang,

Here's how you can tell if it should work or not.

In your dist, find the file US.jar.  For me, it ends up 
as ./WIN32_D.OBJ/bin/chrome/US.jar.  Pull out the region.properties file from 
the jar:

jar -xvf US.jar locale/US/global-region/region.properties

The resultant file, locale/US/global-region/region.properties, relative to 
where you were when you un-jar'd it, should have the line

pluginStartupMessage=Starting Plugin for type

If it doesn't you wont see the message.  If it does, please post.  Then we have 
a problem.

Comment 25

[shrirang khanzode]

the region.properties file does not have the above message. is this a packaging
issue or something?

Comment 26

[edburns]

Yes, it is.  Can you please post the region.properties file you have to the bug?

Ed

Comment 27

[edburns]

This is what's in the trunk and the branch:

http://lxr.mozilla.org/mozilla/source/xpfe/global/resources/locale/en-﷒0﷓

Comment 28

[shrirang khanzode]

my region.properties file :

#
#   Localizable URLs
#
noDefaultPluginMessage=Netscape cannot find the Plugin Downloader Plugin.  
Without the Plugin Downloader Plugin, you cannot automatically download and 
install plugins.  Please visit http://www.netscape.com/ to install the Plugin 
Downloader Plugin.
#
pluginStartupMessage=Starting Plugin for type
#
# brand.properties
#
releaseUrl=http://home.netscape.com/eng/mozilla/ns6/relnotes/pv6-3.html
getNewThemesURL=http://home.netscape.com/bookmark/6_1/themes.html
smartBrowsingURL=http://info.netscape.com/fwd/rlstatic/http://home.netscape.com/
bookmark/6_1/keyw.html

#
# fontdownload.properties
#
fontDownloadURL=http://www.mozilla.org/projects/intl/fonts/win/en/package_%1$s.h
tml

Comment 29

[shrirang khanzode]

aaah....pls IGNORE the earlier one....I manually had added the pluginstartup 
thing(to see it work, and left it without.....i am attaching the correct 
version..Sorry for the foul up.

Comment 30

[shrirang khanzode]

CORRECT region.properties:

#
#   Localizable URLs
#
noDefaultPluginMessage=Netscape cannot find the Plugin Downloader Plugin.  
Without the Plugin Downloader Plugin, you 
cannot automatically download and install plugins.  Please visit 
http://www.netscape.com/ to install the Plugin 
Downloader Plugin.
 
# brand.properties
#
releaseUrl=http://home.netscape.com/eng/mozilla/ns6/relnotes/pv6-3.html
getNewThemesURL=http://home.netscape.com/bookmark/6_1/themes.html
smartBrowsingURL=http://info.netscape.com/fwd/rlstatic/http://home.netscape.com/
bookmark/6_1/keyw.html

#
# fontdownload.properties
#
fontDownloadURL=http://www.mozilla.org/projects/intl/fonts/win/en/package_%1$s.h
tml

Comment 31

[edburns]

How can we fix this problem?

Comment 32

[edburns]

<mkaply> Ed: Commercial has a private version of region.properties that 
overwrites the mozilla one
<mkaply> you need someone to check your change into the NS one in commercial

This is the reason you don't see this bug as fixed. 

I'm working to get someone to check it in for me.

Comment 33

[edburns]

I just spoke with Syd Logan.  He has agreed to check in the fix on the Netscape 
commercial 0.9.4 branch and trunk.

Comment 34

[Syd Logan]

ns tree stuff checked in (changes to the region.properties file)

Comment 35

[shrirang khanzode]

yup, looks fixed on branch 0912. will verify on trunk builds next..

Comment 36

[shrirang khanzode]

verified on win trunk 0913. not working on linux ? is that natural ? Is this 
windows only ?

Comment 37

[David G King]

Reopening as I don't see this message with TRUNK build 2002120204 on WinXP SP1.

Comment 38

[David G King]

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

Comment 39

[David G King]

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

Comment 40

[serge (gone)]

all plugins status messages were removed by patch
http://bugzilla.mozilla.org/attachment.cgi?id=81019&action=view
for bug 106411.
IMHO there is no reason to bloat out the code with useless messages which will
be overwritten anyway by same necko messages or eventually by "Document: Done" msg.

Comment 41

[David G King]

The problem is that, when the Java plugin loads (and locks up Mozilla during the
time it loads), Necko isn't telling the user the something legit is happening.
So, the user naturally assumes that Mozilla (or Netscape) may have hung.

Hence, a simple message to say that a plugin is loading, to be blanked out when
Mozilla finishes loading the plugin (or to be blanked out after a few seconds).

Comment 42

[Peter Lubczynski]

You are describing bug 1785. Prompting for all plugins was intentional removed
in bug 106411.

Comment 43

[Kevin McCluskey (gone)]

FYI: If the plugin needs to synchronously paint a status bar message indicating
it was started it should do it only for Java Plugins not all status bar updates.
See http://bugzilla.mozilla.org/show_bug.cgi?id=129844#c46.