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

RESOLVED FIXED

Status

()

Firefox
General
P3
normal
RESOLVED FIXED
14 years ago
13 years ago

People

(Reporter: Jungshik Shin, Assigned: Jungshik Shin)

Tracking

({fixed-aviary1.0, intl})

unspecified
x86
Windows XP
fixed-aviary1.0, intl
Points:
---
Dependency tree / graph
Bug Flags:
blocking-aviary1.0PR +
blocking-aviary1.0 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Assignee)

Description

14 years ago
While waiting for sr in bug 232969, firefox code has changed. I'm filing a new
bug on the issue for firefox alone.
(Assignee)

Updated

14 years ago
Assignee: firefox → jshin

Comment 1

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

Updated

13 years ago
Blocks: 241602

Comment 2

13 years ago
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 ...
I'm changing Requestee to "?".

This bug is important for non-ASCII environment.
And I think this is fixed easy.
Flags: blocking1.0?
(Assignee)

Comment 4

13 years ago
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. 

Status: NEW → ASSIGNED
Keywords: intl
(Assignee)

Comment 5

13 years ago
I can fix this bug the way I fixed bug 232969, but if we can take advantage of
MSLU, it will simplify things greatly. 
Depends on: 239279
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.
(Assignee)

Comment 7

13 years ago
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). 
(Assignee)

Comment 8

13 years ago
Created attachment 151651 [details] [diff] [review]
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.
(Assignee)

Comment 9

13 years ago
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? 

O.K.
Please wait.
(Assignee)

Comment 11

13 years ago
Created attachment 151672 [details] [diff] [review]
update 

I'm sorry there was a mistake in the previous patch although it may have just
worked. Please, try this one instead.
Attachment #151651 - Attachment is obsolete: true
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.
(Assignee)

Comment 13

13 years ago
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
Attachment #151672 - Flags: review?(bryner)
Flags: blocking-aviary1.0RC1+
Flags: blocking-aviary1.0?
Flags: blocking-aviary1.0+
Priority: -- → P3
(Assignee)

Comment 14

13 years ago
Comment on attachment 151672 [details] [diff] [review]
update 

let's try Blake for r.
Attachment #151672 - Flags: review?(bryner) → review?(firefox)
(Assignee)

Comment 15

13 years ago
Comment on attachment 151672 [details] [diff] [review]
update 

one more try.
Attachment #151672 - Flags: review?(firefox) → review?(bryner)
Comment on attachment 151672 [details] [diff] [review]
update 

s/For the now/For now/ and r=bryner.
Attachment #151672 - Flags: review?(bryner) → review+
(Assignee)

Comment 17

13 years ago
Created attachment 154624 [details] [diff] [review]
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.
(Assignee)

Comment 18

13 years ago
Created attachment 154626 [details] [diff] [review]
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

13 years ago
is this fix ready for the aviary branch?  -thx
(Assignee)

Comment 20

13 years ago
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.
Attachment #154626 - Flags: review?(bryner)
Comment on attachment 154626 [details] [diff] [review]
fix for the off-by-two error (against the trunk)

r=ben@mozilla.org

jshin, which of these patches are landed on the aviary br? can you land asap?
Attachment #154626 - Flags: review?(bryner) → review+
(Assignee)

Comment 22

13 years ago
Created attachment 155661 [details] [diff] [review]
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.
(Assignee)

Comment 23

13 years ago
checked into aviary-1.0 branch
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Whiteboard: fixed-aviary-1.0

Updated

13 years ago
Keywords: fixed-aviary1.0
Whiteboard: fixed-aviary-1.0
You need to log in before you can comment on or make changes to this bug.