Closed
Bug 60064
Opened 24 years ago
Closed 24 years ago
Crashes on "Plugin Downloader" dialog.
Categories
(Core Graveyard :: Plug-ins, defect, P3)
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: rpallath, Assigned: serhunt)
References
()
Details
(Keywords: crash, Whiteboard: suntrak-n6-highp)
Attachments
(2 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
text/html
|
Details |
Invoke Browser.
Prompts for choosing Profile. Create a new Profile
Choose the same and invoke the browser.
Visit www.comdex.com
It will invoke a "Plugin Downloader Popup"
Click on the "OK" button twice/thrice/...
It will crash the browser.
Subsequent invokation however will go thru' smoothly.
No crashes but assertions wil be thrown.
Seems like this crash can be produced very consistently with invokation of a
new profile.
On second invokation on checking the .mozilla/<profile> dir. Found that the
only file that changes is localstore.rdf.
Wondering if it has anything to do with that.
Tried it on Solaris 2.7 (sparc).
The following was used for this build:
1106 Commercial Build
[tag: -r Netscape_20000922_BRANCH -D "10/26/2000 10:00:00"
mozilla source patch: 57725.diffs
PSM: 1.3 & NSS 3.1 (NSS_3_1_RTM tag)
PSM jedimind trick: libpsmshim.so
Cool/AIM: sparc drop 08282000 ,intel drop 10092000
ns source patch for run-mozilla.sh ]
1106 Commercial Blackwood Build
[tag: -r JAVADEV_PR3_20001002 ]
1024 Commercial Build gtk libs
Java Plugin 1.3.0_01 FCS bits
Comment 2•24 years ago
|
||
Comment 3•24 years ago
|
||
Evaluation and fix from Sergei V. Lunegov <lsv@sparc.spb.su>
I found out reason of bug #60064:
on www.comdex.com there are 3 tags which point to flash movies
and for every tag new default plugin and new window
are created.
Every created window is
a modal one. So instead of 3 windows
we can see only one.
And for every tag method "makeWidget" is invoked
in file modules/plugin/default/unix/nullplugin.c
Then invokation of appropriate methods is binded
by method gtk_signal_connect. Since for Cancel button
second argument is Gtk_Widget everything is fine.
And for 'OK' button second argument is PluginInstance.
I'm not expert in GTK and I think
the reason is we same button (the window is modal)
with various sources.
My suggestion is:
in function "void makeWidget(PluginInstance *This)"
(file modules/plugin/default/unix/nullplugin.c)
to set field 'exists' of plugin instance 'This' to 'TRUE'.
And in method "NPError NPP_SetWindow(NPP instance, NPWindow* window)"
(file modules/plugin/default/unix/npshell.c) to add
checking: "if(This->exists != TRUE)"
and only if this statement is true to invoke function "makeWidget(This)".
Diffs for appropriate files are in attachment.
This fix also covers bug #60060
Comment 4•24 years ago
|
||
I'd need av to approve this fix. I'm not at all familiar with this part of the
plugins code. Do we need to bring in a linux guru to review it as well?
blizzard/pavlov?
Comment 5•24 years ago
|
||
mscott: I've added blizzard and pavlov to the cc:. Chris/Stuart, could
you cast your eyes over the evaluation/diffs and comment please? Thanks.
Comment 6•24 years ago
|
||
What happens if there are two plugins with different mime types on a single
page? Does this patch handle that case?
Comment 7•24 years ago
|
||
Adding reply from Sergei, to blizzard's question. My feeling is we should
just let 60064 fix the crasher, and open another bug for the other problem.
Comments please.
Sergei V. Lunegov wrote:
"I thought about this problem.
This patch doesn't handle case when there
are several mime-types which require various plugins on one
page. Problem concerns only non-installed plugins.
If page contains several mime-types which require plugins
and these plugins are installed -- everything is OK.
The window which contains message with suggestion
to download plugin is a modal one. If page
contains several mime-types (and appropriate plugins
are not installed) only one window can be showed at every
time moment. I mean since window is a modal
then it is impossible to show several such windows simultaneously.
This patch prevents browser from crashing
and allows to download first (order of tags in html page)
plugin
but it doesn't solve problem with several mime-types.
I didn't investigate this problems with other browsers (MS-IE)
and I don't know how do they act.
On my opinion there are 2 possibilities:
1) to file new bug with synopsys :"Mozilla doesn't allow to download
several plugins .." and to make refernce to bug 60064
2) to fix this problem in frames of bug 60064".
.
Comment 8•24 years ago
|
||
Oops. I should learn to read all mt email before responding to things.
Here's a followup comment from Sergei:
"I sent all of you message about several mime-types on one page.
I was absolutely incorrect. In previous message were only my theories.
After I sent this message I decided to check up all my considerations
The result is: Mozilla works correct (!!!)
when several mime-types are on one page.
Browser shows several windows: one window for every mime-type
I just forgot that for every another mime-type
the new instance of PluginInstance is created.
In attachment there is testcase for this situation
Mozilla should show 2 dialog windows with suggestion
to download appropriate plugins
With patches for bug #60064 Mozilla doesn't crash."
Comment 9•24 years ago
|
||
Comment 10•24 years ago
|
||
OK sr=blizzard
Assignee | ||
Comment 11•24 years ago
|
||
If 'a' is needed, a=av.
Assignee | ||
Comment 12•24 years ago
|
||
From Sergei Lunegov:
I made add-on fix.
This fix contains only one line -- 'This->exists = FALSE;' this line was added
to function :
NPError
NPP_New(NPMIMEType pluginType,
NPP instance,
uint16 mode,
int16 argc,
char* argn[],
char* argv[],
NPSavedData* saved)
I added this line to initialize field 'This->exists'. Earlier field This->exist'
initialized in method NPP_SetWindow and this wasn't correct because this field
might be changed in other parts of program and the changed value should be kept.
Now the initialization of field 'This->exists' is made during creating of
instance of structure PluginInstance (during creating 'This').
Assignee | ||
Comment 13•24 years ago
|
||
Here would be the diffs assuming that the previous patch has already been
applied:
Index: npshell.c
===================================================================
RCS file: /cvsroot/mozilla/modules/plugin/default/unix/npshell.c,v
retrieving revision 1.8
diff -u -r1.8 npshell.c
--- npshell.c 2000/11/30 21:32:07 1.8
+++ npshell.c 2000/12/08 21:49:39
@@ -124,6 +124,7 @@
This->type = dupMimeType(pluginType);
This->instance = instance;
This->pluginsPageUrl = NULL;
+ This->exists = FALSE;
/* Parse argument list passed to plugin instance */
/* We are interested in these arguments
Assignee | ||
Comment 14•24 years ago
|
||
r=av. Also note that the previous patch (id=10617) is already in.
Comment 15•24 years ago
|
||
Thanks Andrew. Can you check into the trunk with this, or do we
need a super-reviewer? If the latter, who do you recommend?
Comment 16•24 years ago
|
||
sr=blizzard
Assignee | ||
Comment 17•24 years ago
|
||
Checked in.
Comment 18•24 years ago
|
||
Checked in fixes to the OEM branch. a=jdunn
Comment 19•24 years ago
|
||
If this fix is checked in, why is the bug still NEW?
Updated•24 years ago
|
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Comment 20•24 years ago
|
||
An oversight?
Comment 21•24 years ago
|
||
edburns: thank you very much for catching the "new" vs. "fix checked in" status.
I wish somebody were this vigilant with all Bugzilla bugs; if so, maybe the
total bug count would be less than 11000+.
Comment 22•24 years ago
|
||
no longer crashes with linux trunk 0201 build. Marking VERIF.
Status: RESOLVED → VERIFIED
Updated•3 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•