|Summary:||Fix mix of unicode and ansi functions in PluginDatabaseWin.cpp|
|Product:||WebKit||Reporter:||Adrien Nader <camaradetux>|
|Version:||528+ (Nightly build)|
Description Adrien Nader 2009-02-07 08:05:51 PST
In WebCore/plugins/win/PluginDatabaseWin.cpp, there is currently a mix of unicode and ansi functions. That makes me think nobody ever tried to build that with UNICODE not defined (in the -DUNICODE sense).
Comment 1 Adrien Nader 2009-02-07 08:08:02 PST
Created attachment 27448 [details] Use unicode functions everywhere and always I decided to use the unicode functions everywhere and unconditionnaly. That should make it equivalent to what is available currently.
Comment 2 Adrien Nader 2009-02-07 08:09:17 PST
Forgot to say : I am aware that there is no changelog, I forgot it once again...
Comment 3 Darin Adler 2009-02-07 11:18:08 PST
Comment on attachment 27448 [details] Use unicode functions everywhere and always You meant to set this to review? -- asking for review. Not review+ which means it has been reviewed.
Comment 4 Alexey Proskuryakov 2009-02-08 02:23:04 PST
Comment on attachment 27448 [details] Use unicode functions everywhere and always This patch doesn't look right to me. The functions you are renaming are not ANSI - e.g. SHGetValue is a macro defined to either SHGetValueA or SHGetValueW, depending on whether UNICODE is defined. The same is true of the TEXT macro. I'm not a huge Windows expert, but I'm sufficiently sure about these macros' behavior to say r-. But please feel free to mark this for review again if I'm actually wrong.
Comment 5 Adrien Nader 2009-02-08 03:47:12 PST
(In reply to comment #4) > (From update of attachment 27448 [details] [review]) > This patch doesn't look right to me. The functions you are renaming are not > ANSI - e.g. SHGetValue is a macro defined to either SHGetValueA or SHGetValueW, > depending on whether UNICODE is defined. The same is true of the TEXT macro. > > I'm not a huge Windows expert, but I'm sufficiently sure about these macros' > behavior to say r-. But please feel free to mark this for review again if I'm > actually wrong. > Right, by "mix" I actually meant "potential mix", when UNICODE is not defined. Of course it's possible to use the macros everywhere but not trying to restore ansi and only having unicode looked like the right choice. So the question is : do we want to restore the possibility to use the ansi version of these functions or should we go unicode-only ?
Comment 6 Alexey Proskuryakov 2009-02-08 04:09:26 PST
FWIW, building WebKit with UNICODE undefined doesn't sound like a worthy goal to me - and RegOpenKeyExW looks even noisier than RegOpenKeyEx. Maybe someone spending more time with Win32 code will have a different opinion though.
Comment 7 Adrien Nader 2009-02-08 09:44:26 PST
Created attachment 27466 [details] Let the compiler decide which function version use, but be consistent ! The patch now uses the macros, not the functions or types directly. That nearly makes it possible to compile the file using the ansi functions and types. However that is not possible (nor desirable) because two functions from webkit's String.cpp are used so we display a nice error stating : "You need UNICODE defined to compile this file and webkit in general".
Comment 8 Alexey Proskuryakov 2010-06-11 23:07:11 PDT
This patch has never been marked for review, so it got overlooked. I'm not sure if it's worth marking it for review as is now, or if it has bit rotted.