240272 - prefLabel should be saved in the 'native' code in registry (bug 232969) patch

Status: RESOLVED FIXED

(Firefox :: General, defect, P3)

Description

[Jungshik Shin]

While waiting for sr in bug 232969, firefox code has changed. I'm filing a new
bug on the issue for firefox alone.

Comment 1

[Isaac Hwak Han]

*** Bug 237922 has been marked as a duplicate of this bug. ***

Comment 2

[Hidehiro Kozawa]

We have to change
browser\components\shell\src\nsWindowsShellService.cpp
All regkey functions use const char* :(
I wish someone to make it like
http://bugzilla.mozilla.org/attachment.cgi?id=144288&action=view ...

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I'm changing Requestee to "?".

This bug is important for non-ASCII environment.
And I think this is fixed easy.

Comment 4

[Jungshik Shin]

I'll make sure this will be fixed in 1.0. I can make an ad-hoc patch  rather
easily, but I'm a bit reluctant to duplicate quite a lot of code. This bug is
one of bugs that involves 'A' APIs vs 'W' APIs (see bug 162361 for the most
prominent example) and we need to make a decision as to what to do (keep on
adding ad-hoc patches or assume that MSLU is available everywhere and turn to
'W' APIs once and for all). Judging from the way bug 162361 has been handled so
far, I guess I have to resort to 'the old trick' once more here. 

Comment 5

[Jungshik Shin]

I can fix this bug the way I fixed bug 232969, but if we can take advantage of
MSLU, it will simplify things greatly. 

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I agree to make advanced mechanism for Unicode.
But I don't think that Bug 239279 will be able to be fixed before firefox 1.0.
Firefox 1.0 should not have this bug.

This bug is _not_ major. However I think that this bug is important.
Such bugs give the wrong impression to non-ASCII users.
# Such bugs are very conspicuous.

I hope this bug is fixed before '1.0'.
Even if the patch is not best.

Comment 7

[Jungshik Shin]

While trying to fix bug 239279, I'll make a _minimal_ patch for this bug that
will make Japanese characters work under Japanese locale (but not under
non-Japanese locale, e.g. French locale). 

Comment 8

[Jungshik Shin]

Attachment: patch

This patch makes Japanese string work on Japanese windows but non on French
Windows. For that, I need to do one of two: 1) detect the OS (win2k/xp vs
Win9x/ME) and use either 'A' or 'W' APIs accordingly (as was done in bug
232969) 2) use 'W' APIs exclusively assuming MSLU is present (see bug 239279).
I prefer the latter to the former, but that means I have to wait until bug
239279 is resolved. In the mean time, this patch should cover important cases.

Comment 9

[Jungshik Shin]

Nakano-san, can you test my patch on Windows XP? If you can't build, can you ask
someone else at mozilla-jp to build and test it? 

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

O.K.
Please wait.

Comment 11

[Jungshik Shin]

Attachment: update

I'm sorry there was a mistake in the previous patch although it may have just
worked. Please, try this one instead.

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I tested.
JLP for 0.9.1-branch-build doesn't exist, and so I rewrite
'en-US\browser\shellservice.properties' directly.

In this case, the patch works fine.

Comment 13

[Jungshik Shin]

Comment on attachment 151672 [details] [diff] [review]
update 

asking for r.
This should also go in for 0.9branch as well as aviary-1.0 branch

Comment 14

[Jungshik Shin]

Comment on attachment 151672 [details] [diff] [review]
update 

let's try Blake for r.

Comment 15

[Jungshik Shin]

Comment on attachment 151672 [details] [diff] [review]
update 

one more try.

Comment 16

[Brian Ryner (not reading)]

Comment on attachment 151672 [details] [diff] [review]
update 

s/For the now/For now/ and r=bryner.

Comment 17

[Jungshik Shin]

Attachment: patch checked in with the fix for off-by-two errors

The current tree has the following:

exeName = Substring(appPath, n + 1, appPath.Length() - (n - 1));

where 'n' is the offset of the last '\' in appPath. I didn't change it in my
patch (attachment 151672 [details] [diff] [review]).  However, it seems that there is an off-by-two
error. The above line should be

exeName = Substring(appPath, n + 1, appPath.Length() - (n + 1));

Perhaps, we're lucky not to have any trouble so far.

Comment 18

[Jungshik Shin]

Attachment: fix for the off-by-two error (against the trunk)

this contains the fix for the off-by-two error only (against the trunk). 

attachment 151672 [details] [diff] [review] + this patch + caling sequence change for SetRegKey
introduced on June 30th = attachment 154624 [details] [diff] [review]

Just in case I'm missing something, Nakano-san, can you apply this to the trunk
source and see if it works well.

Comment 19

[chris hofmann]

is this fix ready for the aviary branch?  -thx

Comment 20

[Jungshik Shin]

Comment on attachment 154626 [details] [diff] [review]
fix for the off-by-two error (against the trunk)

asking for r.
this should be the right thing to do.

Comment 21

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 154626 [details] [diff] [review]
fix for the off-by-two error (against the trunk)

r=[e-mail]

jshin, which of these patches are landed on the aviary br? can you land asap?

Comment 22

[Jungshik Shin]

Attachment: for aviary 1.0 branch

There have been  a couple of trivial changes due to which attachment 154624 [details] [diff] [review]
can't be applied cleanly to aviary 1.0 branch.

This should just work, but I'm away from my Windows 2k box so that I can't
compile it (I don't have an XP so that I can test it). Nakano-san, can you test
it on aviary-1.0 branch on Windows XP? 
If I dont' hear from you until 7:59am US PDT  Tuesday(= 11:59 pm +0900/JST/KST
Tuesday), I'll just go ahead.

Comment 23

[Jungshik Shin]

checked into aviary-1.0 branch