Opened 3 years ago

Closed 18 months ago

#13504 closed defect (fixed)

wxOSX-Cocoa bug handling non-ASCII file names

Reported by: andrewtrevorrow Owned by: csomor
Priority: normal Milestone: 2.9.5
Component: wxOSX-Cocoa Version: stable-latest
Keywords: regression wxString non-ASCII file name Cc:
Blocked By: Blocking:
Patch: yes

Description

I've attached a modified version of the dnd sample code to make it easy to see the bug. Replace dnd.cpp with my version, remake dnd.app, start it up and then do these steps:

  • Create a file (on the desktop is best) and give it a non-ASCII name by typing something like option-A (å).
  • Drag this file to the drop zone (upper left panel) in the dnd window. This sets a global wxString called lastdrop.
  • Now select Help > About to get a standard open file dialog and choose the same file. This sets a global wxString called lastopen.

The log messages show that these two strings are NOT the same, even though they look identical when displayed via wxLogStatus. In particular, lastopen is 1 character longer than lastdrop, and if it is fprinted out to the Console via

fprintf(stderr, "lastopen=%s\n", (const char*)lastopen.mb_str(wxConvLocal));

the result is an empty string. This problem does not occur in wxMac 2.8.12. Any idea what is going wrong or how I might work around the problem?

Attachments (2)

dnd.cpp download (57.9 KB) - added by andrewtrevorrow 3 years ago.
modified version of dnd sample code
normalize.patch download (3.3 KB) - added by andrewtrevorrow 3 years ago.

Download all attachments as: .zip

Change History (18)

Changed 3 years ago by andrewtrevorrow

modified version of dnd sample code

comment:1 Changed 3 years ago by vadz

Without looking/testing, this is almost surely a normalization problem. AFAIR OS X file system uses NFD while our string stuff probably uses NFC (see http://en.wikipedia.org/wiki/Unicode_equivalence#Normal_forms).

Not sure what to do about this though... Using NFD is more logical in some sense but working with NFC is simpler. So we probably should translate file names to NFC.

Stefan, what do you think?

comment:2 follow-up: Changed 3 years ago by andrewtrevorrow

  • Patch set

Yes, it is a normalization problem. I've attached a patch that fixes things, although probably not in the most sensible way. To avoid code duplication it would probably be better to add a new routine in src/common/filefn.cpp called wxNormalizePath or something similar.

Speaking of filefn.cpp, it looks like there might be memory leaks in wxMacFSRefToPath and wxMacHFSUniStrToString. Both those routines create a CFMutableStringRef by calling CFStringCreateMutableCopy but then fail to call CFRelease. The last lines in both those routines should be replaced by this code:

wxString result = wxCFStringRef(cfMutableString).AsString();
CFRelease(cfMutableString);
return result;

Changed 3 years ago by andrewtrevorrow

comment:3 Changed 3 years ago by csomor

  • Owner set to csomor
  • Status changed from new to accepted

I'll check this, we had once decided that within wx NFC should be used, so apparently when going from Navigation Services to NSFilePanels I lost that

comment:4 in reply to: ↑ 2 ; follow-up: Changed 3 years ago by csomor

Speaking of filefn.cpp, it looks like there might be memory leaks in wxMacFSRefToPath and wxMacHFSUniStrToString. Both those routines create a CFMutableStringRef by calling CFStringCreateMutableCopy but then fail to call CFRelease. The last lines in both those routines should be replaced by this code:

wxString result = wxCFStringRef(cfMutableString).AsString();
CFRelease(cfMutableString);
return result;

wxCFStringRef is releasing cfMutableString in its destructor, did you check the retain counts, am I overlooking an additional retain somewher

comment:5 in reply to: ↑ 4 Changed 3 years ago by andrewtrevorrow

Replying to csomor:

wxCFStringRef is releasing cfMutableString in its destructor...

Ah, I hadn't noticed that. No leakage then.

comment:6 Changed 2 years ago by vadz

We really should extract this in a reusable function (somewhere in include/wx/osx/core/private.h?) instead of repeating the same code in 4 places (and maybe more in the future) but other than this it looks correct to me.

Stefan, shouldn't we apply this for 2.9.3?

comment:7 Changed 2 years ago by vadz

  • Milestone changed from 2.9.3 to 2.9.4

2.9.3 is already past

comment:8 follow-up: Changed 2 years ago by vadz

I don't like the code repetition, could we have wxCFString::AsNFC() or something like this perhaps?

But in any case I think this really ought to be fixed. Stefan, any objections to applying this, possibly after refactoring?

comment:9 Changed 23 months ago by ardi

This patch fixed a bug I was experiencing in OSX-Cocoa 2.9.3: mb_str(wxConvISO8859_1) failed for strings returned by wxFileDialog.GetPath?()

Thanks a lot!! This fixed it!

ardi

comment:10 Changed 23 months ago by ardi

I just found that this bug also affects wxStandardPath.GetResourcesDir() and this patch doesn't fix it. I've looked at src/osx/core/stdpaths_cf.cpp and didn't get an idea on how to fix it. Also, I'm not sure if the OSX-Cocoa port is getting the GetResourcesDir() implementation from that file or not.

Btw, the bug I found here is this: put an app inside a dir with non-ASCII chars. Then, when the app starts, if you try to do  mb_str(wxConvISO8859_1) for GetResourcesDir(), it fails.

ardi

comment:11 Changed 23 months ago by ardi

The system introduced some typos (I'm not used to the autolink option) so I repost without typos (hopefully):

I just found that this bug also affects wxStandardPaths.GetResourcesDir?() and this patch doesn't fix it. I've looked at src/osx/core/stdpaths_cf.cpp and didn't get an idea on how to fix it. Also, I'm not sure if the OSX-Cocoa port is getting the GetResourcesDir() implementation from that file or not.

Btw, the bug I found here is this: put an app inside a dir with non-ASCII chars. Then, when the app starts, if you try to do mb_str(wxConvISO8859_1) for GetResourcesDir() it fails.

ardi

comment:12 Changed 23 months ago by ardi

If somebody has a quick solution for fixing this bug happening on wxStandardPaths.GetResourcesDir?() please tell, because I'd like to have this fixed on 2.9.3 (so that this works fine on my OSX 10.4 builds).

I guess it must be really easy, perhaps three or four lines of code, but I'm too new to OSX internals and I don't understand what the provided patch does, so I'm unable to fix this myself without risk of making things worse...

TIA

ardi

comment:13 Changed 22 months ago by vadz

  • Milestone changed from 2.9.4 to 2.9.5

Too late for 2.9.4, postponing to 2.9.5.

comment:14 in reply to: ↑ 8 Changed 18 months ago by vadz

Replying to vadz:

I don't like the code repetition, could we have wxCFString::AsNFC() or something like this perhaps?

Actually we already have wxCFStringRef::AsStringWithNormalizationFormC() so all we need to do is simply use it. I'll commit this soon.

comment:15 Changed 18 months ago by VZ

(In [72894]) Add wxCFStringRef::AsStringWithNormalizationFormC() Cocoa overload.

Provide an overload taking NSString and casting it to CFStringRef, just as for
AsStringWithNormalizationFormC().

See #13504.

comment:16 Changed 18 months ago by VZ

  • Resolution set to fixed
  • Status changed from accepted to closed

(In [72895]) Ensure that paths used inside wxOSX are always in NFC form.

OSX uses NFKD but this is unexpected for wx applications, so normalize the
string to use the composed form whenever we receive a file system path from OS
X.

Closes #13504.

Note: See TracTickets for help on using tickets.