confirming with build 2001110103 on Win2k and WM 7.1.
->plug-ins? punt as needed.
This works for me with yesterday's build on NT40. Shrirang? Files I copied are: npdrmv2.dll, npdrmv2.zip, npds.zip, npdsplay.dll.
will chek this asap today..am having some problem with my NT..
OK, I see it now. The problematic file is npwmsdrm.dll.
Yup. You are right. It is unfortunate that these names can have non-UTF8 characters in them. Which character code is this ? PluginName and all other cached fields that come from the plugin could use nsIRegistry::SetBytesUTF8() instead of SetStringUTF8(). Dan, would that work ? The other option is to escape/unescape them while writing/reading from registry. Dan, what is the best approach here.
The "UTF8" registry APIs expect UTF8, validate the UTF8, and return a specific error indicating not-UTF8. If you had Unicode you could use the SetString() API, but then you have to worry about what charset the plugin is in in order to convert correctly. You can, as dp suggests, use SetBytes or SetBytesUTF8 (which in this case merely refers to the character coding of the name arg) to preserve the random bytes passed in, but that merely moves the problem since at some point to display this string you're still going to have to figure out what charset it's in and convert to Unicode.
Created attachment 57869 [details] [diff] [review] not fully working patch I did not use string stuff much so far. This patch attempts to address the presence of spec chars in plugin name and description. And it sort of work. dp and dveditz, please advise on how to make it better.
Created attachment 57871 [details] screen shot of how it looks with the patch applied Please note an extra character just before the trademark symbol. There should be nothing, not even a space char. When I look at it in the debugger, the '(R)' symbol value is -82 (negative eighty two).
*** Bug 110011 has been marked as a duplicate of this bug. ***
The default three lines of context is pretty useless for people unfamiliar with code except for the most trivial patches; please use more context for reviews. The deprecated "WithConversion" string methods or explicit conversions from or to ASCII are a big red flag -- do you really have ASCII text? (by ascii we mean the strict 7-bit charset, not the various 8-bit "ascii-plus" national sets people sometimes call ascii, especially here in the US.) If you did then the UTF8 registry methods wouldn't have given you any problems since ascii is a subset of UTF8. Most likely you have Latin-1 data, which means it'll be something completely different on Asian systems and the ConvertASCIItoUCS2() functions will *really* mess you up. Who calls nsPluginsHostImpl::RegisterPlugin()? The only calls I could find were under module/plugin/samples and in the oji/MRJ directories, neither of which seemed right. Who calls this will help determine which charset we should use/expect. Or instead of the ASCII->Unicode conversion just store the text as bytes and let the conversion errors be dealt with somewhere else. Chopped off your patch in the RegisterPlugin routine was a for-loop that also uses SetStringUTF8 for the mime types, descriptions and extensions. The descriptions in particular might be translated into a non-ascii charset so I think the UTF8 routines here are problematic.
dveditz, thanks for your thoughts. RegisterPlugin is supposed to be called by xpcom plugins which are now deprecated. MRJ plugin uses it on Mac. A couple of commersial products are on the market already, e.g. RealPlayer. I'll try SetBytes approach and see how it works.
Created attachment 58184 [details] [diff] [review] patch to fix the bug -- now using SetBytesUTF8 instead of SetString This patch works with no visible problems so far.
Created attachment 58185 [details] the same patch as above, but manually modified to include more surrounding context
If this way to store string as bytes seems to be acceptable, should I explicitly terminate the array with null after |GetBytesUTF8(...)| before I cast it to |const char *|?
Since you are writing strlen()+1 I dont think you need to try null terminate it. It should happen automatically. Patch looks good to me.
Formal r=/sr= please?
You don't need to manually copy in more context, just pass the argument to the cvs diff command so the result is a valid patch that can be applied and tested. Thanks for the context though -- much appreciated! -- I'm just noting that the cvs diff argument is easier and results in a patch that can be used as the main review attachment so you don't have to make two attachments. You did not address my concern about the remaining three SetStringUTF8 calls in nsPluginHostImpl::RegisterPlugin. I'll grant that most likely the MIME type and extension are 7-bit ascii (maybe even required by the spec?), but I don't trust the description. You're not going to bail on error, that's good, but what happens later if there's an uninitialized description? Other than that though the rest looks good and I'll happily give sr= if you reassure me about that one point.
I know about cvs diff -uN thing, just thought that including the whole C function is better. Peter L. told me that there is such cvs option on Mac, but I am not aware of the Win analogue. I will gladly change the the remaining three cases along the same line if dp does not object. dp, do you?
The other three calls are mimetype, mimetype description and mimetype extension. I can see description might need it but not the other two.
dp: isn't that what I said? I have more concerns than I did about MIME type and extension, though. It looks like those values are passed in from the plugin itself, and even if legal values only contain 7-bit ascii (and no-one has asserted that, it's an optimistic guess on my part) there's no guarantee the calling program won't screw up. These reg entries won't get created and this routine has no problem with that, but what happens later when these values aren't found? av: including the whole function is much appreciated. You can usually jigger the context value if you have to make sure its included. -u20 would have gotten nearly all of it (-u30 if you wanted overkill) and has the advantage of 1) being easier to produce for you than hand editing and 2) results in a valid patch that can be reviewed and applied
Dan, yeah. Thats what I get for fastreading...av it is up to you what you wanna do here.
Created attachment 58818 [details] [diff] [review] patch -- now including mimedescriptions to SetBytesUTF8 business Please review/superreview. I guess we are fine expecting lower ASCII chars only for mimetypes and file extensions. If this whenever happens we will fail to launch a plugin as the worst scenario. Evangelism issue, if any. I'll talk about it on the next group gathering, perhaps the best way is just to mention this fact in the plugin documentation.
Comment on attachment 58818 [details] [diff] [review] patch -- now including mimedescriptions to SetBytesUTF8 business sr=dveditz, thanks for making these changes
*** Bug 112046 has been marked as a duplicate of this bug. ***
*** Bug 112050 has been marked as a duplicate of this bug. ***
I checked this in to the trunk. dp, I guess we should also get it into the branch, should we? What branches will benefit?
I dont think plugin caching is on the 0.9.4 branch. So this wont matter for that branch.
*** Bug 112187 has been marked as a duplicate of this bug. ***
*** Bug 112102 has been marked as a duplicate of this bug. ***
Marking fixed. Please file a separate bug for other issues mentioned if needed. For verification purposes please use npwmsdrm.dll on Windows or plugger on Linux.
Sorry, still doesn`t work for me with 20011127 and 20011128 trunk builds, you should verify the code has actually been checked in or reopen.
Martin, what exactly do you see? The best way to see the original problem (which is I beleive fixed) is to place npwmsdrm.dll in Mozilla plugins folder (not Netscape4's) and on about:plugins page see how it looks. Can you please try this?
I'm using build 2001112809 on Win2k (SP2). I just removed all plugins except for npnull.dll from my Netscape4 plugins directory and and have been testing various plugins on their own in Mozilla's plugins directory. I haven't found one plugin yet that *doesn't* cause that error to be thrown. These include: npwmsdrm.dll (Windows Media Player) nppl3260.dll (Real Player) npswf32.dll (flash) np32dsw.dll (shockwave) NPAXDLPI.dll (not sure what plugin) All the Java plugins files etc... This build must have the fix that was checked into the trunk. Did it do more harm than good? Jake
See my description in the Bug 110011 I filed (duplicate) for a description of my problem and the list of plugins I have (I have npwmsdrm.dll and I still see the same problem as before).
I don't have netscape 4.x installed too, must I add. Netscape 6.x and Internet explorer are not on my system too. The only browser I have is mozilla.
*** Bug 112778 has been marked as a duplicate of this bug. ***
I built Mozilla for Solaris/x86 again from the 29 Nov sources, and this time about:plugin works perfectly. Either I did something wrong yesterday, or the bug was finally squashed during the last 24 hours.
Well, the *was* fixed, but I just got Build 2001120303, and now I get the original symptoms, though in more situations. Error: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDOMPlugin.name]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: jar:resource:///chrome/toolkit.jar!/content/global/plugins.html :: <TOP_LEVEL> :: line 95" data: no] So I put only npnul32.dll in the plugins dir, and I get the SAME EXCEPTION, but at leas a line does display... So I added the NPJava11.dll, only the default plugin is displayed, the exception is thrown. NPJava12.dll, only the default plugin is displayed, the exception is thrown. NPJava131.dll, only the default plugin is displayed, the exception is thrown. NPJava32.dll, only the default plugin is displayed, the exception is thrown. any other plugin dll, now no plugins are displayed, and the exception is thrown. Zac (original reporter)
*** Bug 113382 has been marked as a duplicate of this bug. ***