nsIWindowsShellService::getRegistryEntry does not support non-latin1 keys/values

RESOLVED FIXED

Status

()

Firefox
Shell Integration
RESOLVED FIXED
13 years ago
12 years ago

People

(Reporter: Biesinger, Assigned: Jungshik Shin)

Tracking

({intl})

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

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.
Bug 272167 is the seamonkey version.
Keywords: intl
OS: Linux → Windows 2000
Depends on: 239279

Comment 2

12 years ago
Is the new registry interface from bug 292981 better in this respect?
(Assignee)

Comment 3

12 years ago
Yes, it is. Shouldn't we get rid of broken methods in this interface now that we
have a better interface?

Comment 4

12 years ago
Yes.  I really want to go through the tree and change all consumers over to
using the new registry API.
(Assignee)

Comment 5

12 years ago
Created attachment 184218 [details] [diff] [review]
patch to get rid of it

turned out that there's no consumer :-)
Assignee: bugs → jshin1987
Status: NEW → ASSIGNED
Attachment #184218 - Flags: superreview?(darin)
Attachment #184218 - Flags: review?(bugs)

Updated

12 years ago
Attachment #184218 - Flags: superreview?(darin) → superreview+
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.
Attachment #184218 - Flags: review?(bugs) → review+
(Assignee)

Comment 7

12 years ago
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

12 years ago
Thanks for writing up that release note jshin!  Maybe add the ContractID to that
note since the interface file doesn't document it.
(Assignee)

Comment 9

12 years ago
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.
Attachment #184218 - Flags: approval-aviary1.1a1?
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

12 years ago
Comment on attachment 184218 [details] [diff] [review]
patch to get rid of it

a=asa
Attachment #184218 - Flags: approval-aviary1.1a1? → approval-aviary1.1a1+
maybe the release note should also document whether to use createInstance or
getService?

Comment 13

12 years ago
biesi: the release notes are doctor'able... please feel free to edit the entry
for this.
ok, done.

Comment 15

12 years ago
I've created a new bug https://bugzilla.mozilla.org/show_bug.cgi?id=295360 with
some feedback to the new registry system
(Assignee)

Comment 16

12 years ago
sorry forgot to mark as fixed. fix landed on the trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.