102976 - Display garbled of Quicktime part in Plugins page

Status: VERIFIED FIXED

(Core :: Internationalization, defect, P2)

Description

[Yuying Long]

Build: 10-03 branch build on Mac 9.1-Ja

Steps:
1. Launch browser.
2. Help | About Plug-ins

Result:
You will see the quick time plu-in part is garbled - a screen shot will be followed.

Note:
I motioned this in bug 101967, but OS X works fine now.
bug 93160 has similar problem.

Comment 1

[Yuying Long]

Attachment: 10-03 branch build / Mac9.1-Ja

Comment 2

[Frank Tang]

It canonly be reproduce on Mac right now since Window Plugins do not return
localized description all the time. reassign to nhotta. I guess it is the
following lines
nsPluginHostImpl.cpp:    aDescription.Assign(NS_ConvertASCIItoUCS2(mPluginTag.mD
escription));
nsPluginHostImpl.cpp:    aFilename.Assign(NS_ConvertASCIItoUCS2(mPluginTag.mFile
Name));
nsPluginHostImpl.cpp:    aName.Assign(NS_ConvertASCIItoUCS2(mPluginTag.mName));
nsPluginHostImpl.cpp:      if (aName.Equals(NS_ConvertASCIItoUCS2(mPluginTag.mMi
meTypeArray[index])))

or 

nsPluginHostImpl.cpp:        mDescription.AssignWithConversion(aPluginTag->mMime
DescriptionArray[aMimeTypeIndex]);

We are not sure what is the charset the plugin return? UTF-8 or something else?
Probably we can assume they are the same charet as our nsIPlatformCharset. 
we should find out the charset of mPluginTag.mName by calling nsIPlatformCharset
and then convert it to Unicode properly . 

Mark it as P2.

Comment 3

[nhottanscp]

There are platform specific implementation nsPluginFile::GetPluginInfo.
For Macintosh, the data is stored in 'STR#' in platform charset (e.g. Shift_JIS
if JA MacOS).
For Windows, ::GetFileVersionInfo is used can what is store in lpData are
treated as char* which I think in platform charset as Macintosh.
BOOL GetFileVersionInfo(
  LPTSTR lptstrFilename,  // file name
  DWORD dwHandle,         // ignored
  DWORD dwLen,            // size of buffer
  LPVOID lpData           // version information buffer
);

We may apply charset conversion either in the cross platform code or platform
specific code.

Comment 4

[nhottanscp]

In nsPluginHostImpl.cpp, char* is used for plugin info, change it to PRUnichar*
would be a large change.
I think we can put a charset conversion code in one of the constructor and keep
the strings as UTF-8 to avoid many changes from char* to PRUnichar*. That would
be a smaller change.

nsPluginTag::nsPluginTag(nsPluginInfo* aPluginInfo)
http://lxr.mozilla.org/seamonkey/source/modules/plugin/base/src/nsPluginHostImpl.cpp#812

Cc to [e-mail], would that look okay for you?

Comment 5

[av (gone)]

Can you attach a patch? My concern is that char* members of the nsPluginTag do 
not necessarily initialize at the construction time.

Comment 6

[nhottanscp]

I have not started changing the code, so no patch yet.
How about changing DOMPluginImpl instead?
Currently, funcitons like GetDescription() use NS_ConvertASCIItoUCS2 which we
can change to appropriate charset conversion.

http://lxr.mozilla.org/seamonkey/source/modules/plugin/base/src/nsPluginHostImpl.cpp#3924

Comment 7

[nhottanscp]

Attachment: Patch to use a charset convertor in DOMMimeTypeImpl and DOMPluginImpl instead of NS_ConvertASCIItoUCS2.

Comment 8

[nhottanscp]

av, could you review the patch?

Comment 9

[av (gone)]

Comment on attachment 52318 [details] [diff] [review]
Patch to use a charset convertor in DOMMimeTypeImpl and DOMPluginImpl instead of NS_ConvertASCIItoUCS2.

OK, this looks fine to me, given it is tested 
and fixes the problem, r=av

Comment 10

[Christopher Blizzard (:blizzard)]

Comment on attachment 52318 [details] [diff] [review]
Patch to use a charset convertor in DOMMimeTypeImpl and DOMPluginImpl instead of NS_ConvertASCIItoUCS2.

sr=blizzard

Comment 11

[nhottanscp]

Checked in to the trunk.
Please also test Windows and Linux because the fix is cross platform.

Comment 12

[nhottanscp]

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

Comment 13

[nhottanscp]

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

Comment 14

[Yuying Long]

Not reproducible on 11-16 trunk build, mark it as verified.