Closed
Bug 234946
Opened 21 years ago
Closed 19 years ago
Windows File Picker Issues on Winnt (Unicode)
Categories
(Core :: XUL, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: mscott, Assigned: jshin1987)
References
Details
(Keywords: intl)
Attachments
(2 files, 2 obsolete files)
|
(deleted),
patch
|
Details | Diff | Splinter Review | |
|
(deleted),
patch
|
emaijala+moz
:
review+
jshin1987
:
superreview+
|
Details | Diff | Splinter Review |
For reference see:
Bug #232443 ( )
and
Bug #232443 (http://bugzilla.mozilla.org/show_bug.cgi?id=232443#c11)
|
Assignee | |
Comment 1•21 years ago
|
||
Just fixing the file picker (which is easy) wouldn't work without fixing bug
162361. I've been lobbying NSPR people (wtc and leaf) to fix NSPR file I/O APIs,
but they never replied to my email. It's really frustrating.
|
|
Assignee | |
Comment 2•19 years ago
|
||
Now that bug 162361 has been fixed and support for Win 9x/ME (and NT4) was dropped, we should use SendMessageW instead of SendMessageA in nsFilePicker.
Assignee: jag → jshin1987
|
||
Comment 3•19 years ago
|
||
Based on comment #2 idea.
|
|
Assignee | |
Comment 4•19 years ago
|
||
(In reply to comment #3)
> Created an attachment (id=217663) [edit]
> Patch rv 1.0
>
> Based on comment #2 idea.
You're not supposed to take away my bug without talking to me first. I made a similar patch after adding comment #2 but I was just too busy to take care of 'real life issues' to test and upload it.
My patch uses ToNewUnicode and nsMemory::Free instead of strdup.
|
|
||
Comment 5•19 years ago
|
||
Sorry, I didn't notice owner was changed.
Changing owner back to jshin.
Assignee: VYV03354 → jshin1987
Status: ASSIGNED → NEW
|
|
||
Updated•19 years ago
|
Attachment #217663 -
Flags: review?(emaijala)
|
|
Assignee | |
Comment 6•19 years ago
|
||
basically the same as attachment 217663 [details] [diff] [review] except that I got rid of the unncessary header inclusion and used ToNewUnicode/nsMemory::Free.
Attachment #217663 -
Attachment is obsolete: true
|
|
Assignee | |
Comment 7•19 years ago
|
||
SendMessageW is emulated on Win 9x/ME.
|
|
Assignee | |
Comment 8•19 years ago
|
||
(In reply to comment #6)
> Created an attachment (id=217851) [edit]
> patch for trunk
>
> basically the same as attachment 217663 [details] [diff] [review] [edit] except that I got rid of the unncessary
> header inclusion and used ToNewUnicode/nsMemory::Free.
Besides, BFFM_SETSELECTIONW is passed to SendMessageW instead of BFFM_SETSELECTION.
|
|
Assignee | |
Comment 9•19 years ago
|
||
Comment on attachment 217851 [details] [diff] [review]
patch for trunk
asking for r/sr
I've tested this build with thunderbird.
Attachment #217851 -
Flags: superreview?(neil)
Attachment #217851 -
Flags: review?(emaijala)
|
||
Comment 10•19 years ago
|
||
Comment on attachment 217851 [details] [diff] [review]
patch for trunk
>- if (initialDir.Length()) // convert folder path to native, the strdup copy will be released in BrowseCallbackProc
>+ if (initialDir.Length()) // Memory alloc'd by ToNewUnicode will be
>+ // released in BrowseCallbackProc
> {
>- nsCAutoString nativeFolderPath;
>- NS_CopyUnicodeToNative(initialDir, nativeFolderPath);
>- browserInfo.lParam = (LPARAM) nsCRT::strdup(nativeFolderPath.get());
>+ browserInfo.lParam = (LPARAM) ToNewUnicode(initialDir);
> browserInfo.lpfn = &BrowseCallbackProc;
> }
Nit: Because the dialog is modal initialDir.get() will be a valid pointer in BrowseCallbackProc and doesn't need to be copied like nativeFolderPath did.
Attachment #217851 -
Flags: superreview?(neil) → superreview+
|
|
Assignee | |
Comment 11•19 years ago
|
||
I don't clone the string any more per Neil's comment.
Attachment #217851 -
Attachment is obsolete: true
Attachment #218248 -
Flags: superreview+
Attachment #218248 -
Flags: review?(emaijala)
Attachment #217851 -
Flags: review?(emaijala)
|
||
Updated•19 years ago
|
Attachment #218248 -
Flags: review?(emaijala) → review+
|
|
Assignee | |
Comment 12•19 years ago
|
||
patch landed on the trunk. After baking it for a few days, I'll ask for the approval of the branch patch (although they're a little different)
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•