The broken FMX TOpenDialog

Both Core Foundation and Cocoa – Apple’s ‘high level’ APIs (the first for C, the second Objective-C) – use UTF-16 encoding for strings, just like Delphi. Alas, but the XE2 source code consistently ignores this fact, and uses UTF-8 roundtrips instead. I can guess why: on the one hand (and not unreasonably), the RTL uses OS X’s POSIX layer as much as possible, which uses UTF-8; and on the other, the syntax for getting a Delphi string from a Cocoa or Core Foundation string via UTF-8 is, funnily enough, syntactically simpler than the code for avoiding a roundtrip conversion.

Quite apart from causing more code to be executed under the hood for no benefit though, the roundtrips also help cause the odd bug. Here’s one I found concerning the TOpenDialog class:

  • Using (say) TextEdit, create a text file called café.txt.
  • In Delphi, create a new FireMonkey HD application, and drop a TMemo, TButton and TOpenDialog on the form.
  • Handle the button’s OnClick event by executing the dialog, and passing on the chosen file to Memo1.Lines.LoadFromFile.
  • Target OS X, run the application, and try to load the text file created in step 1. If you can, then good for you, but for me there’s a big problem – the file name returned by the dialog is corrupted, leading to the LoadFromFile call to raise an exception.

Running with debug DCUs enabled, I found the problem: when retrieving the file name selected by the user, the UTF8String property of a NSString object is cast straight to string (see line 2661 of FMX.Platform.Mac.pas). What type is this property? Not the Delphi UTF8String type, but PAnsiChar, which should be expected, given NSString knows nothing of Delphi-specific types. Now, you may wonder why a PAnsiChar to string cast is allowed by the compiler in the first place (well, I do), or indeed, why the FMX.Platform.Mac.pas author didn’t use the UTF8ToUnicodeString RTL function. Nonetheless, the code as it is will work properly if the RTL’s default ‘Ansi’ code page is set to UTF-8 (since a PAnsiChar in itself carries no code page information, a global setting has to be used)… which it isn’t on my iMac. More exactly, the RTL’s attempt to find a suitable default (check out the implementation of GetACP in System.pas) fails, leading it to use a hard-coded choice of Windows Latin-1 instead. Since Latin-1 is only compatible with UTF-8 insofar as ASCII characters are used, and accented letters are not in the ASCII range, TOpenDialog therefore is incapable of returning a file name such as café.txt for me.

It is possible there is more than one bug here. If you look at the TEncoding, you’ll see that TEncoding.Default always returns a TUTF8Encoding instance when targetting the Mac, which corresponds to what I’ve read about OS X taking a much tougher line towards legacy code pages compared to Windows. This implies that the non-parameterised AnsiString type should be the same as UTF8String on the Mac, but it isn’t – instead, System.pas provides its own GetACP function for non-Windows targets, which as said, ends up setting Windows Latin-1 as the default if nothing sensible can be got from the OS.

Mind you, talking of bugs concerning TOpenDialog, here’s another: in line 2630 of FMX.Platform.Mac.pas, you’ll see the line

OpenFile.setCanChooseDirectories(True);

As the method name suggests, this means the user can return directories as well as files from a TOpenDialog, which is inconsistent with the behaviour on Windows. To be frank, the whole FMX.Dialogs unit and its support code looks rushed (the print-related dialogs aren’t implemented at all on the Mac!), just like various other parts of the framework. And have I mentioned the largely non-existent help…?

For what it’s worth, I’ve QCed the two definite bugs (99841, 99840), along with the general UTF-8 roundtrip issue (99839). If anyone wants to play the tiresome games required by the QC operator on my behalf, feel free, because I’m not going to.

Advertisements

5 thoughts on “The broken FMX TOpenDialog

    • Seeing it with others put me off adding reports before, and it’s not exactly encouraging me to log more frequently now. Quite simply, Embarcadero have put a person there who knows next to nothing about the product, and doesn’t have the curiosity to learn. Along with being highly annoying for the customer who does know a bit, this leads to reports being left open that can be closed (e.g., there’s an open FireMonkey report about TToolBar that’s based on a misunderstanding).

  1. IMHO you’re wrong. Submitting a test case (or at least clear, simple steps) is importandt for bug verification and opening. A link to a blog is not enough because the target could disappear anytime, while a test case is saved inside QC and can be submitted to the intenal developers.
    Embarcadero is already slow enough to fix bugs, better to submit as much as information as possibile, even if some bugs are so huge they could look obvious.

    • But I have given ‘clear, simple steps’ in the reports – care to show where I haven’t? (The exception is the report for the UTF-16 -> UTF-8 -> UTF-16 issue, since that isn’t a specific bug but a general coding issue.) If the Embarcadero guy can’t be bothered to follow them, then I can’t be bothered to waste any more of my time either.

      “Embarcadero is already slow enough to fix bugs, better to submit as much as information as possibile”

      Why? My complaints haven’t been like Eric Grange’s (for example), which require true expertise to rectify. Moreover, little since the XE2 release so far has made me confident FireMonkey will receive the care and attention it needs from its owners (which is not to say it won’t – I’m in a fence-sitting mood). Even if it does, I’d be surprised if I (as a purchaser of XE2, but unlikely purchaser of XE3) will see the benefit.

  2. Good luck with QC – there’s a good chance that they’ll just treat it as a documentation error/omission and simply update the documentation to describe the borked behaviour, then write it off as “as designed”.

    This is what they are going to do with the bug I reported in UCS4StringToUnicodeString().

    When I pointed out that this routine fails to convert the final character in the passed in array they chose to treat it as a documentation error.

    “The UCS4 String must be null terminated”

    Oh really? Good luck with that, since the code DOESN’T TEST FOR NULL AT ALL. If you DO pass in a NULL terminated string where the NULL does not occur as the final char in the array, it WILL NOT respect the NULL terminator.

    So I just have to wait for them to update the documentation mentioning this null termination expectation and THEN I can re-report the NEW BUG that the function is not respecting NULL termination.

    Pathetic.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s