Last Comment Bug 217715 - Java applet fails to get some of its parameters -- getParameter() returns null
: Java applet fails to get some of its parameters -- getParameter() returns null
Status: RESOLVED FIXED
fixed-aviary1.0
: fixed1.7, helpwanted
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: x86 All
: -- major with 2 votes (vote)
: ---
Assigned To: Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
:
Mentors:
jar:http://bugzilla.mozilla.org/attac...
: 145979 237657 (view as bug list)
Depends on: 239152 144072 585153
Blocks: 242733
  Show dependency treegraph
 
Reported: 2003-08-29 05:14 PDT by Petar Krasimirov
Modified: 2010-08-06 12:58 PDT (History)
15 users (show)
chofmann: blocking1.7+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase - applet source (569 bytes, text/plain)
2004-03-04 23:23 PST, Kyle Yuan
no flags Details
testcase - perl script to generate the html page (326 bytes, text/plain)
2004-03-04 23:24 PST, Kyle Yuan
no flags Details
testcase - zipped version (50.97 KB, application/zip)
2004-03-04 23:54 PST, Kyle Yuan
no flags Details
attempt #1 (47.85 KB, patch)
2004-05-28 00:48 PDT, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
no flags Details | Diff | Splinter Review
attempt#2 (48.97 KB, patch)
2004-05-28 16:04 PDT, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
jst: review+
jst: superreview+
mozilla: approval1.7+
Details | Diff | Splinter Review
branch patch (86.73 KB, patch)
2004-06-02 06:53 PDT, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
no flags Details | Diff | Splinter Review
Patch that compiles on 1.7 branch. (72.04 KB, patch)
2004-06-03 09:35 PDT, Doron Rosenberg (IBM)
no flags Details | Diff | Splinter Review
remove marquee stuff from my tree :) (69.97 KB, patch)
2004-06-03 09:47 PDT, Doron Rosenberg (IBM)
no flags Details | Diff | Splinter Review
Make textarea's method return a PRBool per roc (69.94 KB, patch)
2004-06-03 10:28 PDT, Doron Rosenberg (IBM)
no flags Details | Diff | Splinter Review

Description Petar Krasimirov 2003-08-29 05:14:27 PDT
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] 2003-08-29 06:03:38 PDT
.
Comment 2 Petar Krasimirov 2003-09-19 02:56:50 PDT
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 2004-03-02 04:13:44 PST
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 2004-03-02 05:31:09 PST
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 2004-03-04 23:23:27 PST
Created attachment 142982 [details]
testcase - applet source
Comment 6 Kyle Yuan 2004-03-04 23:24:50 PST
Created attachment 142983 [details]
testcase - perl script to generate the html page
Comment 7 Kyle Yuan 2004-03-04 23:35:08 PST
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 [:bz] 2004-03-04 23:44:06 PST
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 2004-03-04 23:54:39 PST
Created attachment 142985 [details]
testcase - zipped version
Comment 10 Kyle Yuan 2004-03-05 00:03:26 PST
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 2004-03-05 03:53:43 PST
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 [:bz] 2004-03-05 09:17:09 PST
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 2004-03-06 20:36:10 PST
-> Layout per comment 12.
Comment 14 Jonas Sicking (:sicking) PTO Until July 5th 2004-03-09 14:43:23 PST
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 2004-05-02 02:16:45 PDT
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 2004-05-07 18:28:24 PDT
*** Bug 145979 has been marked as a duplicate of this bug. ***
Comment 17 Boris Zbarsky [:bz] 2004-05-27 17:53:46 PDT
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) (Exited; email my personal email if necessary) 2004-05-27 21:24:49 PDT
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 [:bz] 2004-05-27 21:31:40 PDT
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) (Exited; email my personal email if necessary) 2004-05-27 23:08:26 PDT
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) (Exited; email my personal email if necessary) 2004-05-28 00:48:00 PDT
Created attachment 149486 [details] [diff] [review]
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) 2004-05-28 13:09:15 PDT
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) (Exited; email my personal email if necessary) 2004-05-28 13:32:43 PDT
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) (Exited; email my personal email if necessary) 2004-05-28 16:04:39 PDT
Created attachment 149541 [details] [diff] [review]
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) 2004-06-01 09:38:35 PDT
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) (Exited; email my personal email if necessary) 2004-06-01 09:47:49 PDT
Comment on attachment 149541 [details] [diff] [review]
attempt#2

Boris can review this.
Comment 27 Boris Zbarsky [:bz] 2004-06-01 09:52:15 PDT
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, jst@mozilla.com) 2004-06-01 10:08:22 PDT
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) (Exited; email my personal email if necessary) 2004-06-01 10:25:09 PDT
I was trying to resist deCOMtaminating that, but sure, I can do it :-)
Comment 30 Johnny Stenback (:jst, jst@mozilla.com) 2004-06-01 10:30:57 PDT
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) (Exited; email my personal email if necessary) 2004-06-01 10:38:25 PDT
good idea. I'll do that.
Comment 32 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-06-01 18:01:06 PDT
checked into trunk. leaving the bug open for possible branch checkin.
Comment 33 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-06-01 19:54:12 PDT
I think this reduced Tp on btek :-)
Comment 34 Johnny Stenback (:jst, jst@mozilla.com) 2004-06-01 21:38:46 PDT
Sweet! :-)
Comment 35 Kyle Yuan 2004-06-01 23:21:33 PDT
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] 2004-06-02 04:06:50 PDT
Comment on attachment 149541 [details] [diff] [review]
attempt#2

a=mkaply for 1.7
Comment 37 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-06-02 06:24:20 PDT
The branch backport of this is tricky because of some refactoring that jst did
in 1.8a1.
Comment 38 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-06-02 06:53:47 PDT
Created attachment 149829 [details] [diff] [review]
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) 2004-06-02 08:06:26 PDT
I'll test this on linux 1.7 branch today.
Comment 40 Doron Rosenberg (IBM) 2004-06-03 08:31:27 PDT
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) (Exited; email my personal email if necessary) 2004-06-03 09:11:46 PDT
correct
Comment 42 Doron Rosenberg (IBM) 2004-06-03 09:35:26 PDT
Created attachment 149928 [details] [diff] [review]
Patch that compiles on 1.7 branch.
Comment 43 Doron Rosenberg (IBM) 2004-06-03 09:47:00 PDT
Created attachment 149929 [details] [diff] [review]
remove marquee stuff from my tree :)
Comment 44 Doron Rosenberg (IBM) 2004-06-03 10:28:11 PDT
Created attachment 149936 [details] [diff] [review]
Make textarea's method return a PRBool per roc
Comment 45 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-06-03 10:30:11 PDT
Looks good. Check it in.
Comment 46 Bogdan Stroe 2004-06-03 13:19:01 PDT
*** Bug 237657 has been marked as a duplicate of this bug. ***
Comment 47 chris hofmann 2004-06-03 21:19:33 PDT
did this get checked in?  can it be marked fixed?
Comment 48 Mike Kaply [:mkaply] 2004-06-04 05:28:10 PDT
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+8 (review requests must explain patch) 2004-06-04 17:17:39 PDT
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) (Exited; email my personal email if necessary) 2004-06-04 17:56:33 PDT
Right!
Comment 51 Mike Kaply [:mkaply] 2004-06-04 20:29:45 PDT
I'll put this in aviary since there is a tiny correction to doron's patch (not
code related)

Note You need to log in before you can comment on or make changes to this bug.