217715 - Java applet fails to get some of its parameters -- getParameter() returns null

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Petar Krasimirov]

User-Agent:       Mozilla/5.0 (compatible; Konqueror/3.1; Linux)
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4) Gecko/20030701

I use latest java from Sun - 1.4.2. I have tried with Java 1.4.1, 1.3.1, 1.3.2 - same results. 
 
I open a web-page with java applet in it. The HTML contains APPLET tag with about 900 
PARAM attributes. Some of them are long strings - about 1k or 2k text (no fancy symbols, 
just ascii). 
 
In the init() of my applet (extends JApplet) I try to read them one after another via the 
getParameter(String) method. Sometimes I recieve (incorrect) null result and I can clearly 
see the correct PARAM code in HTML page. When I recieve the first incorrect null, all other 
getParameter() requests return null, except for the already-recieved parameters (I suppose 
they are cached). 
 
On a page reload from the server, the applet fails to get the same or different parameter. It 
looks very much to me as a threads race. If I save the HTML page on my disk and I open it 
from there (applet still loads from server) - no getParameter() ever fails. 
 
I tried to find out on which PARAM number applet fails -- it differs every time. Sometimes on 
the 512-th PARAM, sometimes on 318-th, sometimes it doesn't fail (get them all). 
 
I tried to start a separate java thread in the beginning of init() and the parent thread goes 
out of init() (returns). The child-thread was responsible to get the applet parameters. It 
resulted no change in behaviour -- same as with no forking. 
 
The applet always loads correct with MSIE 5.5, 6.0 on Windows 2k/XP with Sun java 1.3.x and 
1.4.x. 
The applet always loads correct with Konqueror 3.0.5 from RedHat 8 with Sun java 1.3.1 and 
1.3.2. The Konqueror browser crushes when few pages with the applet (with different applet 
parameters) are opened at the same time. The applet was "extends Applet", not "extends 
JApplet" at this time. I don't know if it does matter... 
 
The applet fails (getParam() returns null) with Mozilla 1.3 and 1.4 on RedHat 8 with java 1.4.2 
plugin ns610 and on RedHat 9 with java 1.4.2 plugin ns610_gcc3.2. 
The applet fails (getParam() returns null) with Mozilla 1.3 and 1.4  on Windows 2k/XP with 
Sun java 1.3.x and 1.4.0. 
The applet fails (getParam() returns null) with Mozilla 1.3 and 1.4 on Debian unstable (nightly 
updated) with java 1.4.2 plugin ns610_gcc3.2. 
The applet fails (getParam() returns null) with Konqueror 3.1, 3.1.1, 3.1.2, 3.1.3 on Debian 
unstable (nightly updated). 
 

Reproducible: Always

Steps to Reproduce:
1. Create a web-page with APPLET and 900 PARAM attributes. I doubt the lenght of PARAM's 
values does matter. Some of mine are empty (""), some are 2k ascii-text long. 
2. Make the web-page available via a server. I serve it with tomcat and it is generated 
on-the-fly, so there is delay (downloads slow). 
3. Write applet that tells'ya on which PARAM it recieves null. 
4. Open the page with Mozilla. 
Actual Results:  
getParameter(String) returns null and I clearly see the PARAM attribute - it is not null. 

Expected Results:  
Mozilla should give to my applet the requested parameter's value.

Comment 1

[Matthias Versen [:Matti]]

.

Comment 2

[Petar Krasimirov]

You can see example of this bug at http://noms.neterra.net/ - use guest/novuser to login, 
then click "reports" on the left and then click the left eye-view icon to open the report with 
applets. If some parameters are not found the curve won't be displayed. So if you see curves 
1/4, 2/4, 3/4 and no 4/4 curve, this means that some applet parameters are missing. 

Comment 3

[Kyle Yuan]

When clicked the "reports" link, I only saw the "Loading. Please wait..." in the
main screen, there is no applet. Reporter, did the test site change?

Comment 4

[Petar Krasimirov]

We upgraded our server and it is now able to generate web-pages more quickly. This reduced 
the occurrence of the bug on all client machines. 
 
The bug is more often seen on slow mashines running Mozilla. On Pentium4 at 2.4 GHz the 
bug almost never appears. On Pentium3 at 600 MHz the bug can be seen very often. 
 
The test site does not work anymore. The applet was rewritten to avoid the bug. Instead of 
getting all of its data from the PARAMs in the web-page, the applet now fetch data via 
another TCP connection back from server. 
 
The bug in Mozilla remains. The bug is also found in the Phoenix/Firebird/Firefox browser. 
 
The bug can be reproduced with high probability when a HTML page is slowly passed to 
Mozilla and Mozilla is running on slow machine. 

Comment 5

[Kyle Yuan]

Attachment: testcase - applet source

Comment 6

[Kyle Yuan]

Attachment: testcase - perl script to generate the html page

Comment 7

[Kyle Yuan]

In the testcase I just attached, I created a simple applet with ~10000
parameters. In applet's init() method, I try to get all of them. Save the html
and class in a local server, I usually can get all params sucessfully on a P4
1.8GHz box, but only get 2000~3000 params on a 450MHz sparc. If I store them in
a outside server, I can only get 300~800 params every time depends on network
bandwidth.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Kyle, any chance of an actual functioning testcase?  Like the compiled java
bytecode plus the generated HTML pointing to it, all in a nice zip file soit can
just be downloaded, decompressed, and loaded in the browser?  It should compress
quite nicely...

The basic issue, I suspect, is that someone is calling BeginUpdate(), so we
flush out the data as it exists at that point.  The question is who's calling
BeginUpdate, and it would be pretty simple to answer with a usable testcase.

Comment 9

[Kyle Yuan]

Attachment: testcase - zipped version

Comment 10

[Kyle Yuan]

The same testcase works fine with appletviewer. I'm looking into the java source
code to see how getParameter() gets implemented.

Comment 11

[Kyle Yuan]

the problem is likely in mozilla side. If I inserted a printf after this line:
http://lxr.mozilla.org/seamonkey/source/layout/html/base/src/nsObjectFrame.cpp#2892
I found the total number of params was already wrong at that time.

Comment 12

[Boris Zbarsky [:bzbarsky]]

OK.  So the problem seems to be that a reflow event that we post (probably when
we open the body) puts us in PresShell::ProcessReflowCommands, which manually
flushes out all pending notifications.

Given that and the various other codepaths that can result in FlushTags() being
called in the middle of an <object>/<applet>, I think the right course of action
is to simply not append such nodes to the document until we see the end tag
(like we do for scripts in the XML content sink).  Will that break anything? 
Can one put script with document.write() calls inside <applet>?  If so, we can't
do what I propose and instead need to look at ways to prevent content flushing
while mInMonolithicContainer is true....

Comment 13

[Kyle Yuan]

-> Layout per comment 12.

Comment 14

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

I'd really like to say that i think we should just wait with inserting the
<applet> until we reach the </applet>-tag, in fact the code in XML-sink was
designed to support things like that.

Unfortunatly I don't think we can do that since it would break pages like:

<applet>
  <script>document.write('<param name=foo value=bar>');</script>
  <param name=otherParam value=dummy>
</applet>

Since the <script> wouldn't be executed until we close the applet, which means
that the data would be written to the wrong place...

Oh, how I hate document.write...

Comment 15

[Nicola Asuni]

I'm experiencing the same problem on the menu applet on www.tecnick.com using 
various version of Mozilla with various version of Java Sun Plugins and various 
version of Linux and Windows.

I've deeply tested the java applet and seems that Mozilla sometimes try to 
render the applet before loading all parameters. This happens more frequently 
if the applet has a large number of parameters and the connection is slow.

This bug seems related to the 145979 bug.

Comment 16

[Kyle Yuan]

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

Comment 17

[Boris Zbarsky [:bzbarsky]]

roc suggested just not instantiating the plugin if DoneCreatingElement() has not
been called on the node yet (and notifying the frame when it gets called).

That may be the simplest and cleanest solution to this bug...  One that could
even be done for 1.7, I think.

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

DoneCreatingElement only gets called on a few node types and it gets called as
soon as the tag is parsed. We need something else.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Argh.  I was thinking about the post-bug 239152 world.

As it is, we could add a DoneAddingChildren hack like <textarea> and <select>
do... :(

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

That is exactly what I have done. In fact I've unified those hacks into a couple
of methods in nsIContent.

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: attempt #1

Here we go. The patch looks big but it's actually quite simple (although bigger
than it needs to be, I suppose, because of the cleanup that I did).

-- make all NS_NewHTML...Frames take an aFromParser parameter, because that
makes the content sink a bit simpler
-- add DoneAddingChildren/IsDoneAddingChildren methods on nsIContent, only
meaningfully implemented in textareas, applets, objects and selects; if an
element is not created by the parser, then it is always 'done adding children'
-- remove those methods from nsITextArea/nsISelect
-- nsObjectFrame checks that the children are done, or else it refuses to
instantiate the plugin
-- if the applet and object elements detect that children are done, they
recreate all their frames (if any). Normal case is there aren't any frames yet.
It would be slightly more efficient to existing frames, but that seems harder
to do from content. It might be slightly safer too, but it *looks* OK to do the
frame recreation from the sink callback.

Now, with this patch, things still seem to generally work, and I can use Flash
plugins. But I can't get Java to work here at all in any build with or without
the patch, so I can't test Java, and I can't see if the test is fixed. Not sure
why. doron, want to take it for a spin?

Comment 22

[Doron Rosenberg (IBM)]

I crash on windows and linux with the java plugin installed - on windows, I
can't get a stack.  Going to see if linux is better.

Comment 23

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think this is because I forgot to include a file in the patch! sorry! I'll
post a new patch.

Comment 24

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: attempt#2

Sorry, I changed a file in parser and forgot to put it in the diff ... probably
because I never change files in parser :-)

Comment 25

[Doron Rosenberg (IBM)]

I treid the patch and it indeed seems to fix the problems we have seen.  

Nominating this as a 1.7 blocker, as this affects Java-using corps like IBM and
Sun :)

Who would be the right people to review this?

Comment 26

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 149541 [details] [diff] [review]
attempt#2

Boris can review this.

Comment 27

[Boris Zbarsky [:bzbarsky]]

I'll try to get to this before Thurs evening, but if that doesn't happen then it
won't happen till Monday (I'm gone all weekend).

Comment 28

[Johnny Stenback (:jst)]

Comment on attachment 149541 [details] [diff] [review]
attempt#2

Drive-by review comment:

+  virtual void IsDoneAddingChildren(PRBool* aIsDone)

This should be:

+  virtual PRBool IsDoneAddingChildren()

Comment 29

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I was trying to resist deCOMtaminating that, but sure, I can do it :-)

Comment 30

[Johnny Stenback (:jst)]

Comment on attachment 149541 [details] [diff] [review]
attempt#2

+  nsHTMLTextAreaElement(nsINodeInfo *aNodeInfo, PRBool aFromParser);
   nsHTMLTextAreaElement(nsINodeInfo *aNodeInfo);

How about adding an argument to the existing constructor with a default value
in stead of adding a whole new constructor?

r+sr=jst with that, and my drive-by review comment addressed.

Comment 31

[Robert O'Callahan (:roc) (email my personal email if necessary)]

good idea. I'll do that.

Comment 32

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked into trunk. leaving the bug open for possible branch checkin.

Comment 33

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think this reduced Tp on btek :-)

Comment 34

[Johnny Stenback (:jst)]

Sweet! :-)

Comment 35

[Kyle Yuan]

Comment on attachment 149541 [details] [diff] [review]
attempt#2

I tested it on both Solaris & Linux, it works like a charm. Thanks for fixing
this. Should we ask for approval1.7?

Comment 36

[Mike Kaply [:mkaply]]

Comment on attachment 149541 [details] [diff] [review]
attempt#2

a=mkaply for 1.7

Comment 37

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The branch backport of this is tricky because of some refactoring that jst did
in 1.8a1.

Comment 38

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: branch patch

Here's the backport. Someone with a full branch checkout needs to build, test,
and check in this patch.

Comment 39

[Doron Rosenberg (IBM)]

I'll test this on linux 1.7 branch today.

Comment 40

[Doron Rosenberg (IBM)]

You use:

+PRBool
+nsHTMLTextAreaElement::IsDoneAddingChildren()
+{
+  return mDoneAddingChildren;
+}

yet you define it as:
+ virtual void IsDoneAddingChildren(PRBool* aIsDone);

I changed the definition to match the implementation and it compiled, I assume
that is what you meant?

Comment 41

[Robert O'Callahan (:roc) (email my personal email if necessary)]

correct

Comment 42

[Doron Rosenberg (IBM)]

Attachment: Patch that compiles on 1.7 branch.

Comment 43

[Doron Rosenberg (IBM)]

Attachment: remove marquee stuff from my tree :)

Comment 44

[Doron Rosenberg (IBM)]

Attachment: Make textarea's method return a PRBool per roc

Comment 45

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Looks good. Check it in.

Comment 46

[Bogdan Stroe]

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

Comment 47

[chris hofmann]

did this get checked in?  can it be marked fixed?

Comment 48

[Mike Kaply [:mkaply]]

I had to back out from 1.7 because of a mistake checking in.

It will go in again in an hour or so and then I will close it.

Comment 49

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

It looks like mkaply landed this on the 1.7 branch 2004-06-04 08:30 -0700.  Has
it landed on the trunk as well?  Should this now be marked FIXED and fixed1.7?

Comment 50

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Right!

Comment 51

[Mike Kaply [:mkaply]]

I'll put this in aviary since there is a tiny correction to doron's patch (not
code related)