Closed Bug 60064 Opened 24 years ago Closed 24 years ago

Crashes on "Plugin Downloader" dialog.

Categories

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

Sun
Solaris
defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: rpallath, Assigned: serhunt)

References

()

Details

(Keywords: crash, Whiteboard: suntrak-n6-highp)

Attachments

(2 files)

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
Updated Status Whiteboard
Whiteboard: suntrak-n6-highp
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
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?
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.
What happens if there are two plugins with different mime types on a single page? Does this patch handle that case?
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". .
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."
OK sr=blizzard
If 'a' is needed, a=av.
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').
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
r=av. Also note that the previous patch (id=10617) is already in.
Thanks Andrew. Can you check into the trunk with this, or do we need a super-reviewer? If the latter, who do you recommend?
Keywords: crash
sr=blizzard
Checked in.
Checked in fixes to the OEM branch. a=jdunn
If this fix is checked in, why is the bug still NEW?
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
An oversight?
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+.
no longer crashes with linux trunk 0201 build. Marking VERIF.
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: