Last Comment Bug 272166 - nsIWindowsShellService::getRegistryEntry does not support non-latin1 keys/values
: nsIWindowsShellService::getRegistryEntry does not support non-latin1 keys/values
Status: RESOLVED FIXED
: intl
Product: Firefox
Classification: Client Software
Component: Shell Integration (show other bugs)
: unspecified
: x86 Windows 2000
-- normal (vote)
: ---
Assigned To: Jungshik Shin
:
: Robert Strong [:rstrong] (use needinfo to contact me)
Mentors:
http://lxr.mozilla.org/seamonkey/sour...
Depends on: mzlu
Blocks:
  Show dependency treegraph
 
Reported: 2004-11-28 14:52 PST by Christian :Biesinger (don't email me, ping me on IRC)
Modified: 2005-05-24 16:41 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch to get rid of it (4.17 KB, patch)
2005-05-21 22:47 PDT, Jungshik Shin
mconnor: review+
darin.moz: superreview+
asa: approval‑aviary1.1a1+
Details | Diff | Splinter Review

Description User image Christian :Biesinger (don't email me, ping me on IRC) 2004-11-28 14:52:08 PST
79     string getRegistryEntry(in long aHKeyConstant, in string aSubKeyName, in
string aValueName);

This means that only latin1 is supported. Worse, on a non-latin1 (guess that's
really a non-windows1252) system, the characters will be interpreted as latin1,
likely leading to garbage for non-ascii characters.

this should use wstring or AString in the IDL. seamonkey has the same bug, which
I will file in a second.
Comment 1 User image Christian :Biesinger (don't email me, ping me on IRC) 2004-11-28 14:55:10 PST
Bug 272167 is the seamonkey version.
Comment 2 User image Jesse Ruderman 2005-05-21 21:31:02 PDT
Is the new registry interface from bug 292981 better in this respect?
Comment 3 User image Jungshik Shin 2005-05-21 22:14:02 PDT
Yes, it is. Shouldn't we get rid of broken methods in this interface now that we
have a better interface?

Comment 4 User image Darin Fisher 2005-05-21 22:15:39 PDT
Yes.  I really want to go through the tree and change all consumers over to
using the new registry API.
Comment 5 User image Jungshik Shin 2005-05-21 22:47:02 PDT
Created attachment 184218 [details] [diff] [review]
patch to get rid of it

turned out that there's no consumer :-)
Comment 6 User image Mike Connor [:mconnor] 2005-05-23 00:57:54 PDT
Comment on attachment 184218 [details] [diff] [review]
patch to get rid of it

ben's so far behind on reviews right now it isn't funny.  We added this from
winhooks because extensions used it, removing this will break Launchy and other
extensions that are relying on it.

r=me assuming that someone writes me a quick summary for the release notes, and
link to something that explains the new interface.

Email is probably best for those changes.
Comment 7 User image Jungshik Shin 2005-05-23 01:52:55 PDT
Release note entry:

nsIWindowsShellService::getRegistryEntry was removed because it does not support
handling of non-ASCII characters. |open| and |readStringValue| methods of a new
and better interface |nsIWindowsRegKey| can be used. For more details on the
interface including the descriptions of other methods, see
xpcom/ds/nsIWindowsRegKey.idl

I'll send this to Mike

 
Comment 8 User image Darin Fisher 2005-05-23 06:56:57 PDT
Thanks for writing up that release note jshin!  Maybe add the ContractID to that
note since the interface file doesn't document it.
Comment 9 User image Jungshik Shin 2005-05-23 18:55:22 PDT
Comment on attachment 184218 [details] [diff] [review]
patch to get rid of it

Probably too late 1.1a1. (I don't mind this being shifted to 1.1a2) This is a
very low risk trivial patch except that a couple of extensions use the
inferface being removed. I've provided a draft for the release notes.
Comment 10 User image Mike Connor [:mconnor] 2005-05-23 19:34:55 PDT
http://www.mozilla.org/projects/deerpark/new-extension-dev-features.html has
this, and gemal was the main consumer (and the reason it was added in the first
place) so I'm sure this should be a huge fallout.  I'd like to take this now
instead of later, its not a risk to our builds, it just might break older revs
of extensions, but sooner is better than later on that count.
Comment 11 User image Asa Dotzler [:asa] 2005-05-23 20:58:29 PDT
Comment on attachment 184218 [details] [diff] [review]
patch to get rid of it

a=asa
Comment 12 User image Christian :Biesinger (don't email me, ping me on IRC) 2005-05-24 04:35:42 PDT
maybe the release note should also document whether to use createInstance or
getService?
Comment 13 User image Darin Fisher 2005-05-24 09:38:27 PDT
biesi: the release notes are doctor'able... please feel free to edit the entry
for this.
Comment 14 User image Christian :Biesinger (don't email me, ping me on IRC) 2005-05-24 10:00:30 PDT
ok, done.
Comment 15 User image Henrik Gemal 2005-05-24 10:03:58 PDT
I've created a new bug https://bugzilla.mozilla.org/show_bug.cgi?id=295360 with
some feedback to the new registry system
Comment 16 User image Jungshik Shin 2005-05-24 16:41:08 PDT
sorry forgot to mark as fixed. fix landed on the trunk

Note You need to log in before you can comment on or make changes to this bug.