Closed Bug 422079 Opened 14 years ago Closed 14 years ago

Plugin names need to be converted to Unicode

Categories

(Core :: Plug-ins, defect, P2)

x86
Windows XP
defect

Tracking

()

RESOLVED FIXED

People

(Reporter: neil, Assigned: masayuki)

References

Details

Attachments

(3 files)

So that it can provide correct UTF-8 to nsPluginHostImpl.
Flags: blocking1.9?
Attached patch Proposed patchSplinter Review
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #308615 - Flags: superreview?(jst)
Attachment #308615 - Flags: review?(masayuki)
Attached patch -wup versionSplinter Review
Comment on attachment 308616 [details] [diff] [review]
-wup version

Aargh, bugzilla strikes again.
Attachment #308616 - Attachment is patch: true
Attachment #308616 - Attachment mime type: application/octet-stream → text/plain
neil:

I have the questions:

1. Is this a regression of bug 420285?
2. Can this fix this bug? I worry that you sets the UTF-8 string to plugin info. But plugin host should think that the encoding of plugin info is native code on Win32. I think we need to update plugin host too. I.e., plugin host should have mIsUTF8 flag and it should be checked before calling nsPluginTag::EnsureMembersAreUTF8. (If it is true, the method must not be called.) And I need to check other plugin info used points for this patch...
Flags: blocking1.9? → blocking1.9+
Priority: -- → P2
(In reply to comment #4)
>1. Is this a regression of bug 420285?
Yes, thus the dependency.

>2. Can this fix this bug? I worry that you sets the UTF-8 string to plugin
>info. But plugin host should think that the encoding of plugin info is native
>code on Win32. I think we need to update plugin host too. I.e., plugin host
>should have mIsUTF8 flag and it should be checked before calling
>sPluginTag::EnsureMembersAreUTF8. (If it is true, the method must not be
>called.) And I need to check other plugin info used points for this patch...
In bug 420285 you changed the signature of nsIPluginTag from nsACString to nsAUTF8String but you didn't change the data that it returns to UTF-8. You always pass PR_TRUE as aArgsAreUTF8, so you never call EnsureMembersAreUTF8. (If the other platforms are already UTF8, which I'm sure they are, does this mean that this patch obsoletes aArgsAreUTF8 and EnsureMembersAreUTF8?)
(In reply to comment #5)
> (In reply to comment #4)
> >1. Is this a regression of bug 420285?
> Yes, thus the dependency.

Ugh, I'm sorry.

> >2. Can this fix this bug? I worry that you sets the UTF-8 string to plugin
> >info. But plugin host should think that the encoding of plugin info is native
> >code on Win32. I think we need to update plugin host too. I.e., plugin host
> >should have mIsUTF8 flag and it should be checked before calling
> >sPluginTag::EnsureMembersAreUTF8. (If it is true, the method must not be
> >called.) And I need to check other plugin info used points for this patch...
> In bug 420285 you changed the signature of nsIPluginTag from nsACString to
> nsAUTF8String but you didn't change the data that it returns to UTF-8. You
> always pass PR_TRUE as aArgsAreUTF8, so you never call EnsureMembersAreUTF8.

No, the members of nsPluginTag must be UTF-8. And nsPluginTag that is created from nsPluginInfo is always calling EnsureMembersAreUTF8.

> (If the other platforms are already UTF8, which I'm sure they are, does this
> mean that this patch obsoletes aArgsAreUTF8 and EnsureMembersAreUTF8?)

No, Mac is using native code for descriptions of plugin info. And also OS/2...
On my Win32 system the bug is fixed by my patch, so, I think that my patch has another bug, maybe.
Attached patch patchSplinter Review
Ugh... I think this patch might fix the bug of bug 420285 comment 15.

If so, I hope that Neil's patch will be landed after Gecko1.9. (for the risk)
Attachment #309172 - Attachment is patch: true
Neil:

Would you test my patch? I cannot test it, because my environment doesn't have intl name plugins.
(In reply to comment #6)
>No, the members of nsPluginTag must be UTF-8. And nsPluginTag that is created
>from nsPluginInfo is always calling EnsureMembersAreUTF8.
Ah, I see now, I thought you had converted nsPluginInfo to UTF-8 too.

(In reply to comment #8)
>Would you test my patch?
Your patch fixes my plugin that uses Windows-1252 characters.
Attachment #308615 - Flags: superreview?(jst)
Attachment #308615 - Flags: review?(masayuki)
Comment on attachment 309172 [details] [diff] [review]
patch

Neil:

Thank you for your testing.
Attachment #309172 - Flags: superreview?(jst)
Attachment #309172 - Flags: review?(jst)
Duplicate of this bug: 423166
Resummarising, since converting native plugin providers to Unicode is a separate (post-1.9) issue.
Assignee: neil → masayuki
Status: ASSIGNED → NEW
Summary: nsPluginsDirWin.cpp needs to use W APIs → Plugin names need to be converted to Unicode
Attachment #309172 - Flags: superreview?(jst)
Attachment #309172 - Flags: superreview+
Attachment #309172 - Flags: review?(jst)
Attachment #309172 - Flags: review+
checked-in, thanks!
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Blocks: 423340
You need to log in before you can comment on or make changes to this bug.