Invalid unicode string from file dialogue on non-Windows system

[tobil4sk]

When using the save file dialogue on non-Windows systems, any non-ASCII characters in the file name result in an invalid string. This is because the string is returned as a wstring: https://github.com/openfl/lime/blob/develop/project/src/ExternalInterface.cpp#L902

While this works fine on Windows, this is problematic on Linux where wstring is not standard. As a result, if the string ends up being passed to hxcpp's system functions, it is considered invalid and we end up with confusing error messages (like Allocating from a GC-free thread which comes from a failed allocation of an exception string to describe this issue).

It's possible to workaround this issue by adding an extra conversion back to a regular string: develop...tobil4sk:lime:74eefc47a480f8055796310f8f3cd770a00b7b16

However, it would probably be better to refactor the api a bit to remove the need for wstring on non Windows systems altogether, because right now there are redundant conversions between string types just to fit this api:

Conversion to wstring:

lime/project/src/ExternalInterface.cpp

Lines 748 to 750 in d75b9f9

	std::wstring* _title = hxstring_to_wstring (title);
	std::wstring* _filter = hxstring_to_wstring (filter);
	std::wstring* _defaultPath = hxstring_to_wstring (defaultPath);

Conversion from wstring to string:

lime/project/src/ui/FileDialog.cpp

Lines 108 to 110 in d75b9f9

	std::string* _title = wstring_to_string (title);
	std::string* _filter = wstring_to_string (filter);
	std::string* _defaultPath = wstring_to_string (defaultPath);

[joshtynjala]

It looks like the 8.2.0-Dev branch has updated tinyfiledialogs to a newer version, and tinyfiledialogs has switched the default char encoding on Windows to UTF-8, instead of MBCS. If I'm understanding things correctly, it means that we shouldn't need to use the std::wstring APIs on Windows anymore, and Lime's FileDialog can just use std::string everywhere.

[joshtynjala]

Interestingly, with the following code and Lime 8.0.1, I tried opening a file named åéîøü.txt, and it outputs an incorrect string on Windows!,

var dialog = new FileDialog();
dialog.onSelect.add(value -> {
	trace("select file: " + value);
});
dialog.browse(OPEN);

src/Main.hx:26: open file: C:\Users\josht\Desktop\åéîøü.txt

It seems like, if we were using std::wstring to support Unicode on Windows, it was never actually working, or it broke at some point.

[tobil4sk]

src/Main.hx:26: open file: C:\Users\josht\Desktop\åéîøü.txt

This is just a problem with the trace output on Windows hxcpp, the same thing happens if you manually do trace("åéîøü.txt");:

├Ñ├®├«├©├╝.txt

If you read from the file on Windows there are no issues:

var dialog = new FileDialog();

dialog.onSelect.add(value -> {
	trace("select file: " + value); // select file: ...\├Ñ├®├«├©├╝.txt
	trace(sys.io.File.getContent(value)); // hello world!
});
dialog.browse(OPEN);

On other systems, it crashes when trying to read the file.

[joshtynjala]

This is just a problem with the trace output on Windows hxcpp, the same thing happens if you manually do trace("åéîøü.txt");:

Good to know, thanks! I guess I need to render these strings inside Lime/OpenFL instead, to verify that they're correct.

[joshtynjala]

I got std::string working on both Windows and macOS for the cpp target, but it was causing weird freezes in HashLink on Windows (but fine on macOS). I don't have any further time to debug that, so I decided to stick with std::wstring for now. I simply fixed the conversion of std::wstring to std::string on non-Windows.

[tobil4sk]

Nice, thanks. Looks like some builds are failing though, because the <cstring> header has not been included.

When you have a spare moment, could you upload the std::string version somewhere? I'm interested in what could be causing the freezes.

[joshtynjala]

Just adding some debugging code that I used, just in case I need to refer back to it in the future:

var debugTF = new TextField();
debugTF.x = stage.stageWidth / 2.0;
debugTF.width = stage.stageWidth / 2.0;
debugTF.height = stage.stageHeight;
debugTF.wordWrap = true;
addChild(debugTF);
function log(...str:String):Void {
	debugTF.appendText(str.toArray().join(" ") + "\n");
	debugTF.scrollV = debugTF.maxScrollV;
}

var openFile = new TextField();
openFile.autoSize = LEFT;
openFile.text = "Open File";
openFile.selectable = false;
openFile.background = true;
openFile.backgroundColor = 0xcccccc;
openFile.addEventListener(MouseEvent.CLICK, event -> {
	var dialog = new FileDialog();
	dialog.onSelect.add(value -> {
		log("open file: " + value);
		log(sys.io.File.getContent(value));
	});
	dialog.browse(OPEN, null, null, "Open File (åéîøü)");
});
openFile.x = 10;
openFile.y = 10;
addChild(openFile);

var saveFile = new TextField();
saveFile.autoSize = LEFT;
saveFile.text = "Save File";
saveFile.selectable = false;
saveFile.background = true;
saveFile.backgroundColor = 0xcccccc;
saveFile.addEventListener(MouseEvent.CLICK, event -> {
	var dialog = new FileDialog();
	dialog.onSelect.add(value -> {
		log("save file: " + value);
	});
	dialog.browse(SAVE, null, "/Users/joshtynjala/Desktop/åéîøü.txt", "Save File (åéîøü)");
});
saveFile.x = 10;
saveFile.y = 60;
addChild(saveFile);

var openDir = new TextField();
openDir.autoSize = LEFT;
openDir.text = "Open Directory";
openDir.selectable = false;
openDir.background = true;
openDir.backgroundColor = 0xcccccc;
openDir.addEventListener(MouseEvent.CLICK, event -> {
	var dialog = new FileDialog();
	dialog.onSelect.add(value -> {
		log("open dir: " + value);
	});
	dialog.browse(OPEN_DIRECTORY, null, null, "Open Dir (åéîøü)");
});
openDir.x = 10;
openDir.y = 110;
addChild(openDir);

var openMult = new TextField();
openMult.autoSize = LEFT;
openMult.text = "Open Multiple";
openMult.selectable = false;
openMult.background = true;
openMult.backgroundColor = 0xcccccc;
openMult.addEventListener(MouseEvent.CLICK, event -> {
	var dialog = new FileDialog();
	dialog.onSelectMultiple.add(value -> {
		log("open mult: " + value);
	});
	dialog.browse(OPEN_MULTIPLE, null, null, "Open Mult (åéîøü)");
});
openMult.x = 10;
openMult.y = 160;
addChild(openMult);

var openFile2 = new TextField();
openFile2.autoSize = LEFT;
openFile2.text = "Open File 2";
openFile2.selectable = false;
openFile2.background = true;
openFile2.backgroundColor = 0xcccccc;
openFile2.addEventListener(MouseEvent.CLICK, event -> {
	var dialog = new FileDialog();
	dialog.onOpen.add(value -> {
		log("open 2: " + value);
	});
	dialog.open(null, null, "Open File 2 (åéîøü)");
});
openFile2.x = 10;
openFile2.y = 210;
addChild(openFile2);

var saveFile2 = new TextField();
saveFile2.autoSize = LEFT;
saveFile2.text = "Save File 2";
saveFile2.selectable = false;
saveFile2.background = true;
saveFile2.backgroundColor = 0xcccccc;
saveFile2.addEventListener(MouseEvent.CLICK, event -> {
	var dialog = new FileDialog();
	dialog.onSave.add(value -> {
		log("save 2: " + value);
	});
	dialog.save("åéîøü", null, "/Users/joshtynjala/Desktop/åéîøü.txt", "Save File 2 (åéîøü)");
});
saveFile2.x = 10;
saveFile2.y = 260;
addChild(saveFile2);

[tobil4sk]

I've been looking back at this, it looks like maybe the issue here is actually not on lime's side...

Hxcpp's string encoding should be the same regardless of the OS, so I was struggling to understand why we needed have different behaviour for Linux/MacOS and Windows. alloc_wstring() should in theory result in a valid hxcpp string regardless of the platform. It looks like the real problem is that the String::create(wchar_t*) function in hxcpp doesn't take into account that wchat_t has a different length on platforms other than windows, and so it has been creating invalid strings (see HaxeFoundation/hxcpp#1077). Up until the String::create(wchar_t*) call, the strings are all valid.

Using alloc_string() forces it to use String::create(char*), which does not suffer from this issue, so really this might be a workaround rather than a proper fix.

I think this also means that wstring_to_vbytes on hashlink can be the same on all platforms? Since hashlink doesn't suffer from this issue, and from my testing was working fine before the patch.