Summary: | Fix mix of unicode and ansi functions in PluginDatabaseWin.cpp | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adrien Nader <camaradetux> | ||||||
Component: | Platform | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED WONTFIX | ||||||||
Severity: | Major | CC: | ahmad.saleem792, ap, camaradetux, rniwa | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | Windows 2000 | ||||||||
Attachments: |
|
Description
Adrien Nader
2009-02-07 08:05:51 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.
Forgot to say : I am aware that there is no changelog, I forgot it once again... 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 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.
(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 ? 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. 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".
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. PluginDatabaseWin.cpp was removed from WebkitLegacy based on following commit: https://github.com/WebKit/WebKit/commit/6436208aea2068805937c16a62cf97b57503e33f It does not exist in "Webkit" main source code. Further NPAPI and Webkit Plugin support has been removed from Safari and WebkitGTK builds. Can this be marked as "RESOLVED WONTFIX"? Thanks! |