479104 - move windows plugin code to unicode

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: make win32 plugin code go all unicode

The current windows code is all non-unicode; for Windows CE, we need it to be unicode.  Here's a patch that uses all the magic TSTR TCHAR etc. stuff.

Comment 1

[Johnny Stenback (:jst)]

Vlad, Crowder, you all need to talk more :( Vlad, see bug 422784 (which is waiting for my review since forever, need to get to that now that we're past 1.9.1 on the trunk).

Question is, which patch do we prefer. I haven't looked in enough detail yet to know the answer...

Comment 2

[Brian Crowder]

This patch is probably better, and certainly fresher, though I haven't tried it in a WinCE build, yet.

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Ack, my fault, sorry -- I was just working through compile problems as a I ran into them =/

This patch is simpler, but crowder's approach of actually doing the conversion to unicode-everywhere might actually be better for consistency... we won't have to guess which mode we're in if problems come up on the desktop.

Comment 4

[Johnny Stenback (:jst)]

So should I review Crowders patch then, and ignore this one, or should we take this one first and work towards Crowders patch? Crowder, do you think your patch is still good to go, or does it need significant work before it's worth reviewing?

Comment 5

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

After talking with crowder, it might make more sense to review this one -- we probably do want unicode everywhere at some point, but this patch is simpler for review purposes and will solve the immediate problem.

Comment 7

[Brian Crowder]

This is the bug we want reviewed, so I've duped bug 422784 here.

Comment 8

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: updated patch

Updated patch; fixed some things that the try server complained about.  (Mainly missing tchar.h include on the desktop.)

Comment 9

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: additional patch to fix sizeof() and a registry bit

Here's an additional patch to the previous, just placed here for ease of review -- it fixes sizeof() -> NS_ARRAY_LENGTH, and also fixes RegSetValueEx.. apparently REG_SZ means either char or wchar depending on which version you call, but the length is still in bytes.  It's all very horrible!

Comment 10

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: updated patch

Combined patch with the above two.

Comment 11

[Johnny Stenback (:jst)]

Comment on attachment 364577 [details] [diff] [review]
updated patch

Looks good.

Comment 12

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

I had to make a few small build changes, and did a tryserver build here -- https://build.mozilla.org/tryserver-builds/2009-03-01_14:[e-mail]-479104unicode-plugins/

I've confirmed that all the plugins I have installed are recognized the same with this patch as without, so gonna land this shortly.

Comment 13

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Checked in.

Comment 14

[[e-mail]]

Comment on attachment 364577 [details] [diff] [review]
updated patch

>+t_NS_NewNativeLocalFile(wchar_t *path, PRBool b, nsILocalFile **retval)
>+{
>+  return NS_NewNativeLocalFile(NS_ConvertUTF16toUTF8(path), b, retval);
>+}
Native != UTF8 so this will fail for plugins in non-ASCII paths...

Comment 15

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Hm, indeed.  Though that code path is only with UNICODE, so it won't affect our current win32 builds.  Will fix in a followup.